This is an automated email from the ASF dual-hosted git repository.
mihaibudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 7c0dfef966 [CALCITE-7443] Incorrect simplification for large interval
7c0dfef966 is described below
commit 7c0dfef96602b320da006c4086aec9ba38edbaef
Author: Mihai Budiu <[email protected]>
AuthorDate: Tue Mar 24 17:55:14 2026 -0700
[CALCITE-7443] Incorrect simplification for large interval
Signed-off-by: Mihai Budiu <[email protected]>
---
.../java/org/apache/calcite/prepare/Prepare.java | 13 ++++--
.../org/apache/calcite/runtime/SqlFunctions.java | 20 +++++++-
.../apache/calcite/sql2rel/ConvertToChecked.java | 54 ++++++++++++++++++----
.../calcite/sql2rel/StandardConvertletTable.java | 2 +-
core/src/test/resources/sql/scalar.iq | 26 +++++++++++
5 files changed, 100 insertions(+), 15 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/prepare/Prepare.java
b/core/src/main/java/org/apache/calcite/prepare/Prepare.java
index 586cce8385..1c6d8875cb 100644
--- a/core/src/main/java/org/apache/calcite/prepare/Prepare.java
+++ b/core/src/main/java/org/apache/calcite/prepare/Prepare.java
@@ -256,11 +256,14 @@ public PreparedResult prepareSql(
RelRoot root =
sqlToRelConverter.convertQuery(sqlQuery, needsValidation, true);
- if (this.context.config().conformance().checkedArithmetic()) {
- ConvertToChecked checkedConv = new
ConvertToChecked(root.rel.getCluster().getRexBuilder());
- RelNode rel = checkedConv.visit(root.rel);
- root = root.withRel(rel);
- }
+ boolean convertToChecked =
this.context.config().conformance().checkedArithmetic();
+ // Convert some operations to use checked arithmetic:
+ // - all arithmetic operations on exact types if the conformance requires
checked arithmetic
+ // - all arithmetic that produces INTERVAL results, regardless of the
conformance
+ ConvertToChecked checkedConv =
+ new ConvertToChecked(root.rel.getCluster().getRexBuilder(),
convertToChecked);
+ RelNode rel = checkedConv.visit(root.rel);
+ root = root.withRel(rel);
Hook.CONVERTED.run(root.rel);
if (timingTracer != null) {
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index cc6b56520d..c1ebafe8ed 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -2976,10 +2976,14 @@ public static long checkedDivide(long b0, long b1) {
if ((b0 & b1 & q) >= 0) {
return q;
} else {
- throw new ArithmeticException("integer overflow");
+ throw new ArithmeticException("long overflow");
}
}
+ public static double checkedDivide(int b0, double b1) {
+ return b0 / b1;
+ }
+
public static UByte checkedDivide(UByte b0, UByte b1) {
return UByte.valueOf(b0.intValue() / b1.intValue());
}
@@ -2996,6 +3000,16 @@ public static ULong checkedDivide(ULong b0, ULong b1) {
return
ULong.valueOf(UnsignedType.toBigInteger(b0).divide(UnsignedType.toBigInteger(b1)));
}
+ // The definition of this function must match the divide function with the
same signature
+ public static int checkedDivide(int b0, BigDecimal b1) {
+ return BigDecimal.valueOf(b0)
+ .divide(b1, RoundingMode.HALF_DOWN).intValueExact();
+ }
+
+ public static BigDecimal checkedDivide(BigDecimal b0, BigDecimal b1) {
+ return b0.divide(b1, RoundingMode.HALF_DOWN);
+ }
+
// *
/** SQL <code>*</code> operator applied to int values. */
@@ -3118,6 +3132,10 @@ public static ULong checkedMultiply(ULong b0, ULong b1) {
return
ULong.valueOf(UnsignedType.toBigInteger(b0).multiply(UnsignedType.toBigInteger(b1)));
}
+ public static BigDecimal checkedMultiply(BigDecimal b0, long b1) {
+ return b0.multiply(BigDecimal.valueOf(b1));
+ }
+
/** SQL <code>SAFE_ADD</code> function applied to long values. */
public static @Nullable Long safeAdd(long b0, long b1) {
try {
diff --git
a/core/src/main/java/org/apache/calcite/sql2rel/ConvertToChecked.java
b/core/src/main/java/org/apache/calcite/sql2rel/ConvertToChecked.java
index a5e658e54a..e8c4697c12 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/ConvertToChecked.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/ConvertToChecked.java
@@ -38,8 +38,8 @@
public class ConvertToChecked extends RelHomogeneousShuttle {
final ConvertRexToChecked converter;
- public ConvertToChecked(RexBuilder builder) {
- this.converter = new ConvertRexToChecked(builder);
+ public ConvertToChecked(RexBuilder builder, boolean allArithmetic) {
+ this.converter = new ConvertRexToChecked(builder, allArithmetic);
}
@Override public RelNode visit(RelNode other) {
@@ -48,14 +48,36 @@ public ConvertToChecked(RexBuilder builder) {
}
/**
- * Visitor which rewrites an expression tree such that all
- * arithmetic operations that produce numeric values use checked arithmetic.
+ * Visitor which rewrites an expression tree such that arithmetic operations
+ * use checked arithmetic.
*/
class ConvertRexToChecked extends RexShuttle {
private final RexBuilder builder;
+ // If true all arithmetic operations are converted.
+ // Otherwise, only arithmetic operations on INTERVAL values is checked.
+ private final boolean allArithmetic;
+ /**
+ * Create a visitor which converts all arithmetic operations to checked.
+ *
+ * @deprecated Use #ConvertRexToChecked(RexBuilder, boolean).
+ */
+ @Deprecated
ConvertRexToChecked(RexBuilder builder) {
+ this(builder, true);
+ }
+
+ /**
+ * Create a converter that replaces arithmetic with checked arithmetic.
+ *
+ * @param builder RexBuilder to use.
+ * @param allArithmetic If true all exact arithmetic operations are
converted to checked.
+ * If false, only operations that produce
INTERVAL-typed results
+ * are converted to checked.
+ */
+ ConvertRexToChecked(RexBuilder builder, boolean allArithmetic) {
this.builder = builder;
+ this.allArithmetic = allArithmetic;
}
@Override public RexNode visitSubQuery(RexSubQuery subQuery) {
@@ -72,6 +94,22 @@ class ConvertRexToChecked extends RexShuttle {
List<RexNode> clonedOperands = visitList(call.operands, update);
SqlKind kind = call.getKind();
SqlOperator operator = call.getOperator();
+ SqlTypeName resultType = call.getType().getSqlTypeName();
+ boolean anyOperandIsInterval = false;
+ for (RexNode op : call.getOperands()) {
+ if
(SqlTypeName.INTERVAL_TYPES.contains(op.getType().getSqlTypeName())) {
+ anyOperandIsInterval = true;
+ break;
+ }
+ }
+ boolean resultIsInterval =
SqlTypeName.INTERVAL_TYPES.contains(resultType);
+ boolean rewrite =
+ // Do not rewrite operator if the type is e.g., DOUBLE or DATE
+ (this.allArithmetic && SqlTypeName.EXACT_TYPES.contains(resultType))
+ // But always rewrite if the type is an INTERVAL and any operand is
INTERVAL
+ // This will not rewrite date subtraction, for example
+ || (resultIsInterval && anyOperandIsInterval);
+
switch (kind) {
case PLUS:
operator = SqlStdOperatorTable.CHECKED_PLUS;
@@ -91,8 +129,7 @@ class ConvertRexToChecked extends RexShuttle {
default:
break;
}
- SqlTypeName resultType = call.getType().getSqlTypeName();
- if (resultType == SqlTypeName.DECIMAL) {
+ if (resultType == SqlTypeName.DECIMAL && this.allArithmetic) {
// Checked decimal arithmetic is implemented using unchecked
// arithmetic followed by a CAST, which is always checked
RexCall result;
@@ -102,8 +139,9 @@ class ConvertRexToChecked extends RexShuttle {
result = call;
}
return builder.makeCast(call.getParserPosition(), call.getType(),
result);
- } else if (!SqlTypeName.EXACT_TYPES.contains(resultType)) {
- // Do not rewrite operator if the type is e.g., DOUBLE or DATE
+ }
+
+ if (!rewrite) {
operator = call.getOperator();
}
update[0] = update[0] || operator != call.getOperator();
diff --git
a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
index e8b1bcbd7a..739c3e30fa 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
@@ -552,7 +552,7 @@ private static RexNode convertInterval(SqlRexContext cx,
SqlCall call) {
SqlLiteral.createInterval(1, "1", intervalQualifier,
call.getParserPosition());
final SqlCall multiply =
- SqlStdOperatorTable.MULTIPLY.createCall(call.getParserPosition(), n,
+
SqlStdOperatorTable.CHECKED_MULTIPLY.createCall(call.getParserPosition(), n,
literal);
return cx.convertExpression(multiply);
}
diff --git a/core/src/test/resources/sql/scalar.iq
b/core/src/test/resources/sql/scalar.iq
index d82d69f589..e4ef19c0d3 100644
--- a/core/src/test/resources/sql/scalar.iq
+++ b/core/src/test/resources/sql/scalar.iq
@@ -18,6 +18,32 @@
!set outputformat mysql
!use scott
+# 5 test cases for [CALCITE-7443] Incorrect simplification for large interval
+SELECT -(INTERVAL -2147483648 months);
+java.lang.ArithmeticException: integer overflow
+
+!error
+
+SELECT INTERVAL 2147483647 years;
+java.lang.ArithmeticException: integer overflow
+
+!error
+
+SELECT -(INTERVAL -9223372036854775.808 SECONDS);
+java.lang.ArithmeticException: long overflow
+
+!error
+
+SELECT INTERVAL 3000000 months * 1000;
+java.lang.ArithmeticException: integer overflow
+
+!error
+
+SELECT INTERVAL 3000000 months / .0001;
+java.lang.ArithmeticException: Overflow
+
+!error
+
select deptno, (select min(empno) from "scott".emp where deptno = dept.deptno)
as x from "scott".dept;
+--------+------+
| DEPTNO | X |
