diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java index 1ea24311e34..ff93bce0b4c 100644 --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java @@ -513,7 +513,6 @@ import static org.apache.calcite.sql.fun.SqlStdOperatorTable.RAND_INTEGER; import static org.apache.calcite.sql.fun.SqlStdOperatorTable.RANK; import static org.apache.calcite.sql.fun.SqlStdOperatorTable.REGR_COUNT; -import static org.apache.calcite.sql.fun.SqlStdOperatorTable.REINTERPRET; import static org.apache.calcite.sql.fun.SqlStdOperatorTable.REPLACE; import static org.apache.calcite.sql.fun.SqlStdOperatorTable.RIGHTSHIFT; import static org.apache.calcite.sql.fun.SqlStdOperatorTable.ROUND; @@ -1170,7 +1169,6 @@ void populate2() { define(SAFE_CAST, new CastImplementor()); define(TRY_CAST, new CastImplementor()); - define(REINTERPRET, new ReinterpretImplementor()); define(CONVERT, new ConvertImplementor()); define(TRANSLATE, new TranslateImplementor()); @@ -3796,19 +3794,6 @@ private static RelDataType nullifyType(JavaTypeFactory typeFactory, } } - /** Implementor for the {@code REINTERPRET} internal SQL operator. */ - private static class ReinterpretImplementor extends AbstractRexCallImplementor { - ReinterpretImplementor() { - super("reinterpret", NullPolicy.STRICT, false); - } - - @Override Expression implementSafe(final RexToLixTranslator translator, - final RexCall call, final List argValueList) { - assert call.getOperands().size() == 1; - return argValueList.get(0); - } - } - /** Implementor for sort_array. */ private static class SortArrayImplementor extends AbstractRexCallImplementor { SortArrayImplementor() { diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java index 9bad3dc3f5f..cf9cbde5e37 100644 --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java @@ -720,6 +720,20 @@ private Expression getConvertExpression( case INTEGER: case TINYINT: case SMALLINT: { + if (sourceType.getFamily() == SqlTypeFamily.INTERVAL_DAY_TIME + || sourceType.getFamily() == SqlTypeFamily.INTERVAL_YEAR_MONTH) { + // An interval is represented by its count of base units (milliseconds + // or months); the cast yields the count of the interval's end unit, + // truncated towards zero. + final BigDecimal multiplier = + sourceType.getSqlTypeName().getEndUnit().multiplier; + final Expression ticks = EnumUtils.convert(operand, long.class); + final Expression scaled = multiplier.equals(BigDecimal.ONE) + ? ticks + : Expressions.divide(ticks, + Expressions.constant(multiplier.longValueExact())); + return EnumUtils.convert(scaled, typeFactory.getJavaClass(targetType)); + } if (SqlTypeName.NUMERIC_TYPES.contains(sourceType.getSqlTypeName())) { Type javaClass = typeFactory.getJavaClass(targetType); Primitive primitive = Primitive.of(javaClass); @@ -1393,6 +1407,10 @@ private static Expression scaleValue( // multiplyDivide cannot handle DECIMALs, but for DECIMAL // target types the result is already scaled. && targetType.getSqlTypeName() != SqlTypeName.DECIMAL + // Integer targets divide before narrowing, in getConvertExpression; + // dividing here, after the narrowing, would overflow for tick counts + // wider than the target type. + && !SqlTypeName.INT_TYPES.contains(targetType.getSqlTypeName()) && (sourceFamily == SqlTypeFamily.INTERVAL_YEAR_MONTH || sourceFamily == SqlTypeFamily.INTERVAL_DAY_TIME)) { // Scale to the given field. diff --git a/core/src/main/java/org/apache/calcite/plan/Strong.java b/core/src/main/java/org/apache/calcite/plan/Strong.java index ee349b837c4..b6aa9678ed8 100644 --- a/core/src/main/java/org/apache/calcite/plan/Strong.java +++ b/core/src/main/java/org/apache/calcite/plan/Strong.java @@ -358,7 +358,7 @@ private static Map createPolicyMap() { map.put(SqlKind.DIVIDE, Policy.ANY); map.put(SqlKind.CAST, Policy.ANY); - map.put(SqlKind.REINTERPRET, Policy.ANY); + map.put(SqlKind.REINTERPRET, Policy.ANY); // deprecated, kept until removed map.put(SqlKind.TRIM, Policy.ANY); map.put(SqlKind.LTRIM, Policy.ANY); map.put(SqlKind.RTRIM, Policy.ANY); diff --git a/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java b/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java index 104e34bfaeb..1327c1b49a6 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java @@ -221,7 +221,10 @@ private CoreRules() {} /** Rule that reduces operations on the DECIMAL type, such as casts or * arithmetic, into operations involving more primitive types such as BIGINT - * and DOUBLE. */ + * and DOUBLE. + * + * @deprecated See {@link ReduceDecimalsRule}. */ + @Deprecated // to be removed before 2.0 public static final ReduceDecimalsRule CALC_REDUCE_DECIMALS = ReduceDecimalsRule.Config.DEFAULT.toRule(); diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java index 06bf0f67eec..8918bf55f9e 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java @@ -73,8 +73,19 @@ * would like to push down decimal operations to an external database. * * @see CoreRules#CALC_REDUCE_DECIMALS + * + * @deprecated The rule rewrites decimal values as their unscaled BIGINT + * representation, connected by REINTERPRET operators. This assumes a physical + * representation of DECIMAL values that only an adapter or calling convention + * knows, so the rewritten plan is no longer a logical plan. This rule is opt-in + * for engines that represent DECIMAL values as scaled integers. The + * REINTERPRET operator is deprecated. */ +@Deprecated // to be removed before 2.0 @Value.Enclosing +// Immutables copies this suppression into the generated class, which +// references this deprecated class +@SuppressWarnings("deprecation") public class ReduceDecimalsRule extends RelRule implements TransformationRule { diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java index 95030b5b905..667de88227f 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java +++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java @@ -874,9 +874,6 @@ public RexNode makeCast( } return literal2; } - } else if (SqlTypeUtil.isExactNumeric(type) - && SqlTypeUtil.isInterval(exp.getType())) { - return makeCastIntervalToExact(pos, type, exp); } else if (sqlType == SqlTypeName.BOOLEAN && SqlTypeUtil.isExactNumeric(exp.getType())) { return makeCastExactToBoolean(type, exp); @@ -1125,16 +1122,6 @@ private RexNode makeCastBooleanToExact(RelDataType toType, RexNode exp) { casted, makeNullLiteral(toType))); } - private RexNode makeCastIntervalToExact(SqlParserPos pos, RelDataType toType, RexNode exp) { - final TimeUnit endUnit = exp.getType().getSqlTypeName().getEndUnit(); - final TimeUnit baseUnit = baseUnit(exp.getType().getSqlTypeName()); - final BigDecimal multiplier = baseUnit.multiplier; - final BigDecimal divider = endUnit.multiplier; - RexNode value = - multiplyDivide(pos, decodeIntervalOrDecimal(pos, exp), multiplier, divider); - return ensureType(pos, toType, value, false); - } - public RexNode multiplyDivide(RexNode e, BigDecimal multiplier, BigDecimal divider) { return multiplyDivide(SqlParserPos.ZERO, e, multiplier, divider); @@ -1177,7 +1164,10 @@ public RexNode multiplyDivide(SqlParserPos pos, RexNode e, BigDecimal multiplier * arithmetic, but is often required for rounding and * explicit casts. * @return the integer reinterpreted as an opaque decimal type + * + * @deprecated The REINTERPRET operator is deprecated */ + @Deprecated // to be removed before 2.0 public RexNode encodeIntervalOrDecimal( RexNode value, RelDataType type, @@ -1185,6 +1175,11 @@ public RexNode encodeIntervalOrDecimal( return encodeIntervalOrDecimal(SqlParserPos.ZERO, value, type, checkOverflow); } + /** Encodes an interval or decimal, with an explicit parser position. + * + * @deprecated The REINTERPRET operator is deprecated + */ + @Deprecated // to be removed before 2.0 public RexNode encodeIntervalOrDecimal( SqlParserPos pos, RexNode value, @@ -1201,11 +1196,19 @@ public RexNode encodeIntervalOrDecimal( * * @param node the interval or decimal value as an opaque type * @return an integer representation of the decimal value + * + * @deprecated The REINTERPRET operator is deprecated */ + @Deprecated // to be removed before 2.0 public RexNode decodeIntervalOrDecimal(RexNode node) { return decodeIntervalOrDecimal(SqlParserPos.ZERO, node); } + /** Decodes an interval or decimal, with an explicit parser position. + * + * @deprecated The REINTERPRET operator is deprecated + */ + @Deprecated // to be removed before 2.0 public RexNode decodeIntervalOrDecimal(SqlParserPos pos, RexNode node) { assert SqlTypeUtil.isDecimal(node.getType()) || SqlTypeUtil.isInterval(node.getType()); @@ -1289,7 +1292,13 @@ public RexNode makeAbstractCast( * @param exp expression to be casted * @param checkOverflow whether an overflow check is required * @return a RexCall with two operands and a special return type + * + * @deprecated The REINTERPRET operator is deprecated; its semantics depend + * on the physical representation of values, which only an adapter or + * calling convention knows. Use {@link #makeCast(RelDataType, RexNode)} + * instead */ + @Deprecated // to be removed before 2.0 public RexNode makeReinterpretCast( RelDataType type, RexNode exp, @@ -1305,7 +1314,13 @@ public RexNode makeReinterpretCast( * @param exp expression to be cast * @param checkOverflow whether an overflow check is required * @return a RexCall with two operands and a special return type + * + * @deprecated The REINTERPRET operator is deprecated; its semantics depend + * on the physical representation of values, which only an adapter or + * calling convention knows. Use + * {@link #makeCast(SqlParserPos, RelDataType, RexNode)} instead */ + @Deprecated // to be removed before 2.0 public RexNode makeReinterpretCast( SqlParserPos pos, RelDataType type, diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java b/core/src/main/java/org/apache/calcite/rex/RexUtil.java index b592093a5ae..e24a75d3c9f 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java +++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java @@ -1018,7 +1018,12 @@ public static boolean containsFieldAccess(RexNode node) { * @param expr expression possibly in need of expansion * @param recurse whether to check nested calls * @return whether the expression requires expansion + * + * @deprecated Used only by + * {@link org.apache.calcite.rel.rules.ReduceDecimalsRule}, which is + * deprecated */ + @Deprecated // to be removed before 2.0 public static boolean requiresDecimalExpansion( RexNode expr, boolean recurse) { @@ -1069,7 +1074,12 @@ public static boolean requiresDecimalExpansion( /** * Determines whether any operand of a set requires decimal expansion. + * + * @deprecated Used only by + * {@link org.apache.calcite.rel.rules.ReduceDecimalsRule}, which is + * deprecated */ + @Deprecated // to be removed before 2.0 public static boolean requiresDecimalExpansion( List operands, boolean recurse) { @@ -1087,7 +1097,12 @@ public static boolean requiresDecimalExpansion( /** * Returns whether a {@link RexProgram} contains expressions which require * decimal expansion. + * + * @deprecated Used only by + * {@link org.apache.calcite.rel.rules.ReduceDecimalsRule}, which is + * deprecated */ + @Deprecated // to be removed before 2.0 public static boolean requiresDecimalExpansion( RexProgram program, boolean recurse) { @@ -1100,6 +1115,11 @@ public static boolean requiresDecimalExpansion( return false; } + /** Returns whether a REINTERPRET call performs an overflow check. + * + * @deprecated The REINTERPRET operator is deprecated + */ + @Deprecated // to be removed before 2.0 public static boolean canReinterpretOverflow(RexCall call) { assert call.isA(SqlKind.REINTERPRET) : "call is not a reinterpret"; return call.operands.size() > 1; diff --git a/core/src/main/java/org/apache/calcite/sql/SqlKind.java b/core/src/main/java/org/apache/calcite/sql/SqlKind.java index a70c71df761..92bbfe4c599 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlKind.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlKind.java @@ -1005,6 +1005,11 @@ public enum SqlKind { /** * The internal REINTERPRET operator (meaning a reinterpret cast). * An internal operator that does not appear in SQL syntax. + * + *

Do not use. The + * {@link org.apache.calcite.sql.fun.SqlStdOperatorTable#REINTERPRET} + * operator is deprecated and will be removed, together with this value; + * use {@link #CAST} instead. */ REINTERPRET, diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java index 20a5e690ffe..be9f23abe67 100644 --- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java +++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java @@ -1786,7 +1786,14 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable { * it accepts one operand and stores the target type as the return type. It * performs an overflow check if it has any second operand, whether * true or not. + * + * @deprecated The semantics of REINTERPRET depend on the physical + * representation of values, which only an adapter or calling convention + * knows; a logical plan must not contain this operator. + * The enumerable convention does not implement it. + * Use {@link #CAST} instead. */ + @Deprecated // to be removed before 2.0 public static final SqlSpecialOperator REINTERPRET = new SqlSpecialOperator("Reinterpret", SqlKind.REINTERPRET) { @Override public SqlOperandCountRange getOperandCountRange() { diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java index 3ca7ae44f85..5c524eee17b 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java @@ -6572,23 +6572,12 @@ private class HistogramShuttle extends RexShuttle { if (histogramOp != null) { final RelDataType histogramType = computeHistogramType(type); - // For DECIMAL, since it's already represented as a bigint we - // want to do a reinterpretCast instead of a cast to avoid - // losing any precision. - boolean reinterpretCast = - type.getSqlTypeName() == SqlTypeName.DECIMAL; - // Replace original expression with CAST of not one // of the supported types if (histogramType != type) { exprs = new ArrayList<>(exprs); - exprs.set( - 0, - reinterpretCast - ? rexBuilder.makeReinterpretCast( - call.getParserPosition(), histogramType, exprs.get(0), - rexBuilder.makeLiteral(false)) - : rexBuilder.makeCast(call.getParserPosition(), histogramType, exprs.get(0))); + exprs.set(0, + rexBuilder.makeCast(call.getParserPosition(), histogramType, exprs.get(0))); } RexNode over = @@ -6615,16 +6604,8 @@ private class HistogramShuttle extends RexShuttle { // If needed, post Cast result back to original // type. if (histogramType != type) { - if (reinterpretCast) { - histogramCall = - rexBuilder.makeReinterpretCast(call.getParserPosition(), - type, - histogramCall, - rexBuilder.makeLiteral(false)); - } else { - histogramCall = - rexBuilder.makeCast(call.getParserPosition(), type, histogramCall); - } + histogramCall = + rexBuilder.makeCast(call.getParserPosition(), type, histogramCall); } return histogramCall; diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index bad13247c3e..56a2315ff6a 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -10752,6 +10752,7 @@ public interface Config extends RelRule.Config { * Test case for * [CALCITE-3319] * AssertionError for ReduceDecimalsRule. */ + @SuppressWarnings("deprecation") // tests the deprecated ReduceDecimalsRule @Test void testReduceDecimal() { final String sql = "select ename from emp where sal > cast (100.0 as decimal(4, 1))"; sql(sql) diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java index 2f4ba891942..8592c1fa714 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -6335,6 +6335,40 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) { + " supported")); } + /** Test case for + * [CALCITE-7451] + * REINTERPRET should not be used in logical plans. + * + *

Casting an interval to an exact numeric type, which the TIMESTAMPDIFF + * family of functions relies on, remains a CAST call in the logical plan; + * it used to be rewritten in terms of the deprecated REINTERPRET + * operator. */ + @Test void testCastIntervalToNumericNoReinterpret() { + final String sql = "select cast(x as integer) as i,\n" + + " cast(x as decimal(6, 1)) as d,\n" + + " timestampdiff(minute, ts, ts) as m\n" + + "from (values (interval '90' minute,\n" + + " timestamp '2020-01-01 00:00:00')) as t(x, ts)"; + final String plan = RelOptUtil.toString(sql(sql).toRel()); + assertThat(plan, not(containsString("Reinterpret"))); + sql(sql).ok(); + } + + /** Test case for + * [CALCITE-7451] + * REINTERPRET should not be used in logical plans. + * + *

The deprecated REINTERPRET operator must not appear in a logical plan; + * FLOOR and CEIL of an interval literal used to produce one. */ + @Test void testFloorCeilOfIntervalLiteral() { + final String sql = "select floor(interval '3:4:5' hour to second) as f,\n" + + " ceil(interval '3:4:5' hour to second) as c\n" + + "from emp"; + final String plan = RelOptUtil.toString(sql(sql).toRel()); + assertThat(plan, not(containsString("Reinterpret"))); + sql(sql).ok(); + } + /** Test case of * [CALCITE-5406] * Support the SELECT DISTINCT ON statement for PostgreSQL dialect. */ diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml index a8da739aa94..013943904b4 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -802,6 +802,21 @@ GROUP BY GROUPING SETS ( LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}]]) LogicalProject(EMPNO=[$0], EXPR$1=[CASE(SEARCH($1, Sarg['Eric':VARCHAR(20), 'Fred':VARCHAR(20)]:VARCHAR(20)), 'Manager', 'Other ')]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + + + + + + + + @@ -2636,6 +2651,19 @@ from (values (interval '3:4:5' hour to second)) as t(x)]]> =($0, 0), $0, -($0, 3599999)), 3600000), 3600000)], C=[*(/INT(CASE(>=($0, 0), +($0, 3599999), $0), 3600000), 3600000)]) LogicalValues(tuples=[[{ 11045000 }]]) +]]> + + + + + + + + =(11045000:INTERVAL HOUR TO SECOND, 0), 11045000:INTERVAL HOUR TO SECOND, -(11045000:INTERVAL HOUR TO SECOND, 3599999:INTERVAL HOUR TO SECOND)), 3600000), 3600000)], C=[*(/INT(CASE(>=(11045000:INTERVAL HOUR TO SECOND, 0), +(11045000:INTERVAL HOUR TO SECOND, 3599999:INTERVAL HOUR TO SECOND), 11045000:INTERVAL HOUR TO SECOND), 3600000), 3600000)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> diff --git a/core/src/test/resources/sql/operator.iq b/core/src/test/resources/sql/operator.iq index 45bee30f0c9..5b0a082595d 100644 --- a/core/src/test/resources/sql/operator.iq +++ b/core/src/test/resources/sql/operator.iq @@ -883,4 +883,18 @@ select floor(interval '2' hour + interval '90' minute) = interval '3' hour as fa !ok +# [CALCITE-7451] REINTERPRET should not be used in logical plans +# CAST of a non-literal interval to an exact numeric type. A DECIMAL target +# preserves the fractional part; an integer target truncates towards zero. +select cast(x as decimal(2,1)) as d, cast(x as integer) as i +from (values (interval '1.29' second(1,2))) as t(x); ++-----+---+ +| D | I | ++-----+---+ +| 1.2 | 1 | ++-----+---+ +(1 row) + +!ok + # End operator.iq