use string representation for matching (#2571)

main/src/com/google/refine/LookupCacheManager.java

@@ -87,8 +87,9 @@ public class LookupCacheManager {
             Row targetRow = targetProject.rows.get(r);
             Object value = targetRow.getCellValue(targetColumn.getCellIndex());
             if (ExpressionUtils.isNonBlankData(value)) {
-                lookup.valueToRowIndices.putIfAbsent(value, new ArrayList<>());
+                String valueStr = value.toString();
-                lookup.valueToRowIndices.get(value).add(r);
+                lookup.valueToRowIndices.putIfAbsent(valueStr, new ArrayList<>());
+                lookup.valueToRowIndices.get(valueStr).add(r);
             }
         }
     }
@@ -106,11 +107,13 @@ public class LookupCacheManager {
         }
 
         public HasFieldsListImpl getRows(Object value) {
-            if (ExpressionUtils.isNonBlankData(value) && valueToRowIndices.containsKey(value)) {
+            if (!ExpressionUtils.isNonBlankData(value)) return null;
+            String valueStr = value.toString();
+            if (valueToRowIndices.containsKey(valueStr)) {
                 Project targetProject = ProjectManager.singleton.getProject(targetProjectID);
                 if (targetProject != null) {
                     HasFieldsListImpl rows = new HasFieldsListImpl();
-                    for (Integer r : valueToRowIndices.get(value)) {
+                    for (Integer r : valueToRowIndices.get(valueStr)) {
                         Row row = targetProject.rows.get(r);
                         rows.add(new WrappedRow(targetProject, r, row));
                     }

main/src/com/google/refine/expr/functions/Cross.java

@@ -76,17 +76,17 @@ public class Cross implements Function {
                 }
             }
         }
-        return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects a cell or cell value, a project name to look up, and a column name in that project");
+        return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects a cell or value, a project name to look up, and a column name in that project");
     }
 
     @Override
     public String getDescription() {
-        return "Looks up the given value in the target column of the target project, returns an array of matched rows, cell will be interpreted as cell.value";
+        return "Looks up the given value in the target column of the target project, returns an array of matched rows, cell will be interpreted as cell.value. Two values match if and only if they have the same string representation";
     }
 
     @Override
     public String getParams() {
-        return "cell c or object value, string projectName, string columnName";
+        return "cell or value, string projectName, string columnName";
     }
 
     @Override

main/tests/server/src/com/google/refine/expr/functions/CrossTests.java

@@ -149,8 +149,10 @@ public class CrossTests extends RefineTest {
         Assert.assertEquals(address, "50 Broadway Ave.");
     }
 
+    /**
+     * The result shouldn't depend on the based column in "bindings" when the first argument is a WrappedCell instance.
+     */
     @Test
-    // cross result shouldn't depend on the based column in "bindings" when the first argument is a WrappedCell instance
     public void crossFunctionDifferentColumnTest() throws Exception {
         Project project = (Project) bindings.get("project");
         bindings.put("columnName", "gift"); // change the based column
@@ -221,6 +223,29 @@ public class CrossTests extends RefineTest {
         Assert.assertEquals(address, "integer");
     }
 
+    @Test
+    public void crossFunctionIntegerArgumentTest1() throws Exception {
+        Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", 1600L, "My Address Book", "friend")).get(0)).row);
+        String address = row.getCell(1).value.toString();
+        Assert.assertEquals(address, "integer");
+    }
+
+    @Test
+    public void crossFunctionIntegerArgumentTest2() throws Exception {
+        Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", "1600", "My Address Book", "friend")).get(0)).row);
+        String address = row.getCell(1).value.toString();
+        Assert.assertEquals(address, "integer");
+    }
+
+    /**
+     * Two values will match if and only if they have the same string representation.
+     * In this case, "1600.0" doesn't equal to "1600".
+     */
+    @Test
+    public void crossFunctionIntegerArgumentTest3() throws Exception {
+        Assert.assertNull(invoke("cross", "1600.0", "My Address Book", "friend"));
+    }
+
     @Test
     public void crossFunctionLongArgumentTest() throws Exception {
         Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", 123456789123456789L, "My Address Book", "friend")).get(0)).row);
@@ -228,6 +253,13 @@ public class CrossTests extends RefineTest {
         Assert.assertEquals(address, "long");
     }
 
+    @Test
+    public void crossFunctionLongArgumentTest1() throws Exception {
+        Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", "123456789123456789", "My Address Book", "friend")).get(0)).row);
+        String address = row.getCell(1).value.toString();
+        Assert.assertEquals(address, "long");
+    }
+
     @Test
     public void crossFunctionDoubleArgumentTest() throws Exception {
         Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", 3.14, "My Address Book", "friend")).get(0)).row);
@@ -235,6 +267,20 @@ public class CrossTests extends RefineTest {
         Assert.assertEquals(address, "double");
     }
 
+    @Test
+    public void crossFunctionDoubleArgumentTest1() throws Exception {
+        Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", 3.14f, "My Address Book", "friend")).get(0)).row);
+        String address = row.getCell(1).value.toString();
+        Assert.assertEquals(address, "double");
+    }
+
+    @Test
+    public void crossFunctionDoubleArgumentTest2() throws Exception {
+        Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", "3.14", "My Address Book", "friend")).get(0)).row);
+        String address = row.getCell(1).value.toString();
+        Assert.assertEquals(address, "double");
+    }
+
     @Test
     public void crossFunctionDateTimeArgumentTest() throws Exception {
         Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", dateTimeValue, "My Address Book", "friend")).get(0)).row);
@@ -242,6 +288,13 @@ public class CrossTests extends RefineTest {
         Assert.assertEquals(address, "dateTime");
     }
 
+    @Test
+    public void crossFunctionDateTimeArgumentTest1() throws Exception {
+        Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", dateTimeValue.toString(), "My Address Book", "friend")).get(0)).row);
+        String address = row.getCell(1).value.toString();
+        Assert.assertEquals(address, "dateTime");
+    }
+
     @Test
     public void crossFunctionBooleanArgumentTest() throws Exception {
         Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", true, "My Address Book", "friend")).get(0)).row);
@@ -249,6 +302,13 @@ public class CrossTests extends RefineTest {
         Assert.assertEquals(address, "boolean");
     }
 
+    @Test
+    public void crossFunctionBooleanArgumentTest1() throws Exception {
+        Row row = (((WrappedRow) ((HasFieldsListImpl) invoke("cross", "true", "My Address Book", "friend")).get(0)).row);
+        String address = row.getCell(1).value.toString();
+        Assert.assertEquals(address, "boolean");
+    }
+
     /**
      * If no match, return null.
      * 
@@ -268,7 +328,7 @@ public class CrossTests extends RefineTest {
     @Test
     public void crossFunctionNonLiteralValue() throws Exception {
         Assert.assertEquals(((EvalError) invoke("cross", null, "My Address Book", "friend")).message,
-                "cross expects a cell or cell value, a project name to look up, and a column name in that project");
+                "cross expects a cell or value, a project name to look up, and a column name in that project");
     }
     
     /**
@@ -289,7 +349,7 @@ public class CrossTests extends RefineTest {
     
     @Test
     public void serializeCross() {
-        String json = "{\"description\":\"Looks up the given value in the target column of the target project, returns an array of matched rows, cell will be interpreted as cell.value\",\"params\":\"cell c or object value, string projectName, string columnName\",\"returns\":\"array\"}";
+        String json = "{\"description\":\"Looks up the given value in the target column of the target project, returns an array of matched rows, cell will be interpreted as cell.value. Two values match if and only if they have the same string representation\",\"params\":\"cell or value, string projectName, string columnName\",\"returns\":\"array\"}";
         TestUtils.isSerializedTo(new Cross(), json);
     }
 }