From 211330aa7965b78fc9aba2dd812453f94e3015b2 Mon Sep 17 00:00:00 2001 From: Will Noble Date: Tue, 4 Jan 2022 16:28:28 -0800 Subject: [PATCH] Apply all feedback and bring up to speed with the Looker fork --- .../calcite/rel/rel2sql/SqlImplementor.java | 5 ++++ .../calcite/sql/type/SqlTypeTransforms.java | 2 ++ .../apache/calcite/sql/type/SqlTypeUtil.java | 2 +- .../calcite/sql/type/SqlTypeFactoryTest.java | 2 +- .../calcite/sql/type/SqlTypeFixture.java | 2 ++ .../calcite/sql/type/SqlTypeUtilTest.java | 28 +++++++++++++------ site/_docs/reference.md | 3 +- 7 files changed, 32 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java index 24549be3f4e..251febb8a4d 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java @@ -786,6 +786,11 @@ private SqlNode callToSql(@Nullable RexProgram program, RexCall call0, SqlNode fieldOperand = field(ordinal); return SqlStdOperatorTable.CURSOR.createCall(SqlParserPos.ZERO, fieldOperand); } + // Ideally the UNKNOWN type would never exist in a fully-formed, validated rel node, but + // it can be useful in certain situations where determining the type of an expression is + // infeasible, such as inserting arbitrary user-provided SQL snippets into an otherwise + // manually-constructed (as opposed to parsed) rel node. + // In such a context, assume that casting anything to UNKNOWN is a no-op. if (ignoreCast || call.getType().getSqlTypeName() == SqlTypeName.UNKNOWN) { assert nodeList.size() == 1; return nodeList.get(0); diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java index 92165f5bd2b..cc4e63794bf 100644 --- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java +++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java @@ -145,6 +145,8 @@ private SqlTypeName toVar(RelDataType type) { return SqlTypeName.ANY; case NULL: return SqlTypeName.NULL; + case UNKNOWN: + return SqlTypeName.UNKNOWN; default: throw Util.unexpected(sqlTypeName); } 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 d51a07fbbb7..0fcff9c339f 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 @@ -1036,7 +1036,7 @@ public static SqlDataTypeSpec convertTypeToSpec(RelDataType type, assert typeName != null; final SqlTypeNameSpec typeNameSpec; - if (isAtomic(type) || isNull(type)) { + if (isAtomic(type) || isNull(type) || type.getSqlTypeName() == SqlTypeName.UNKNOWN) { int precision = typeName.allowsPrec() ? type.getPrecision() : -1; // fix up the precision. if (maxPrecision > 0 && precision > maxPrecision) { diff --git a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java index 8a835f0e462..fa07a3167ba 100644 --- a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java +++ b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java @@ -289,7 +289,7 @@ private void checkCreateSqlTypeWithPrecision( } /** Test that the {code UNKNOWN} type does not does not change class when nullified. */ - @Test void testUnknownCreateWithNullabiliityTypeConsistency() { + @Test void testUnknownCreateWithNullabilityTypeConsistency() { SqlTypeFixture f = new SqlTypeFixture(); RelDataType unknownType = f.typeFactory.createUnknownType(); diff --git a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFixture.java b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFixture.java index 68270249999..05fa8fed58a 100644 --- a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFixture.java +++ b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFixture.java @@ -44,6 +44,8 @@ class SqlTypeFixture { typeFactory.createSqlType(SqlTypeName.VARCHAR), true); final RelDataType sqlNull = typeFactory.createTypeWithNullability( typeFactory.createSqlType(SqlTypeName.NULL), false); + final RelDataType sqlUnknown = typeFactory.createTypeWithNullability( + typeFactory.createSqlType(SqlTypeName.UNKNOWN), false); final RelDataType sqlAny = typeFactory.createTypeWithNullability( typeFactory.createSqlType(SqlTypeName.ANY), false); final RelDataType sqlFloat = typeFactory.createTypeWithNullability( diff --git a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java index 1da3dfeef53..0e93d110949 100644 --- a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java +++ b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java @@ -38,7 +38,6 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test of {@link org.apache.calcite.sql.type.SqlTypeUtil}. @@ -156,6 +155,10 @@ class SqlTypeUtilTest { (SqlBasicTypeNameSpec) convertTypeToSpec(f.sqlNull).getTypeNameSpec(); assertThat(nullSpec.getTypeName().getSimple(), is("NULL")); + SqlBasicTypeNameSpec unknownSpec = + (SqlBasicTypeNameSpec) convertTypeToSpec(f.sqlUnknown).getTypeNameSpec(); + assertThat(unknownSpec.getTypeName().getSimple(), is("UNKNOWN")); + SqlBasicTypeNameSpec basicSpec = (SqlBasicTypeNameSpec) convertTypeToSpec(f.sqlBigInt).getTypeNameSpec(); assertThat(basicSpec.getTypeName().getSimple(), is("BIGINT")); @@ -242,14 +245,21 @@ private void compareTypesIgnoringNullability( } BasicSqlType nullableFromType = fromType.createWithNullability(!fromType.isNullable); - assertTrue(SqlTypeUtil.canCastFrom(unknownType, fromType, false)); - assertTrue(SqlTypeUtil.canCastFrom(unknownType, fromType, true)); - assertTrue(SqlTypeUtil.canCastFrom(unknownType, nullableFromType, false)); - assertTrue(SqlTypeUtil.canCastFrom(unknownType, nullableFromType, true)); - assertTrue(SqlTypeUtil.canCastFrom(nullableUnknownType, fromType, false)); - assertTrue(SqlTypeUtil.canCastFrom(nullableUnknownType, fromType, true)); - assertTrue(SqlTypeUtil.canCastFrom(nullableUnknownType, nullableFromType, false)); - assertTrue(SqlTypeUtil.canCastFrom(nullableUnknownType, nullableFromType, true)); + assertCanCast(unknownType, fromType); + assertCanCast(unknownType, nullableFromType); + assertCanCast(nullableUnknownType, fromType); + assertCanCast(nullableUnknownType, nullableFromType); } } + + private static void assertCanCast(RelDataType toType, RelDataType fromType) { + assertThat( + String.format( + "Expected to be able to cast from %s to %s without coercion.", fromType, toType), + SqlTypeUtil.canCastFrom(toType, fromType, /* coerce= */ false), is(true)); + assertThat( + String.format( + "Expected to be able to cast from %s to %s with coercion.", fromType, toType), + SqlTypeUtil.canCastFrom(toType, fromType, /* coerce= */ true), is(true)); + } } diff --git a/site/_docs/reference.md b/site/_docs/reference.md index 2afd767c1aa..c8a225b2d48 100644 --- a/site/_docs/reference.md +++ b/site/_docs/reference.md @@ -1150,7 +1150,8 @@ Note: | Type | Description | Example literals |:-------- |:---------------------------|:--------------- -| ANY | A value of an unknown type | +| ANY | A value of an unknown type, subject to explicit casting to maintain type consistency in expressions | +| UNKNOWN | A value of an unknown type, which Calcite assume's will simply work "as is" without any explicit casting | | ROW | Row with 1 or more columns | Example: Row(f0 int null, f1 varchar) | MAP | Collection of keys mapped to values | | MULTISET | Unordered collection that may contain duplicates | Example: int multiset