You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by ta...@apache.org on 2023/08/08 22:11:43 UTC
[calcite] 01/02: [CALCITE-5869] LEAST_RESTRICTIVE does not use inner type of MEASURE for comparisons
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 <ta...@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");