[CALCITE-7692] FLOOR/CEIL of INTERVAL produces wrong results

core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java

@@ -827,39 +827,47 @@
     return rexBuilder.makeCast(call.getParserPosition(), type, arg, safe, safe, formatArg);
   }
 
   protected RexNode convertFloorCeil(SqlRexContext cx, SqlCall call) {
     final boolean floor = call.getKind() == SqlKind.FLOOR;
     final SqlParserPos pos = call.getParserPosition();
-    // Rewrite floor, ceil of interval
-    if (call.operandCount() == 1
-        && call.operand(0) instanceof SqlIntervalLiteral) {
-      final SqlIntervalLiteral literal = call.operand(0);
-      SqlIntervalLiteral.IntervalValue interval =
-          literal.getValueAs(SqlIntervalLiteral.IntervalValue.class);
-      BigDecimal val =
-          interval.getIntervalQualifier().getStartUnit().multiplier;
-      RexNode rexInterval = cx.convertExpression(literal);
-
+    // Rewrite floor, ceil of an interval as arithmetic that rounds to a
+    // multiple of the interval's leading unit.
+    if (call.operandCount() == 1) {
       final RexBuilder rexBuilder = cx.getRexBuilder();
-      RexNode zero = rexBuilder.makeExactLiteral(BigDecimal.valueOf(0));
-      RexNode cond = ge(pos, rexBuilder, rexInterval, zero);
-
-      RexNode pad =
-          rexBuilder.makeExactLiteral(val.subtract(BigDecimal.ONE));
-      RexNode cast =
-          rexBuilder.makeReinterpretCast(pos, rexInterval.getType(), pad,
-              rexBuilder.makeLiteral(false));
-      RexNode sum =
-          floor ? minus(pos, rexBuilder, rexInterval, cast)
-              : plus(pos, rexBuilder, rexInterval, cast);
-
-      RexNode kase = floor
-          ? case_(rexBuilder, rexInterval, cond, sum)
-          : case_(rexBuilder, sum, cond, rexInterval);
-
-      RexNode factor = rexBuilder.makeExactLiteral(val);
-      RexNode div = divideInt(pos, rexBuilder, kase, factor);
-      return multiply(pos, rexBuilder, div, factor);
+      final RexNode rexInterval = cx.convertExpression(call.operand(0));
+      final SqlIntervalQualifier qualifier =
+          rexInterval.getType().getIntervalQualifier();
+      if (qualifier != null) {
+        if (qualifier.timeFrameName != null) {
+          throw new UnsupportedOperationException((floor ? "FLOOR" : "CEIL")
+              + " of an interval with custom time frame '"
+              + qualifier.timeFrameName + "' is not supported");
+        }
+        if (!RexUtil.isDeterministic(rexInterval)) {
+          throw new UnsupportedOperationException((floor ? "FLOOR" : "CEIL")
+              + " of a non-deterministic interval expression is not"
+              + " supported");
+        }
+        BigDecimal val = qualifier.getStartUnit().multiplier;
+        RexNode zero = rexBuilder.makeExactLiteral(BigDecimal.valueOf(0));
+        RexNode cond = ge(pos, rexBuilder, rexInterval, zero);
+
+        RexNode pad =
+            rexBuilder.makeIntervalLiteral(val.subtract(BigDecimal.ONE),
+                qualifier);
+        RexNode sum =
+            floor ? minus(pos, rexBuilder, rexInterval, pad)
+                : plus(pos, rexBuilder, rexInterval, pad);
+
+        // CASE operands are (when, then, else)
+        RexNode kase = floor
+            ? case_(rexBuilder, cond, rexInterval, sum)
+            : case_(rexBuilder, cond, sum, rexInterval);
+
+        RexNode factor = rexBuilder.makeExactLiteral(val);
+        RexNode div = divideInt(pos, rexBuilder, kase, factor);
+        return multiply(pos, rexBuilder, div, factor);
+      }
     }
 
     // normal floor, ceil function

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

@@ -82,6 +82,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Unit test for {@link org.apache.calcite.sql2rel.SqlToRelConverter}.
@@ -6309,6 +6310,36 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) {
     assertThat(plan, containsString("FLOOR($4, FLAG(WEEK))"));
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7692">[CALCITE-7692]
+   * FLOOR/CEIL of INTERVAL produces wrong results</a>.
+   *
+   * <p>FLOOR and CEIL of an interval expression, literal or not, are rewritten
+   * as arithmetic that rounds to a multiple of the interval's leading unit. */
+  @Test void testFloorCeilOfInterval() {
+    final String sql = "select floor(x) as f, ceil(x) as c\n"
+        + "from (values (interval '3:4:5' hour to second)) as t(x)";
+    sql(sql).ok();
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7692">[CALCITE-7692]
+   * FLOOR/CEIL of INTERVAL produces wrong results</a>.
+   *
+   * <p>The rewrite evaluates its operand more than once, which is unsound
+   * for a non-deterministic operand; conversion must fail rather than
+   * produce incorrect results. */
+  @Test void testFloorOfNonDeterministicInterval() {
+    final String sql = "select floor(x * rand()) as f\n"
+        + "from (values (interval '3:4:5' hour to second)) as t(x)";
+    final UnsupportedOperationException e =
+        assertThrows(UnsupportedOperationException.class,
+            () -> sql(sql).toRel());
+    assertThat(e.getMessage(),
+        is("FLOOR of a non-deterministic interval expression is not"
+            + " supported"));
+  }
+
   /** 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>. */

core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml

@@ -2624,6 +2624,18 @@ LogicalSort(fetch=[+(1, ABS(-2))])
       <![CDATA[
 LogicalProject(EXPR$0=[ROW(ITEM($3, 1).EMPNO, ITEM($3, 1).ENAME, ROW(ITEM($3, 1).DETAIL.SKILLS))])
   LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testFloorCeilOfInterval">
+    <Resource name="sql">
+      <![CDATA[select floor(x) as f, ceil(x) as c
+from (values (interval '3:4:5' hour to second)) as t(x)]]>
+    </Resource>
+    <Resource name="plan">
+      <![CDATA[
+LogicalProject(F=[*(/INT(CASE(>=($0, 0), $0, -($0, 3599999)), 3600000), 3600000)], C=[*(/INT(CASE(>=($0, 0), +($0, 3599999), $0), 3600000), 3600000)])
+  LogicalValues(tuples=[[{ 11045000 }]])
 ]]>
     </Resource>
   </TestCase>

core/src/test/resources/sql/operator.iq

@@ -842,4 +842,45 @@ SELECT
 
 !ok
 
+# [CALCITE-7692] FLOOR/CEIL of INTERVAL produces wrong results
+# FLOOR and CEIL of an interval round to the interval's leading unit,
+# whether or not the operand is a literal.
+select floor(x) = interval '3' hour as f,
+  ceil(x) = interval '4' hour as c
+from (values (interval '3:4:5' hour to second)) as t(x);
++------+------+
+| F    | C    |
++------+------+
+| true | true |
++------+------+
+(1 row)
+
+!ok
+
+select floor(interval '-6.3' second) = interval '-7' second as fneg,
+  ceil(interval '-6.3' second) = interval '-6' second as cneg,
+  floor(interval '5-1' year to month) = interval '5' year as fym,
+  ceil(interval '-5-1' year to month) = interval '-5' year as cym;
++------+------+------+------+
+| FNEG | CNEG | FYM  | CYM  |
++------+------+------+------+
+| true | true | true | true |
++------+------+------+------+
+(1 row)
+
+!ok
+
+# The operand's interval type may be computed rather than declared; here
+# HOUR + MINUTE yields INTERVAL HOUR TO MINUTE, whose leading unit is HOUR.
+select floor(interval '2' hour + interval '90' minute) = interval '3' hour as fa,
+  ceil(interval '2' hour + interval '90' minute) = interval '4' hour as ca;
++------+------+
+| FA   | CA   |
++------+------+
+| true | true |
++------+------+
+(1 row)
+
+!ok
+
 # End operator.iq

site/_docs/reference.md

@@ -1608,6 +1608,8 @@ Not implemented:
 | EXTRACT(timeUnit FROM datetime) | Extracts and returns the value of a specified datetime field from a datetime value expression
 | FLOOR(datetime TO timeUnit) | Rounds *datetime* down to *timeUnit*
 | CEIL(datetime TO timeUnit) | Rounds *datetime* up to *timeUnit*
+| FLOOR(interval) | Rounds *interval* down to a multiple of its leading time unit; for example, `FLOOR(INTERVAL '3:04:05' HOUR TO SECOND)` returns `INTERVAL '3:00:00' HOUR TO SECOND`
+| CEIL(interval) | Rounds *interval* up to a multiple of its leading time unit
 | YEAR(date)                | Equivalent to `EXTRACT(YEAR FROM date)`. Returns an integer.
 | QUARTER(date)             | Equivalent to `EXTRACT(QUARTER FROM date)`. Returns an integer between 1 and 4.
 | MONTH(date)               | Equivalent to `EXTRACT(MONTH FROM date)`. Returns an integer between 1 and 12.
@@ -1628,12 +1630,6 @@ standard SQL. Calls with parentheses, such as `CURRENT_DATE()` are accepted in c
 
 Not implemented:
 
-* CEIL(interval)
-* FLOOR(interval)
-* \+ interval
-* \- interval
-* interval + interval
-* interval - interval
 * interval / interval
 
 ### System functions

testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java

@@ -14121,11 +14121,11 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
     f.checkNull("ceiling(cast(null as double))");
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7692">[CALCITE-7692]
+   * FLOOR/CEIL of INTERVAL produces wrong results</a>. */
   @Test void testCeilFuncInterval() {
     final SqlOperatorFixture f = fixture();
-    if (!f.brokenTestsEnabled()) {
-      return;
-    }
     f.checkScalar("ceil(interval '3:4:5' hour to second)",
         "+4:00:00.000000", "INTERVAL HOUR TO SECOND NOT NULL");
     f.checkScalar("ceil(interval '-6.3' second)",
@@ -14354,11 +14354,11 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
             "-4", "INTEGER NOT NULL");
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7692">[CALCITE-7692]
+   * FLOOR/CEIL of INTERVAL produces wrong results</a>. */
   @Test void testFloorFuncInterval() {
     final SqlOperatorFixture f = fixture();
-    if (!f.brokenTestsEnabled()) {
-      return;
-    }
     f.checkScalar("floor(interval '3:4:5' hour to second)",
         "+3:00:00.000000",
         "INTERVAL HOUR TO SECOND NOT NULL");
@@ -14368,6 +14368,12 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
         "+5-00", "INTERVAL YEAR TO MONTH NOT NULL");
     f.checkScalar("floor(interval '-5-1' year to month)",
         "-6-00", "INTERVAL YEAR TO MONTH NOT NULL");
+    f.checkNull("floor(cast(null as interval year))");
+    if (!f.brokenTestsEnabled()) {
+      return;
+    }
+    // FLOOR(interval TO time unit) is not implemented; the validator accepts
+    // only DATE, TIME and TIMESTAMP before TO.
     f.checkScalar("floor(interval '-6.3' second to second)",
         "-7.000000", "INTERVAL SECOND NOT NULL");
     f.checkScalar("floor(interval '6-3' minute to second to minute)",
@@ -14384,7 +14390,6 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
         "201", "INTERVAL YEAR TO MONTH NOT NULL");
     f.checkScalar("floor(interval '1004-1' year to month to millennium)",
         "2001-00", "INTERVAL YEAR TO MONTH NOT NULL");
-    f.checkNull("floor(cast(null as interval year))");
   }
 
   @Test void testTimestampAdd() {