This is an automated email from the ASF dual-hosted git repository.
mbudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 424f66783c [CALCITE-6735] Type coercion for comparisons does not
coerce ROW types
424f66783c is described below
commit 424f66783cf7f5aeafed8fca4cfa8c004716385f
Author: Mihai Budiu <[email protected]>
AuthorDate: Tue Dec 17 17:13:53 2024 -0800
[CALCITE-6735] Type coercion for comparisons does not coerce ROW types
Signed-off-by: Mihai Budiu <[email protected]>
---
.../adapter/enumerable/RexToLixTranslator.java | 30 ++++++++++++++++++++++
.../validate/implicit/AbstractTypeCoercion.java | 23 +++++++++++++++++
.../java/org/apache/calcite/test/JdbcTest.java | 8 +++++-
.../org/apache/calcite/test/SqlValidatorTest.java | 2 ++
.../org/apache/calcite/test/TypeCoercionTest.java | 2 ++
core/src/test/resources/sql/misc.iq | 29 +++++++++++++++++++++
6 files changed, 93 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
index d3411654fe..e38ac3210b 100644
---
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
+++
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
@@ -32,6 +32,7 @@ import org.apache.calcite.linq4j.tree.ParameterExpression;
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 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
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
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java
b/core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java
index 2d5970440d..8c57c82214 100644
---
a/core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java
+++
b/core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java
@@ -22,6 +22,7 @@ import org.apache.calcite.rel.type.RelDataType;
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 @@ public abstract class AbstractTypeCoercion implements
TypeCoercion {
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;
}
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index f08e99322e..0dcc797d4e 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -6955,7 +6955,13 @@ public class JdbcTest {
@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");
}
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 37a7050701..24951049ef 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -1945,6 +1945,8 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
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() {
diff --git a/core/src/test/java/org/apache/calcite/test/TypeCoercionTest.java
b/core/src/test/java/org/apache/calcite/test/TypeCoercionTest.java
index e5375686b3..7377b9cfe1 100644
--- a/core/src/test/java/org/apache/calcite/test/TypeCoercionTest.java
+++ b/core/src/test/java/org/apache/calcite/test/TypeCoercionTest.java
@@ -404,6 +404,8 @@ class TypeCoercionTest {
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)));
diff --git a/core/src/test/resources/sql/misc.iq
b/core/src/test/resources/sql/misc.iq
index 23a602bcbb..8c34645347 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/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);
+--------+
