You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by mb...@apache.org on 2025/09/02 05:34:48 UTC
(calcite) 02/02: Reject some date arithmetic on timeframes that do not make sense
This is an automated email from the ASF dual-hosted git repository.
mbudiu pushed a commit to branch issue7149
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit f342f80e2f3db1bc64b39a3bd9b3c558c6b054f3
Author: Mihai Budiu <mb...@feldera.com>
AuthorDate: Mon Sep 1 22:34:31 2025 -0700
Reject some date arithmetic on timeframes that do not make sense
Signed-off-by: Mihai Budiu <mb...@feldera.com>
---
.../apache/calcite/runtime/CalciteResource.java | 3 +++
.../calcite/sql/fun/SqlTimestampAddFunction.java | 12 ++++++++++--
.../calcite/sql/fun/SqlTimestampDiffFunction.java | 22 ++++++++++++++++++++--
.../calcite/sql2rel/StandardConvertletTable.java | 8 +++-----
.../calcite/runtime/CalciteResource.properties | 1 +
.../apache/calcite/test/SqlToRelConverterTest.java | 6 ------
.../org/apache/calcite/test/SqlValidatorTest.java | 10 ++++++++++
.../apache/calcite/test/SqlToRelConverterTest.xml | 11 -----------
8 files changed, 47 insertions(+), 26 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
index dea1d15052..2eaf952d15 100644
--- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
+++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -645,6 +645,9 @@ ExInst<CalciteException> illegalArgumentForTableFunctionCall(String a0,
@BaseMessage("''{0}'' is not a valid time frame")
ExInst<SqlValidatorException> invalidTimeFrame(String a0);
+ @BaseMessage("''{0}'' is not a valid time frame in ''{1}''")
+ ExInst<SqlValidatorException> invalidTimeFrameInOperation(String a0, String a1);
+
@BaseMessage("Cannot INSERT into generated column ''{0}''")
ExInst<SqlValidatorException> insertIntoAlwaysGenerated(String a0);
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java
index 08bf7ce0f5..ec886d2013 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampAddFunction.java
@@ -38,6 +38,7 @@
import java.util.Map;
+import static org.apache.calcite.util.Static.RESOURCE;
import static org.apache.calcite.util.Util.first;
/**
@@ -129,8 +130,15 @@ static RelDataType deduceType(RelDataTypeFactory typeFactory,
// with startUnit = EPOCH and timeFrameName = 'MINUTE15'.
//
// If the latter, check that timeFrameName is valid.
- validator.validateTimeFrame(
- (SqlIntervalQualifier) call.getOperandList().get(0));
+ SqlIntervalQualifier op0 = call.operand(0);
+ validator.validateTimeFrame(op0);
+ if (op0.timeFrameName == null
+ && op0.timeUnitRange.startUnit.multiplier == null) {
+ // Not all time frames can be used in date arithmetic, e.g., DOW
+ throw validator.newValidationError(op0,
+ RESOURCE.invalidTimeFrameInOperation(
+ op0.timeUnitRange.toString(), call.getOperator().getName()));
+ }
}
/** Creates a SqlTimestampAddFunction. */
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampDiffFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampDiffFunction.java
index 44b077da0d..806e29512d 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampDiffFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampDiffFunction.java
@@ -30,6 +30,8 @@
import org.apache.calcite.sql.validate.SqlValidator;
import org.apache.calcite.sql.validate.SqlValidatorScope;
+import static org.apache.calcite.util.Static.RESOURCE;
+
/**
* The <code>TIMESTAMPDIFF</code> function, which calculates the difference
* between two timestamps.
@@ -107,9 +109,25 @@ private static RelDataType inferReturnType2(SqlOperatorBinding opBinding) {
//
// If the latter, check that timeFrameName is valid.
if (call.operand(2) instanceof SqlIntervalQualifier) {
- validator.validateTimeFrame(call.operand(2));
+ SqlIntervalQualifier op2 = call.operand(2);
+ validator.validateTimeFrame(op2);
+ if (op2.timeFrameName == null
+ && op2.timeUnitRange.startUnit.multiplier == null) {
+ // Not all time frames can be used in date arithmetic, e.g., DOW
+ throw validator.newValidationError(op2,
+ RESOURCE.invalidTimeFrameInOperation(
+ op2.timeUnitRange.toString(), call.getOperator().getName()));
+ }
} else {
- validator.validateTimeFrame(call.operand(0));
+ SqlIntervalQualifier op0 = call.operand(0);
+ validator.validateTimeFrame(op0);
+ if (op0.timeFrameName == null
+ && op0.timeUnitRange.startUnit.multiplier == null) {
+ // Not all time frames can be used in date arithmetic, e.g., DOW
+ throw validator.newValidationError(op0,
+ RESOURCE.invalidTimeFrameInOperation(
+ op0.timeUnitRange.toString(), call.getOperator().getName()));
+ }
}
}
}
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 b286d8f16b..eee3d330e9 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
@@ -2351,6 +2351,9 @@ private static class TimestampAddConvertlet implements SqlRexConvertlet {
RexNode interval2Add;
BigDecimal multiplier = unit.multiplier;
+ if (multiplier == null) {
+ throw new IllegalArgumentException("Impossible conversion to " + unit);
+ }
switch (unit) {
case MICROSECOND:
case NANOSECOND:
@@ -2360,11 +2363,6 @@ private static class TimestampAddConvertlet implements SqlRexConvertlet {
rexBuilder.makeIntervalLiteral(BigDecimal.ONE, qualifier), op1),
BigDecimal.ONE.divide(multiplier, RoundingMode.UNNECESSARY));
break;
- case DOY:
- case DOW:
- multiplier = TimeUnit.DAY.multiplier;
- // There is no multiplier for these units; treat as if the unit is DAY
- // fall through
default:
interval2Add =
multiply(pos, rexBuilder,
diff --git a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
index 055233677a..3628d6a982 100644
--- a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
+++ b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -216,6 +216,7 @@ DelimiterIsRequired=Call to function ''{0}'' with argument of type ''{1}'' requi
IllegalArgumentForTableFunctionCall=Wrong arguments for table function ''{0}'' call. Expected ''{1}'', actual ''{2}''
CannotCallTableFunctionHere=Cannot call table function here: ''{0}''
InvalidTimeFrame=''{0}'' is not a valid time frame
+InvalidTimeFrameInOperation=''{0}'' is not a valid time frame in ''{1}''
InsertIntoAlwaysGenerated=Cannot INSERT into generated column ''{0}''
ArgumentMustHaveScaleZero=Argument to function ''{0}'' must have a scale of 0
PreparationAborted=Statement preparation aborted
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 04f5b31967..8f020b6f78 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -1274,12 +1274,6 @@ public static void checkActualAndReferenceFiles() {
sql(sql).ok();
}
- /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-7149">[CALCITE-7149]
- * Constant TIMESTAMPADD expression causes assertion failure in validator</a>. */
- @Test void testTimestampAdd() {
- sql("SELECT TIMESTAMPADD(DOY, 2, TIMESTAMP '2020-06-21 14:23:44.123')").ok();
- }
-
@Test void testWithAlias() {
final String sql = "with w(x, y) as\n"
+ " (select * from dept where deptno > 10)\n"
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 8928861e5d..de7cd015ff 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -1898,6 +1898,16 @@ void testLikeAndSimilarFails() {
.fails("No match found for function signature FOO..");
}
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-7149">[CALCITE-7149]
+ * Constant TIMESTAMPADD expression causes assertion failure in validator</a>. */
+ @Test void testTimestampAdd() {
+ sql("SELECT TIMESTAMPADD(^DOY^, 2, TIMESTAMP '2020-06-21 14:23:44.123')")
+ .fails("'DOY' is not a valid time frame in 'TIMESTAMPADD'");
+ sql("SELECT TIMESTAMPDIFF(^EPOCH^, TIMESTAMP '2020-06-21 14:23:44.123', "
+ + "TIMESTAMP '2022-06-21 14:23:44.123')")
+ .fails("'EPOCH' is not a valid time frame in 'TIMESTAMPDIFF'");
+ }
+
@Test void testInvalidTableFunction() {
// A table function at most have one input table with row semantics
sql("select * from table(^invalid(table orders, table emp)^)")
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 a366ff3b8b..d46d1931bb 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -8682,17 +8682,6 @@ LogicalProject(ROWTIME=[$0], PRODUCTID=[$1], ORDERID=[$2], RANK_NUMBER=[$3])
<![CDATA[
LogicalProject(DEPTNO=[$0], NAME=[$1])
LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
-]]>
- </Resource>
- </TestCase>
- <TestCase name="testTimestampAdd">
- <Resource name="sql">
- <![CDATA[SELECT TIMESTAMPADD(DOY, 2, TIMESTAMP '2020-06-21 14:23:44.123')]]>
- </Resource>
- <Resource name="plan">
- <![CDATA[
-LogicalProject(EXPR$0=[+(2020-06-21 14:23:44.123:TIMESTAMP(3), *(86400000:INTERVAL DOY, 2))])
- LogicalValues(tuples=[[{ 0 }]])
]]>
</Resource>
</TestCase>