Skip to content

Commit

Permalink
DRILL-8435: Pin the version of Calcite in the master branch (#2804)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
jnturton authored May 18, 2023
1 parent 111e0e8 commit e4b9c73
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 86 deletions.
53 changes: 7 additions & 46 deletions exec/java-exec/src/main/codegen/templates/Parser.jj
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -5136,8 +5135,6 @@ SqlIntervalQualifier TimeUnit() : {
| <MINUTE> { return new SqlIntervalQualifier(TimeUnit.MINUTE, null, getPos()); }
| <HOUR> { return new SqlIntervalQualifier(TimeUnit.HOUR, null, getPos()); }
| <DAY> { return new SqlIntervalQualifier(TimeUnit.DAY, null, getPos()); }
| <DAYOFWEEK> { return new SqlIntervalQualifier(TimeUnit.DOW, null, getPos()); }
| <DAYOFYEAR> { return new SqlIntervalQualifier(TimeUnit.DOY, null, getPos()); }
| <DOW> { return new SqlIntervalQualifier(TimeUnit.DOW, null, getPos()); }
| <DOY> { return new SqlIntervalQualifier(TimeUnit.DOY, null, getPos()); }
| <ISODOW> { return new SqlIntervalQualifier(TimeUnit.ISODOW, null, getPos()); }
Expand Down Expand Up @@ -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 ---------------------------------------
(
( <CAST> { f = SqlStdOperatorTable.CAST; }
| <SAFE_CAST> { f = SqlLibraryOperators.SAFE_CAST; }
)
{ s = span(); }
<CAST> { s = span(); }
<LPAREN> AddExpression(args, ExprContext.ACCEPT_SUB_QUERY)
<AS>
(
Expand All @@ -6023,7 +6015,7 @@ SqlNode BuiltinFunctionCall() :
<INTERVAL> e = IntervalQualifier() { args.add(e); }
)
<RPAREN> {
return f.createCall(s.end(this), args);
return SqlStdOperatorTable.CAST.createCall(s.end(this), args);
}
|
<EXTRACT> { s = span(); }
Expand Down Expand Up @@ -6051,39 +6043,11 @@ SqlNode BuiltinFunctionCall() :
|
<CONVERT> { s = span(); }
<LPAREN>
(
// 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 <USING> vs. <COMMA>)
LOOKAHEAD(2)
AddExpression(args, ExprContext.ACCEPT_SUB_QUERY)
<USING> name = SimpleIdentifier() { args.add(name); }
<RPAREN> {
return SqlStdOperatorTable.CONVERT.createCall(s.end(this), args);
}
| // mssql CONVERT(type, val [,style])
(
dt = DataType() { args.add(dt); }
|
<INTERVAL> e = IntervalQualifier() { args.add(e); }
)
<COMMA>
AddExpression(args, ExprContext.ACCEPT_SUB_QUERY)
[
<COMMA>
(
style = UnsignedNumericLiteral() { args.add(style); }
|
<NULL> { args.add(SqlLiteral.createNull(getPos())); }
)
]
<RPAREN> {
return SqlLibraryOperators.MSSQL_CONVERT.createCall(s.end(this), args);
}
)
|
AddExpression(args, ExprContext.ACCEPT_SUB_QUERY)
<USING> name = SimpleIdentifier() { args.add(name); }
<RPAREN> {
return SqlStdOperatorTable.CONVERT.createCall(s.end(this), args);
} |
<TRANSLATE> { s = span(); }
<LPAREN>
AddExpression(args, ExprContext.ACCEPT_SUB_QUERY)
Expand Down Expand Up @@ -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" >
Expand Down Expand Up @@ -8117,7 +8079,6 @@ SqlPostfixOperator PostfixRowOperator() :
| < ROW_NUMBER: "ROW_NUMBER" >
| < ROWS: "ROWS" >
| < RUNNING: "RUNNING" >
| < SAFE_CAST: "SAFE_CAST" >
| < SATURDAY: "SATURDAY" >
| < SAVEPOINT: "SAVEPOINT" >
| < SCALAR: "SCALAR" >
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,7 +51,6 @@ public static void setup() throws Exception {
startCluster(builder);
}

@Ignore
@Test // DRILL-3610
public void testTimestampAddDiffLiteralTypeInference() throws Exception {
Map<String, String> dateTypes = new HashMap<>();
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
<parquet.version>1.12.3</parquet.version>
<parquet.format.version>2.9.0</parquet.format.version>
<calcite.groupId>org.apache.calcite</calcite.groupId>
<calcite.version>1.35.0-SNAPSHOT</calcite.version>
<calcite.version>1.34.0</calcite.version>
<avatica.version>1.23.0</avatica.version>
<janino.version>3.1.8</janino.version>
<sqlline.version>1.12.0</sqlline.version>
Expand Down Expand Up @@ -83,7 +83,7 @@
<hbase.version>2.4.9</hbase.version>
<fmpp.version>1.0</fmpp.version>
<freemarker.version>2.3.30</freemarker.version>
<javassist.version>3.28.0-GA</javassist.version>
<javassist.version>3.29.2-GA</javassist.version>
<msgpack.version>0.6.6</msgpack.version>
<reflections.version>0.9.10</reflections.version>
<avro.version>1.11.1</avro.version>
Expand Down

0 comments on commit e4b9c73

Please sign in to comment.