[CALCITE-6735] Type coercion for comparisons does not coerce ROW types

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java

@@ -32,6 +32,7 @@
 import org.apache.calcite.linq4j.tree.Primitive;
 import org.apache.calcite.linq4j.tree.Statement;
 import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexCall;
 import org.apache.calcite.rex.RexCallBinding;
@@ -331,6 +332,35 @@ private Expression getConvertExpression(
       return Expressions.convert_(cast, typeFactory.getJavaClass(nullableTarget));
     }
 
+    if (targetType.getSqlTypeName() == SqlTypeName.ROW) {
+      assert sourceType.getSqlTypeName() == SqlTypeName.ROW;
+      List<RelDataTypeField> targetTypes = targetType.getFieldList();
+      List<RelDataTypeField> sourceTypes = sourceType.getFieldList();
+      assert targetTypes.size() == sourceTypes.size();
+      List<Expression> fields = new ArrayList<>();
+      for (int i = 0; i < targetTypes.size(); i++) {
+        RelDataTypeField targetField = targetTypes.get(i);
+        RelDataTypeField sourceField = sourceTypes.get(i);
+        Expression field = Expressions.arrayIndex(operand, Expressions.constant(i));
+        // In the generated Java code 'field' is an Object,
+        // we need to also cast it to the correct type to enable correct method dispatch in Java.
+        // We force the type to be nullable; this way, instead of (int) we get (Integer).
+        // Casting an object ot an int is not legal.
+        RelDataType nullableSourceFieldType =
+            typeFactory.createTypeWithNullability(sourceField.getType(), true);
+        Type javaType = typeFactory.getJavaClass(nullableSourceFieldType);
+        if (!javaType.getTypeName().equals("java.lang.Void")
+            && !nullableSourceFieldType.isStruct()) {
+          // Cannot cast to Void - this is the type of NULL literals.
+          field = Expressions.convert_(field, javaType);
+        }
+        Expression convert =
+            getConvertExpression(sourceField.getType(), targetField.getType(), field, format);
+        fields.add(convert);
+      }
+      return Expressions.call(BuiltInMethod.ARRAY.method, fields);
+    }
+
     switch (targetType.getSqlTypeName()) {
     case VARIANT:
       // Converting any type to a VARIANT invokes the Variant constructor

core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java

@@ -22,6 +22,7 @@
 import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeFactoryImpl;
 import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
 import org.apache.calcite.runtime.PairList;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlCharStringLiteral;
@@ -667,6 +668,28 @@ private RelDataType getTightestCommonTypeOrThrow(
           resultType, type1.isNullable() || type2.isNullable());
     }
 
+    if (typeName1 == SqlTypeName.ROW) {
+      if (typeName2 != typeName1) {
+        return null;
+      }
+      if (type1.getFieldCount() != type2.getFieldCount()) {
+        return null;
+      }
+      List<RelDataTypeField> leftFields = type1.getFieldList();
+      List<RelDataTypeField> rightFields = type2.getFieldList();
+      List<RelDataTypeField> resultFields = new ArrayList<>();
+      for (int i = 0; i < leftFields.size(); i++) {
+        RelDataTypeField leftField = leftFields.get(i);
+        RelDataType type =
+            commonTypeForBinaryComparison(leftField.getType(), rightFields.get(i).getType());
+        if (type == null) {
+          return null;
+        }
+        resultFields.add(new RelDataTypeFieldImpl(leftField.getName(), i, type));
+      }
+      return factory.createStructType(resultFields);
+    }
+
     return null;
   }
 

core/src/test/java/org/apache/calcite/test/JdbcTest.java

@@ -6955,7 +6955,13 @@ private void checkGetTimestamp(Connection con) throws SQLException {
   @Test void testRowComparison() {
     CalciteAssert.that()
         .with(CalciteAssert.Config.JDBC_SCOTT)
-        .query("SELECT empno FROM JDBC_SCOTT.emp WHERE (ename, job) < ('Blake', 'Manager')")
+        // The extra casts are necessary because HSQLDB does not support a ROW type,
+        // and in the absence of these explicit casts the code generated contains
+        // a cast of a ROW value.  The correct way to fix this would be to improve
+        // the code generation for HSQLDB to expand suc casts into constructs
+        // supported by HSQLDB.
+        .query("SELECT empno FROM JDBC_SCOTT.emp WHERE (ename, job) < "
+            + "(CAST('Blake' AS VARCHAR(10)), CAST('Manager' AS VARCHAR(9)))")
         .returnsUnordered("EMPNO=7876", "EMPNO=7499", "EMPNO=7698");
   }
 

core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java

@@ -1945,6 +1945,8 @@ void testLikeAndSimilarFails() {
     sql("select row((select deptno from dept where dept.deptno = emp.deptno), emp.ename)\n"
         + "from emp")
         .columnType("RecordType(INTEGER EXPR$0, VARCHAR(20) NOT NULL EXPR$1) NOT NULL");
+    sql("select ROW^(x'12') <> ROW(0.01)^")
+        .fails("Cannot apply '<>' to arguments of type.*");
   }
 
   @Test void testRowWithValidDot() {

core/src/test/java/org/apache/calcite/test/TypeCoercionTest.java

@@ -404,6 +404,8 @@ private static ImmutableList<RelDataType> combine(
         null);
     f.comparisonCommonType(f.recordType("a", f.intType),
         f.recordType("a", f.intType), f.recordType("a", f.intType));
+    f.comparisonCommonType(f.recordType("a", f.intType),
+        f.recordType("a", f.charType), f.recordType("a", f.intType));
     f.comparisonCommonType(f.recordType("a", f.arrayType(f.intType)),
         f.recordType("a", f.arrayType(f.intType)),
         f.recordType("a", f.arrayType(f.intType)));

core/src/test/resources/sql/misc.iq

@@ -18,6 +18,35 @@
 !use post
 !set outputformat mysql
 
+# Compared by casting varchar to number
+SELECT '1' > 0 AS C;
++------+
+| C    |
++------+
+| true |
++------+
+(1 row)
+
+!ok
+
+# [CALCITE-6735] Type coercion for comparisons does not coerce ROW types
+# These are compared in the same way as '1' > 0
+SELECT ROW('1') > ROW(0) AS C;
++------+
+| C    |
++------+
+| true |
++------+
+(1 row)
+
+!ok
+
+# [CALCITE-6735] Type coercion for comparisons does not coerce ROW types
+# These are compared in the same way as x'10' > 0, which throws
+SELECT ROW(x'10') > ROW(0) AS C;
+java.sql.SQLException: Error while executing SQL "SELECT ROW(x'10') > ROW(0) AS C": From line 1, column 11 to line 1, column 26: Cannot apply '>' to arguments of type '<RECORDTYPE(BINARY(1) EXPR$0)> > <RECORDTYPE(INTEGER EXPR$0)>'.
+!error
+
 # [CALCITE-6733] Type inferred by coercion for comparisons with decimal is too narrow
 SELECT ASCII('8') >= ABS(1.1806236821);
 +--------+