This is an automated email from the ASF dual-hosted git repository. tanner pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 95c4acc3cccdc66a3fd39a41250cb0d7f86c56ee Author: Tanner Clary <tannercl...@google.com> AuthorDate: Sun Jul 23 19:37:29 2023 -0700 [CALCITE-5869] LEAST_RESTRICTIVE does not use inner type of MEASURE for comparisons --- .../src/main/java/org/apache/calcite/sql/SqlOperator.java | 6 ++++++ .../java/org/apache/calcite/sql/type/SqlTypeUtil.java | 10 ++++++++++ .../apache/calcite/sql/type/RelDataTypeSystemTest.java | 15 +++++++++++++++ .../java/org/apache/calcite/test/SqlValidatorTest.java | 9 +++++++++ .../calcite/test/catalog/MockCatalogReaderExtended.java | 4 +++- 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java index 342113b306..4eb0a7fc8b 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java @@ -22,6 +22,7 @@ import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.type.MeasureSqlType; import org.apache.calcite.sql.type.SqlOperandTypeChecker; import org.apache.calcite.sql.type.SqlOperandTypeInference; import org.apache.calcite.sql.type.SqlReturnTypeInference; @@ -537,6 +538,11 @@ public abstract class SqlOperator { + opBinding.collectOperandTypes()); } + // MEASURE wrapper should be removed, e.g. MEASURE<DOUBLE> should just be DOUBLE + if (returnType instanceof MeasureSqlType) { + returnType = returnType.getMeasureElementType(); + } + if (operandTypeInference != null && opBinding instanceof SqlCallBinding && this instanceof SqlFunction) { diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java index 5503a2034d..3c838c8081 100644 --- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java +++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java @@ -783,6 +783,10 @@ public abstract class SqlTypeUtil { return t.getFamily() == SqlTypeFamily.ANY; } + private static boolean isMeasure(RelDataType t) { + return t instanceof MeasureSqlType; + } + /** * Tests whether a value can be assigned to a site. * @@ -896,6 +900,12 @@ public abstract class SqlTypeUtil { if (toType.equals(fromType)) { return true; } + // If fromType is a measure, you should compare the inner type otherwise it will + // always return TRUE because the SqlTypeFamily of MEASURE is ANY + if (isMeasure(fromType)) { + return canCastFrom(toType, requireNonNull(fromType.getMeasureElementType()), + typeMappingRule); + } if (isAny(toType) || isAny(fromType)) { return true; } diff --git a/core/src/test/java/org/apache/calcite/sql/type/RelDataTypeSystemTest.java b/core/src/test/java/org/apache/calcite/sql/type/RelDataTypeSystemTest.java index 9a9513fcea..bf74949e0d 100644 --- a/core/src/test/java/org/apache/calcite/sql/type/RelDataTypeSystemTest.java +++ b/core/src/test/java/org/apache/calcite/sql/type/RelDataTypeSystemTest.java @@ -19,12 +19,15 @@ package org.apache.calcite.sql.type; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.rel.type.RelDataTypeSystemImpl; +import org.apache.calcite.sql.fun.SqlLibraryOperators; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import com.google.common.collect.Lists; import org.junit.jupiter.api.Test; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -152,6 +155,18 @@ class RelDataTypeSystemTest { .newArrayList(operand1, operand2)); assertEquals(SqlTypeName.DOUBLE, dataType.getSqlTypeName()); } + + /** Tests that LEAST_RESTRICTIVE considers a MEASURE's element type + * <a href="https://issues.apache.org/jira/browse/CALCITE-5869">[CALCITE-5869] + * LEAST_RESTRICTIVE does not use MEASURE element type</a>. */ + @Test void testLeastRestrictiveUsesMeasureElementType() { + RelDataType innerType = TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE); + RelDataType operand1 = TYPE_FACTORY.createMeasureType(innerType); + RelDataType operand2 = TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER); + RelDataType dataType = SqlLibraryOperators.IFNULL + .inferReturnType(TYPE_FACTORY, Lists.newArrayList(operand1, operand2)); + assertThat(dataType, is(innerType)); + } @Test void testCustomDecimalPlusReturnTypeInference() { RelDataType operand1 = CUSTOM_FACTORY.createSqlType(SqlTypeName.DECIMAL, 38, 10); 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 6d5823e22d..576cc7a2f0 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -3879,6 +3879,15 @@ public class SqlValidatorTest extends SqlValidatorTestCase { .isAggregate(is(false)); } + @Test void testLeastRestrictiveUsesMeasureElement() { + SqlValidatorFixture f = + fixture().withExtendedCatalog() + .withOperatorTable(operatorTableFor(SqlLibrary.BIG_QUERY)); + f.withSql("select ifnull(count_times_100, 0) from empm") + .type("RecordType(MEASURE<DECIMAL(19, 0) NOT NULL> " + + "NOT NULL EXPR$0) NOT NULL"); + } + @Test void testAmbiguousColumnInIn() { // ok: cyclic reference sql("select * from emp as e\n" diff --git a/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java b/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java index e8ef58f84a..c4a4eada2e 100644 --- a/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java +++ b/testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java @@ -139,7 +139,7 @@ public class MockCatalogReaderExtended extends MockCatalogReaderSimple { registerTable(mockEmpViewTable3); // Register "EMPM" table. - // Same as "EMP" but with a "COUNT_PLUS_100" measure column. + // Same as "EMP" but with "COUNT_PLUS_100" and "COUNT_TIMES_100" measure columns. final MockTable empmTable = MockTable.create(this, salesSchema, "EMPM", false, 14); empmTable.addColumn("EMPNO", f.intType, true); @@ -153,6 +153,8 @@ public class MockCatalogReaderExtended extends MockCatalogReaderSimple { empmTable.addColumn("SLACKER", f.booleanType); empmTable.addColumn("COUNT_PLUS_100", f.typeFactory.createMeasureType(f.intType)); + empmTable.addColumn("COUNT_TIMES_100", + f.typeFactory.createMeasureType(f.decimalType)); registerTable(empmTable); MockSchema structTypeSchema = new MockSchema("STRUCT");