[CALCITE-7451] REINTERPRET should not be used in logical plans

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<Expression> argValueList) {
-      assert call.getOperands().size() == 1;
-      return argValueList.get(0);
-    }
-  }
-
   /** Implementor for sort_array. */
   private static class SortArrayImplementor extends AbstractRexCallImplementor {
     SortArrayImplementor() {

core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java

@@ -720,6 +720,20 @@
     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 @@
         // 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.

core/src/main/java/org/apache/calcite/plan/Strong.java

@@ -358,7 +358,7 @@ private static Map<SqlKind, Policy> 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);

core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java

@@ -221,8 +221,11 @@
 
   /** 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();
 
   /** Rule that reduces constants inside a {@link LogicalCalc}.

core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java

@@ -73,9 +73,20 @@
  * 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<ReduceDecimalsRule.Config>
     implements TransformationRule {
 

core/src/main/java/org/apache/calcite/rex/RexBuilder.java

@@ -874,9 +874,6 @@
         }
         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 @@
             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,15 +1164,23 @@
    *                      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,
       boolean checkOverflow) {
     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,
       RelDataType type,
@@ -1201,12 +1196,20 @@
    *
    * @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());
     RelDataType bigintType = typeFactory.createSqlType(SqlTypeName.BIGINT);
@@ -1289,7 +1292,13 @@
    * @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 @@
    * @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,

core/src/main/java/org/apache/calcite/rex/RexUtil.java

@@ -1018,8 +1018,13 @@
    * @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) {
     if (!(expr instanceof RexCall)) {
@@ -1069,8 +1074,13 @@
 
   /**
    * 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<RexNode> operands,
       boolean recurse) {
     for (RexNode operand : operands) {
@@ -1087,8 +1097,13 @@
   /**
    * 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) {
     final List<RexNode> exprList = program.getExprList();
@@ -1100,7 +1115,12 @@
     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;
   }

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.
+   *
+   * <p>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,
 

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 <i>any</i> 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() {

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;

core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java

@@ -10752,6 +10752,7 @@ public interface Config extends RelRule.Config {
    * Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-3319">[CALCITE-3319]
    * AssertionError for ReduceDecimalsRule</a>. */
+  @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)

core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java

@@ -6335,6 +6335,40 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) {
             + " supported"));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7451">[CALCITE-7451]
+   * REINTERPRET should not be used in logical plans</a>.
+   *
+   * <p>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
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7451">[CALCITE-7451]
+   * REINTERPRET should not be used in logical plans</a>.
+   *
+   * <p>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
    * <a href="https://issues.apache.org/jira/browse/CALCITE-5406">[CALCITE-5406]
    * Support the SELECT DISTINCT ON statement for PostgreSQL dialect</a>. */