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 92e0739624f..bff49f3924e 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java @@ -830,36 +830,44 @@ protected RexNode convertCast( 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 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 6ce401502c9..1ce00a043dc 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/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 + * [CALCITE-7692] + * FLOOR/CEIL of INTERVAL produces wrong results. + * + *

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 + * [CALCITE-7692] + * FLOOR/CEIL of INTERVAL produces wrong results. + * + *

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 * [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 6e98c11baa8..343c9ace258 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -2624,6 +2624,18 @@ LogicalSort(fetch=[+(1, ABS(-2))]) + + + + + + + + =($0, 0), $0, -($0, 3599999)), 3600000), 3600000)], C=[*(/INT(CASE(>=($0, 0), +($0, 3599999), $0), 3600000), 3600000)]) + LogicalValues(tuples=[[{ 11045000 }]]) ]]> diff --git a/core/src/test/resources/sql/operator.iq b/core/src/test/resources/sql/operator.iq index 41a470e5f93..45bee30f0c9 100644 --- a/core/src/test/resources/sql/operator.iq +++ b/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 diff --git a/site/_docs/reference.md b/site/_docs/reference.md index 4b198449ae0..fda50d5bfc9 100644 --- a/site/_docs/reference.md +++ b/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 diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java index 387915bca6b..9205e88b823 100644 --- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java +++ b/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 + * [CALCITE-7692] + * FLOOR/CEIL of INTERVAL produces wrong results. */ @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 + * [CALCITE-7692] + * FLOOR/CEIL of INTERVAL produces wrong results. */ @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() {