From e4b9c73007374bcf622035e48a5e19a8f7654ccc Mon Sep 17 00:00:00 2001 From: James Turton <9107319+jnturton@users.noreply.github.com> Date: Thu, 18 May 2023 06:55:12 +0200 Subject: [PATCH] DRILL-8435: Pin the version of Calcite in the master branch (#2804) * Upgrade javassist -> 3.29.2-GA. * Pin Calcite to 1.34.0 and reenable testTimestampAddDiffLiteralTypeInference. * Adjust casting costs so that VARCHAR -> BIGINT is cheaper than VARCHAR -> FLOAT8. * Undo DRILL-8424 (#daaa40325), RexBuilder changes for SAFE_CAST. --- .../src/main/codegen/templates/Parser.jj | 53 +++---------------- .../sql/conversion/DrillRexBuilder.java | 8 +-- .../exec/resolver/ResolverTypePrecedence.java | 21 ++++---- .../org/apache/drill/TestImplicitCasting.java | 8 ++- .../exec/fn/impl/TestNewDateFunctions.java | 20 ------- .../impl/TestTimestampAddDiffFunctions.java | 2 - pom.xml | 4 +- 7 files changed, 30 insertions(+), 86 deletions(-) diff --git a/exec/java-exec/src/main/codegen/templates/Parser.jj b/exec/java-exec/src/main/codegen/templates/Parser.jj index 472ac65869b..ccd2091f7ec 100644 --- a/exec/java-exec/src/main/codegen/templates/Parser.jj +++ b/exec/java-exec/src/main/codegen/templates/Parser.jj @@ -63,7 +63,6 @@ import org.apache.calcite.sql.SqlDynamicParam; import org.apache.calcite.sql.SqlExplain; import org.apache.calcite.sql.SqlExplainFormat; import org.apache.calcite.sql.SqlExplainLevel; -import org.apache.calcite.sql.SqlFunction; import org.apache.calcite.sql.SqlFunctionCategory; import org.apache.calcite.sql.SqlHint; import org.apache.calcite.sql.SqlIdentifier; @@ -5136,8 +5135,6 @@ SqlIntervalQualifier TimeUnit() : { | { return new SqlIntervalQualifier(TimeUnit.MINUTE, null, getPos()); } | { return new SqlIntervalQualifier(TimeUnit.HOUR, null, getPos()); } | { return new SqlIntervalQualifier(TimeUnit.DAY, null, getPos()); } -| { return new SqlIntervalQualifier(TimeUnit.DOW, null, getPos()); } -| { return new SqlIntervalQualifier(TimeUnit.DOY, null, getPos()); } | { return new SqlIntervalQualifier(TimeUnit.DOW, null, getPos()); } | { return new SqlIntervalQualifier(TimeUnit.DOY, null, getPos()); } | { return new SqlIntervalQualifier(TimeUnit.ISODOW, null, getPos()); } @@ -6005,16 +6002,11 @@ SqlNode BuiltinFunctionCall() : SqlDataTypeSpec dt; final SqlIntervalQualifier unit; final SqlNode node; - final SqlLiteral style; // mssql convert 'style' operand - final SqlFunction f; } { //~ FUNCTIONS WITH SPECIAL SYNTAX --------------------------------------- ( - ( { f = SqlStdOperatorTable.CAST; } - | { f = SqlLibraryOperators.SAFE_CAST; } - ) - { s = span(); } + { s = span(); } AddExpression(args, ExprContext.ACCEPT_SUB_QUERY) ( @@ -6023,7 +6015,7 @@ SqlNode BuiltinFunctionCall() : e = IntervalQualifier() { args.add(e); } ) { - return f.createCall(s.end(this), args); + return SqlStdOperatorTable.CAST.createCall(s.end(this), args); } | { s = span(); } @@ -6051,39 +6043,11 @@ SqlNode BuiltinFunctionCall() : | { s = span(); } - ( - // CONVERT in the form of CONVERT(x USING y) - - // "AddExpression" matches INTERVAL, - // which can also be 1st token in args of MSSQL CONVERT - // So lookahead another token (to match vs. ) - LOOKAHEAD(2) - AddExpression(args, ExprContext.ACCEPT_SUB_QUERY) - name = SimpleIdentifier() { args.add(name); } - { - return SqlStdOperatorTable.CONVERT.createCall(s.end(this), args); - } - | // mssql CONVERT(type, val [,style]) - ( - dt = DataType() { args.add(dt); } - | - e = IntervalQualifier() { args.add(e); } - ) - - AddExpression(args, ExprContext.ACCEPT_SUB_QUERY) - [ - - ( - style = UnsignedNumericLiteral() { args.add(style); } - | - { args.add(SqlLiteral.createNull(getPos())); } - ) - ] - { - return SqlLibraryOperators.MSSQL_CONVERT.createCall(s.end(this), args); - } - ) - | + AddExpression(args, ExprContext.ACCEPT_SUB_QUERY) + name = SimpleIdentifier() { args.add(name); } + { + return SqlStdOperatorTable.CONVERT.createCall(s.end(this), args); + } | { s = span(); } AddExpression(args, ExprContext.ACCEPT_SUB_QUERY) @@ -7766,8 +7730,6 @@ SqlPostfixOperator PostfixRowOperator() : | < DATETIME_INTERVAL_CODE: "DATETIME_INTERVAL_CODE" > | < DATETIME_INTERVAL_PRECISION: "DATETIME_INTERVAL_PRECISION" > | < DAY: "DAY" > -| < DAYOFWEEK: "DAYOFWEEK" > -| < DAYOFYEAR: "DAYOFYEAR" > | < DAYS: "DAYS" > | < DEALLOCATE: "DEALLOCATE" > | < DEC: "DEC" > @@ -8117,7 +8079,6 @@ SqlPostfixOperator PostfixRowOperator() : | < ROW_NUMBER: "ROW_NUMBER" > | < ROWS: "ROWS" > | < RUNNING: "RUNNING" > -| < SAFE_CAST: "SAFE_CAST" > | < SATURDAY: "SATURDAY" > | < SAVEPOINT: "SAVEPOINT" > | < SCALAR: "SCALAR" > diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillRexBuilder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillRexBuilder.java index 7c232ec52f0..299859c8427 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillRexBuilder.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillRexBuilder.java @@ -65,9 +65,9 @@ public RexNode ensureType( * @return Call to CAST operator */ @Override - public RexNode makeCast(RelDataType type, RexNode exp, boolean matchNullability, boolean safe) { + public RexNode makeCast(RelDataType type, RexNode exp, boolean matchNullability) { if (matchNullability) { - return makeAbstractCast(type, exp, safe); + return makeAbstractCast(type, exp); } // for the case when BigDecimal literal has a scale or precision // that differs from the value from specified RelDataType, cast cannot be removed @@ -81,11 +81,11 @@ public RexNode makeCast(RelDataType type, RexNode exp, boolean matchNullability, BigDecimal bigDecimal = (BigDecimal) value; DecimalUtility.checkValueOverflow(bigDecimal, precision, scale); if (bigDecimal.precision() != precision || bigDecimal.scale() != scale) { - return makeAbstractCast(type, exp, safe); + return makeAbstractCast(type, exp); } } } - return super.makeCast(type, exp, false, safe); + return super.makeCast(type, exp, false); } private void validatePrecisionAndScale(int precision, int scale) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java index abc181402f5..bb7c256c6a3 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java @@ -118,7 +118,7 @@ public class ResolverTypePrecedence { .putEdgeValue(MinorType.INTERVALYEAR, MinorType.INTERVAL, 10f) // interval conversions // prefer the cast in the opposite direction - .putEdgeValue(MinorType.INTERVAL, MinorType.VARCHAR, 100f) + .putEdgeValue(MinorType.INTERVAL, MinorType.VARCHAR, 1000f) // dict widening .putEdgeValue(MinorType.DICT, MinorType.MAP, 10f) @@ -138,15 +138,16 @@ public class ResolverTypePrecedence { .putEdgeValue(MinorType.FIXEDCHAR, MinorType.VARCHAR, 10f) .putEdgeValue(MinorType.FIXEDBINARY, MinorType.VARBINARY, 10f) // char and binary conversions - .putEdgeValue(MinorType.VARCHAR, MinorType.INT, 10f) - .putEdgeValue(MinorType.VARCHAR, MinorType.FLOAT8, 20f) - .putEdgeValue(MinorType.VARCHAR, MinorType.FLOAT4, 21f) - .putEdgeValue(MinorType.VARCHAR, MinorType.VARDECIMAL, 30f) - .putEdgeValue(MinorType.VARCHAR, MinorType.TIMESTAMP, 40f) - .putEdgeValue(MinorType.VARCHAR, MinorType.INTERVALDAY, 50f) - .putEdgeValue(MinorType.VARCHAR, MinorType.BIT, 60f) - .putEdgeValue(MinorType.VARCHAR, MinorType.VARBINARY, 70f) - .putEdgeValue(MinorType.VARBINARY, MinorType.VARCHAR, 80f) + .putEdgeValue(MinorType.VARCHAR, MinorType.BIGINT, 100f) + .putEdgeValue(MinorType.VARCHAR, MinorType.INT, 101f) + .putEdgeValue(MinorType.VARCHAR, MinorType.FLOAT8, 102f) + .putEdgeValue(MinorType.VARCHAR, MinorType.FLOAT4, 103f) + .putEdgeValue(MinorType.VARCHAR, MinorType.VARDECIMAL, 104f) + .putEdgeValue(MinorType.VARCHAR, MinorType.TIMESTAMP, 105f) + .putEdgeValue(MinorType.VARCHAR, MinorType.INTERVALDAY, 106f) + .putEdgeValue(MinorType.VARCHAR, MinorType.BIT, 107f) + .putEdgeValue(MinorType.VARCHAR, MinorType.VARBINARY, 108f) + .putEdgeValue(MinorType.VARBINARY, MinorType.VARCHAR, 109f) // union type sink vertex .putEdgeValue(MinorType.LIST, MinorType.UNION, 10f) diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java b/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java index 54a0fe738b9..f8a43b30dea 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestImplicitCasting.java @@ -99,9 +99,9 @@ public void testCastingCosts() { ResolverTypePrecedence.computeCost(TypeProtos.MinorType.MAP, TypeProtos.MinorType.FLOAT8), 0f ); - // VARCHAR -> INT -> BIGINT + // VARCHAR -> BIGINT assertEquals( - 10f+10f, + 100f, ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.BIGINT), 0f ); @@ -155,6 +155,10 @@ public void testCastingPreferences() { ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.BIGINT), lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.TIMESTAMP)) ); + assertThat( + ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.BIGINT), + lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.FLOAT8)) + ); assertThat( ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.FLOAT8), lessThan(ResolverTypePrecedence.computeCost(TypeProtos.MinorType.VARCHAR, TypeProtos.MinorType.FLOAT4)) diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewDateFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewDateFunctions.java index 6d514300525..626b6d18ca7 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewDateFunctions.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewDateFunctions.java @@ -152,26 +152,6 @@ public void testExtractQuarter() throws Exception { .go(); } - @Test - public void testDayOfYear() throws Exception { - testBuilder() - .sqlQuery("SELECT EXTRACT(DOY FROM date '2023-04-17') AS col1, EXTRACT(DAYOFYEAR FROM date '2023-04-17') as col2") - .unOrdered() - .baselineColumns("col1", "col2") - .baselineValues(107L, 107L) - .go(); - } - - @Test - public void testDayOfWeek() throws Exception { - testBuilder() - .sqlQuery("SELECT EXTRACT(DOW FROM date '2023-04-17') AS col1, EXTRACT(DAYOFWEEK FROM date '2023-04-17') as col2") - .unOrdered() - .baselineColumns("col1", "col2") - .baselineValues(1L, 1L) - .go(); - } - @Test public void testLocalTimestamp() throws Exception { testBuilder() diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestTimestampAddDiffFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestTimestampAddDiffFunctions.java index 807214b82b2..977fd4b5c5f 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestTimestampAddDiffFunctions.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestTimestampAddDiffFunctions.java @@ -21,7 +21,6 @@ import org.apache.drill.test.ClusterFixtureBuilder; import org.apache.drill.test.ClusterTest; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import java.time.LocalDateTime; @@ -52,7 +51,6 @@ public static void setup() throws Exception { startCluster(builder); } - @Ignore @Test // DRILL-3610 public void testTimestampAddDiffLiteralTypeInference() throws Exception { Map dateTypes = new HashMap<>(); diff --git a/pom.xml b/pom.xml index 6684b44c8ad..59ebc44a6bf 100644 --- a/pom.xml +++ b/pom.xml @@ -54,7 +54,7 @@ 1.12.3 2.9.0 org.apache.calcite - 1.35.0-SNAPSHOT + 1.34.0 1.23.0 3.1.8 1.12.0 @@ -83,7 +83,7 @@ 2.4.9 1.0 2.3.30 - 3.28.0-GA + 3.29.2-GA 0.6.6 0.9.10 1.11.1