From 03794f588e13458cbabfd7a38eefce5ccaed9cce Mon Sep 17 00:00:00 2001 From: Kirill Tkalenko Date: Mon, 20 Jul 2026 20:42:44 +0300 Subject: [PATCH 1/5] [CALCITE-7662] Wip --- core/src/main/codegen/templates/Parser.jj | 25 ++- .../adapter/enumerable/EnumerableLimit.java | 10 +- .../enumerable/EnumerableLimitSort.java | 8 +- .../enumerable/EnumerableMergeUnionRule.java | 17 +- .../rel/metadata/RelMdMinRowCount.java | 6 +- .../calcite/rel/metadata/RelMdUtil.java | 5 +- .../rel/rel2sql/RelToSqlConverter.java | 11 +- .../rel/rules/SortJoinTransposeRule.java | 5 +- .../rel/rules/SortUnionTransposeRule.java | 24 ++- .../java/org/apache/calcite/rex/RexUtil.java | 69 ++++++- .../calcite/runtime/CalciteResource.java | 14 +- .../org/apache/calcite/sql/SqlDialect.java | 54 +++-- .../calcite/sql/dialect/MysqlSqlDialect.java | 10 +- .../calcite/sql/dialect/SqliteSqlDialect.java | 9 +- .../sql/validate/SqlValidatorImpl.java | 32 +-- .../calcite/sql2rel/RelDecorrelator.java | 6 +- .../org/apache/calcite/tools/RelBuilder.java | 48 ++--- .../runtime/CalciteResource.properties | 6 +- .../rel/rel2sql/RelToSqlConverterTest.java | 56 ++++++ .../apache/calcite/rex/RexProgramTest.java | 11 ++ .../org/apache/calcite/test/JdbcTest.java | 149 ++++++++++++-- .../apache/calcite/test/RelBuilderTest.java | 66 +++++++ .../apache/calcite/test/RelMetadataTest.java | 28 +++ .../apache/calcite/test/RelOptRulesTest.java | 49 +++++ .../calcite/test/SqlToRelConverterTest.java | 6 + .../apache/calcite/test/SqlValidatorTest.java | 22 ++- .../enumerable/EnumerableMergeUnionTest.java | 100 ++++++++-- .../apache/calcite/test/RelOptRulesTest.xml | 117 +++++++++++ .../calcite/test/SqlToRelConverterTest.xml | 12 ++ core/src/test/resources/sql/offset.iq | 186 ++++++++++++++++++ .../org/apache/calcite/test/ServerTest.java | 22 +++ site/_docs/reference.md | 18 +- .../calcite/sql/parser/SqlParserTest.java | 21 +- 33 files changed, 1064 insertions(+), 158 deletions(-) create mode 100644 core/src/test/resources/sql/offset.iq diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj index 5c403e1cedd8..c949c7b5e90b 100644 --- a/core/src/main/codegen/templates/Parser.jj +++ b/core/src/main/codegen/templates/Parser.jj @@ -671,13 +671,13 @@ SqlNode ExprOrJoinOrOrderedQuery(ExprContext exprContext) : * * 
  *    [ LIMIT { count | ALL } ]
- *    [ OFFSET start ]
+ * [ OFFSET { start | expression } ] *
* *

Trino syntax for limit: * * 

- *    [ OFFSET start ]
+ *    [ OFFSET { start | expression } ]
  *    [ LIMIT { count | ALL } ]
*
* @@ -690,7 +690,7 @@ SqlNode ExprOrJoinOrOrderedQuery(ExprContext exprContext) : *

SQL:2008 syntax for limit: * * 

- *    [ OFFSET start { ROW | ROWS } ]
+ *    [ OFFSET { start | expression } { ROW | ROWS } ]
  *    [ FETCH { FIRST | NEXT } [ count | (expression) ] { ROW | ROWS } ONLY ]
*
*/ @@ -765,7 +765,7 @@ void OffsetClause(Span s, SqlNode[] offsetFetch) : // ROW or ROWS is required in SQL:2008 but we make it optional // because it is not present in Postgres-style syntax. { s.add(this); } - offsetFetch[0] = UnsignedNumericLiteralOrParam() + offsetFetch[0] = OffsetCount() [ | ] } @@ -782,6 +782,23 @@ void FetchClause(SqlNode[] offsetFetch) : ( | ) } +/** + * Parses the start value or expression of an OFFSET clause. + */ +SqlNode OffsetCount() : +{ + final SqlNode e; +} +{ + // Unlike FETCH expressions, OFFSET expressions do not require parentheses + // and may start with a numeric literal or dynamic parameter. Therefore, a + // separate UnsignedNumericLiteralOrParam alternative would consume only + // the prefix of expressions such as "OFFSET 1 + 2" or "OFFSET ? + 1". + // Expression also covers standalone start values. + e = Expression(ExprContext.ACCEPT_NON_QUERY) + { return e; } +} + /** * Parses the row count of a FETCH clause. Expressions must be parenthesized. */ diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimit.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimit.java index de1f94d562d5..96a04be1b862 100644 --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimit.java +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimit.java @@ -107,14 +107,14 @@ public static EnumerableLimit create(final RelNode input, @Nullable RexNode offs builder.append("offset", Expressions.call(BuiltInMethod.SKIP_BIG_DECIMAL.method, v, getExpression(offset, "OFFSET", implementor, builder, - roundingPolicyExp, false))); + roundingPolicyExp))); } if (fetch != null) { v = builder.append("fetch", Expressions.call(BuiltInMethod.TAKE_BIG_DECIMAL.method, v, getExpression(fetch, "FETCH", implementor, builder, - roundingPolicyExp, true))); + roundingPolicyExp))); } builder.add(Expressions.return_(null, v)); @@ -123,7 +123,7 @@ public static EnumerableLimit create(final RelNode input, @Nullable RexNode offs static Expression getExpression(RexNode rexNode, String kind, EnumerableRelImplementor implementor, BlockBuilder builder, - Expression roundingPolicy, boolean translateExpression) { + Expression roundingPolicy) { final Expression value; if (rexNode instanceof RexDynamicParam) { final RexDynamicParam param = (RexDynamicParam) rexNode; @@ -134,10 +134,6 @@ static Expression getExpression(RexNode rexNode, String kind, } else if (rexNode instanceof RexLiteral) { value = Expressions.constant(RexLiteral.bigDecimalValue(rexNode)); } else { - if (!translateExpression) { - throw new IllegalArgumentException(kind + " must be a literal or dynamic parameter"); - } - value = RexToLixTranslator.forAggregation(implementor.getTypeFactory(), builder, null, implementor.getConformance()) diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitSort.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitSort.java index 97d9fd8169b7..68759436d0fd 100644 --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitSort.java +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitSort.java @@ -105,7 +105,7 @@ public static EnumerableLimitSort create( fetchVal = Expressions.constant(BigDecimal.valueOf(Integer.MAX_VALUE)); } else { fetchVal = - getExpression(this.fetch, "FETCH", implementor, builder, roundingPolicyExp, true); + getExpression(this.fetch, "FETCH", implementor, builder, roundingPolicyExp); } final Expression offsetVal; @@ -113,7 +113,7 @@ public static EnumerableLimitSort create( offsetVal = Expressions.constant(BigDecimal.ZERO); } else { offsetVal = - getExpression(this.offset, "OFFSET", implementor, builder, roundingPolicyExp, false); + getExpression(this.offset, "OFFSET", implementor, builder, roundingPolicyExp); } builder.add( @@ -125,10 +125,10 @@ public static EnumerableLimitSort create( builder.appendIfNotNull("comparator", pair.right)) .appendIfNotNull( builder.appendIfNotNull("offset", - Expressions.constant(offsetVal))) + offsetVal)) .appendIfNotNull( builder.appendIfNotNull("fetch", - Expressions.constant(fetchVal)))))); + fetchVal))))); return implementor.result(physType, builder.toBlock()); } } diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java index 57f864794aa9..9315f62a8a5a 100644 --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java @@ -27,7 +27,6 @@ import org.apache.calcite.rel.logical.LogicalUnion; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeField; -import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.rex.RexNode; import org.apache.calcite.rex.RexUtil; import org.apache.calcite.tools.RelBuilder; @@ -86,19 +85,19 @@ public EnumerableMergeUnionRule(Config config) { final Union union = call.rel(1); final int unionInputsSize = union.getInputs().size(); - // Push down sort limit, if possible. + // Push down sort limit, if possible. Do not push it through UNION DISTINCT: + // duplicate rows do not count towards the final limit, so limiting an input + // before duplicate elimination could discard a row needed in the result. RexNode inputFetch = null; - if (sort.fetch != null) { + if (union.all && sort.fetch != null) { final boolean safeToReevaluate = - RexUtil.isDeterministic(sort.fetch); + RexUtil.isDeterministic(sort.fetch) + && (sort.offset == null || RexUtil.isDeterministic(sort.offset)); if (sort.offset == null && safeToReevaluate) { inputFetch = sort.fetch; - } else if (safeToReevaluate - && sort.fetch instanceof RexLiteral - && sort.offset instanceof RexLiteral) { + } else if (safeToReevaluate) { inputFetch = - call.builder().literal(RexLiteral.bigDecimalValue(sort.fetch) - .add(RexLiteral.bigDecimalValue(sort.offset))); + RexUtil.makeOffsetFetchSum(sort.getCluster().getRexBuilder(), sort.offset, sort.fetch); } } diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMinRowCount.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMinRowCount.java index 2cb710f39808..298e77f2d8e4 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMinRowCount.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMinRowCount.java @@ -117,7 +117,8 @@ public Double getMinRowCount(Sort rel, RelMetadataQuery mq) { } final double offset = - literalValueApproximatedByDouble(rel.offset, 0D); + literalValueApproximatedByDouble(rel.offset, + rel.offset == null ? 0D : rowCount); rowCount = Math.max(rowCount - offset, 0D); final double limit = @@ -133,7 +134,8 @@ public Double getMinRowCount(EnumerableLimit rel, RelMetadataQuery mq) { } final double offset = - literalValueApproximatedByDouble(rel.offset, 0D); + literalValueApproximatedByDouble(rel.offset, + rel.offset == null ? 0D : rowCount); rowCount = Math.max(rowCount - offset, 0D); final double limit = diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java index 5b096289382e..ab7dab016b4b 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java @@ -29,7 +29,6 @@ import org.apache.calcite.rel.core.Union; import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexCall; -import org.apache.calcite.rex.RexDynamicParam; import org.apache.calcite.rex.RexInputRef; import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.rex.RexLocalRef; @@ -1057,7 +1056,9 @@ private static boolean alreadySmaller(RelMetadataQuery mq, RelNode input, } } final Double rowCount = mq.getMaxRowCount(input); - if (rowCount == null || offset instanceof RexDynamicParam || !(fetch instanceof RexLiteral)) { + if (rowCount == null + || (offset != null && !(offset instanceof RexLiteral)) + || !(fetch instanceof RexLiteral)) { // Cannot be determined return false; } diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java index dc91fca54ecb..0ca774dd205f 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java @@ -1188,7 +1188,7 @@ public Result visit(Sort e) { SqlNodeList sortExps = exprList(builder.context, e.getSortExps()); sqlSelect.setOrderBy(sortExps); if (e.offset != null) { - SqlNode offset = builder.context.toSql(null, e.offset); + SqlNode offset = toSqlOffset(e, builder.context); sqlSelect.setOffset(offset); } if (e.fetch != null) { @@ -1253,10 +1253,17 @@ void offsetFetch(Sort e, Builder builder) { builder.setFetch(toSqlFetch(e, builder.context)); } if (e.offset != null) { - builder.setOffset(builder.context.toSql(null, e.offset)); + builder.setOffset(toSqlOffset(e, builder.context)); } } + private static SqlNode toSqlOffset(Sort sort, Context context) { + final RexNode offset = requireNonNull(sort.offset, "offset"); + final @Nullable RexLiteral reduced = + RexUtil.reduceOffsetToLiteral(sort.getCluster(), offset); + return context.toSql(null, reduced == null ? offset : reduced); + } + private static SqlNode toSqlFetch(Sort sort, Context context) { final RexNode fetch = requireNonNull(sort.fetch, "fetch"); final @Nullable RexLiteral reduced = diff --git a/core/src/main/java/org/apache/calcite/rel/rules/SortJoinTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/SortJoinTransposeRule.java index df967e56aad6..dfb2d8401d7c 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/SortJoinTransposeRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/SortJoinTransposeRule.java @@ -30,7 +30,6 @@ import org.apache.calcite.rel.metadata.RelMdUtil; import org.apache.calcite.rel.metadata.RelMetadataQuery; import org.apache.calcite.rex.RexBuilder; -import org.apache.calcite.rex.RexDynamicParam; import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.rex.RexNode; import org.apache.calcite.tools.RelBuilderFactory; @@ -106,8 +105,8 @@ public SortJoinTransposeRule(Class sortClass, final Join join = call.rel(1); // The pushed fetch is calculated from literal offset and fetch values. - if (sort.offset instanceof RexDynamicParam - || sort.fetch != null && !(sort.fetch instanceof RexLiteral)) { + if ((sort.offset != null && !(sort.offset instanceof RexLiteral)) + || (sort.fetch != null && !(sort.fetch instanceof RexLiteral))) { return false; } diff --git a/core/src/main/java/org/apache/calcite/rel/rules/SortUnionTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/SortUnionTransposeRule.java index 93b6af657c43..21ec54ec60fb 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/SortUnionTransposeRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/SortUnionTransposeRule.java @@ -23,6 +23,7 @@ import org.apache.calcite.rel.core.Union; import org.apache.calcite.rel.metadata.RelMdUtil; import org.apache.calcite.rel.metadata.RelMetadataQuery; +import org.apache.calcite.rex.RexNode; import org.apache.calcite.rex.RexUtil; import org.apache.calcite.tools.RelBuilderFactory; @@ -67,12 +68,14 @@ public SortUnionTransposeRule( @Override public boolean matches(RelOptRuleCall call) { final Sort sort = call.rel(0); final Union union = call.rel(1); - // Re-evaluating a non-deterministic FETCH in every branch can produce a - // different limit from the top Sort. + // Re-evaluating a non-deterministic OFFSET or FETCH in every branch can + // produce a different limit from the top Sort. // There is a flag indicating if this rule should be applied when // Sort.fetch is null. return union.all - && sort.offset == null + && (sort.offset == null + || sort.fetch != null + && RexUtil.isDeterministic(sort.offset)) && (sort.fetch == null || RexUtil.isDeterministic(sort.fetch)) && (config.matchNullFetch() || sort.fetch != null); @@ -81,6 +84,17 @@ public SortUnionTransposeRule( @Override public void onMatch(RelOptRuleCall call) { final Sort sort = call.rel(0); final Union union = call.rel(1); + // OFFSET cannot be pushed into each input independently. However, only + // the first OFFSET + FETCH rows of an input can contribute to the final + // result, so use that value as the input FETCH and retain the original + // OFFSET and FETCH in the top Sort. + final RexNode inputFetch; + if (sort.fetch == null || sort.offset == null) { + inputFetch = sort.fetch; + } else { + inputFetch = + RexUtil.makeOffsetFetchSum(sort.getCluster().getRexBuilder(), sort.offset, sort.fetch); + } List inputs = new ArrayList<>(); // Thus we use 'ret' as a flag to identify if we have finished pushing the // sort past a union. @@ -88,11 +102,11 @@ public SortUnionTransposeRule( final RelMetadataQuery mq = call.getMetadataQuery(); for (RelNode input : union.getInputs()) { if (!RelMdUtil.checkInputForCollationAndLimit(mq, input, - sort.getCollation(), sort.offset, sort.fetch)) { + sort.getCollation(), null, inputFetch)) { ret = false; Sort branchSort = sort.copy(sort.getTraitSet(), input, - sort.getCollation(), sort.offset, sort.fetch); + sort.getCollation(), null, inputFetch); inputs.add(branchSort); } else { inputs.add(input); diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java b/core/src/main/java/org/apache/calcite/rex/RexUtil.java index b592093a5aef..b7c30b7608f9 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java +++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java @@ -66,6 +66,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; import java.math.BigDecimal; +import java.math.RoundingMode; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -881,12 +882,49 @@ public static boolean containsDynamicParam(RexNode e) { /** Converts a FETCH expression result to its validated canonical representation. */ public static BigDecimal validateFetchValue(@Nullable Number value) { + return validateOffsetFetchValue(value, "FETCH"); + } + + /** Creates the FETCH needed when OFFSET and FETCH are pushed into an input. + * + *

Enumerable execution rounds OFFSET and FETCH independently to whole + * row counts. Therefore, the input must fetch + * {@code CEIL(offset) + CEIL(fetch)} rows rather than + * {@code CEIL(offset + fetch)} rows. The latter can be one row smaller when + * both values have a fractional part. */ + public static RexNode makeOffsetFetchSum(RexBuilder rexBuilder, + RexNode offset, RexNode fetch) { + if (offset instanceof RexLiteral && fetch instanceof RexLiteral) { + return rexBuilder.makeExactLiteral( + RexLiteral.bigDecimalValue(offset).setScale(0, RoundingMode.CEILING) + .add(RexLiteral.bigDecimalValue(fetch) + .setScale(0, RoundingMode.CEILING))); + } + return rexBuilder.makeCall(SqlStdOperatorTable.PLUS, + ceil(rexBuilder, offset), ceil(rexBuilder, fetch)); + } + + private static RexNode ceil(RexBuilder rexBuilder, RexNode node) { + if (node instanceof RexLiteral) { + return rexBuilder.makeExactLiteral( + RexLiteral.bigDecimalValue(node).setScale(0, RoundingMode.CEILING)); + } + return rexBuilder.makeCall(SqlStdOperatorTable.CEIL, node); + } + + /** Converts an OFFSET expression result to its validated canonical representation. */ + public static BigDecimal validateOffsetValue(@Nullable Number value) { + return validateOffsetFetchValue(value, "OFFSET"); + } + + private static BigDecimal validateOffsetFetchValue(@Nullable Number value, + String kind) { if (value == null) { - throw new IllegalArgumentException("FETCH expression evaluated to NULL"); + throw new IllegalArgumentException(kind + " expression evaluated to NULL"); } final BigDecimal decimal = NumberUtil.toBigDecimal(value); if (decimal.signum() < 0) { - throw new IllegalArgumentException("FETCH value " + value + throw new IllegalArgumentException(kind + " value " + value + " is out of range; expected a non-negative value"); } return decimal; @@ -895,28 +933,39 @@ public static BigDecimal validateFetchValue(@Nullable Number value) { /** Reduces a constant FETCH expression to a validated literal. */ public static @Nullable RexLiteral reduceFetchToLiteral( RelOptCluster cluster, RexNode fetch) { + return reduceOffsetFetchToLiteral(cluster, fetch, "FETCH"); + } + + /** Reduces a constant OFFSET expression to a validated literal. */ + public static @Nullable RexLiteral reduceOffsetToLiteral( + RelOptCluster cluster, RexNode offset) { + return reduceOffsetFetchToLiteral(cluster, offset, "OFFSET"); + } + + private static @Nullable RexLiteral reduceOffsetFetchToLiteral( + RelOptCluster cluster, RexNode node, String kind) { final RexLiteral literal; - if (fetch instanceof RexLiteral) { - literal = (RexLiteral) fetch; + if (node instanceof RexLiteral) { + literal = (RexLiteral) node; } else { - if (!isConstant(fetch) - || !isDeterministic(fetch) - || containsDynamicFunction(fetch) - || containsDynamicParam(fetch)) { + if (!isConstant(node) + || !isDeterministic(node) + || containsDynamicFunction(node) + || containsDynamicParam(node)) { return null; } final RexExecutor executor = Util.first(cluster.getPlanner().getExecutor(), EXECUTOR); final List reducedValues = new ArrayList<>(1); executor.reduce(cluster.getRexBuilder(), - Collections.singletonList(fetch), reducedValues); + Collections.singletonList(node), reducedValues); final RexNode reduced = reducedValues.get(0); if (!(reduced instanceof RexLiteral)) { return null; } literal = (RexLiteral) reduced; } - validateFetchValue(literal.getValueAs(Number.class)); + validateOffsetFetchValue(literal.getValueAs(Number.class), kind); return literal; } 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 e5932b63768d..6252728bf799 100644 --- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java +++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java @@ -164,14 +164,16 @@ ExInstWithCause validatorContext(int a0, int a1, @BaseMessage("Values passed to {0} operator must have compatible types") ExInst incompatibleValueType(String a0); - @BaseMessage("FETCH expression must have a numeric type; actual type is ''{0}''") - ExInst fetchExpressionMustBeNumeric(String type); + @BaseMessage("{0} expression must have a numeric type; actual type is ''{1}''") + ExInst offsetFetchExpressionMustBeNumeric(String kind, + String type); - @BaseMessage("FETCH expression cannot reference table column ''{0}''") - ExInst fetchExpressionCannotReferenceColumn(String column); + @BaseMessage("{0} expression cannot reference table column ''{1}''") + ExInst offsetFetchExpressionCannotReferenceColumn( + String kind, String column); - @BaseMessage("FETCH expression evaluated to NULL") - ExInst fetchExpressionEvaluatedToNull(); + @BaseMessage("{0} expression evaluated to NULL") + ExInst offsetFetchExpressionEvaluatedToNull(String kind); @BaseMessage("Values in expression list must have compatible types") ExInst incompatibleTypesInList(); diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java index 663aee63b0db..1cbf52298639 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java @@ -1071,7 +1071,7 @@ protected static void unparseFetchUsingAnsi(SqlWriter writer, @Nullable SqlNode final SqlWriter.Frame offsetFrame = writer.startList(SqlWriter.FrameTypeEnum.OFFSET); writer.keyword("OFFSET"); - offset.unparse(writer, -1, -1); + unparseOffsetExpression(writer, offset); writer.keyword("ROWS"); writer.endList(offsetFrame); } @@ -1081,24 +1081,36 @@ protected static void unparseFetchUsingAnsi(SqlWriter writer, @Nullable SqlNode writer.startList(SqlWriter.FrameTypeEnum.FETCH); writer.keyword("FETCH"); writer.keyword("NEXT"); - if (fetch instanceof SqlLiteral - || fetch instanceof SqlDynamicParam) { - fetch.unparse(writer, -1, -1); - } else { - final SqlWriter.Frame expressionFrame = writer.startList("(", ")"); - if (fetch instanceof SqlCall) { - writer.getDialect().unparseCall(writer, (SqlCall) fetch, 0, 0); - } else { - fetch.unparse(writer, 0, 0); - } - writer.endList(expressionFrame); - } + unparseOffsetFetchExpression(writer, fetch); writer.keyword("ROWS"); writer.keyword("ONLY"); writer.endList(fetchFrame); } } + private static void unparseOffsetExpression(SqlWriter writer, SqlNode offset) { + if (offset instanceof SqlCall) { + writer.getDialect().unparseCall(writer, (SqlCall) offset, 0, 0); + } else { + offset.unparse(writer, 0, 0); + } + } + + private static void unparseOffsetFetchExpression(SqlWriter writer, SqlNode node) { + if (node instanceof SqlLiteral + || node instanceof SqlDynamicParam) { + node.unparse(writer, -1, -1); + } else { + final SqlWriter.Frame expressionFrame = writer.startList("(", ")"); + if (node instanceof SqlCall) { + writer.getDialect().unparseCall(writer, (SqlCall) node, 0, 0); + } else { + node.unparse(writer, 0, 0); + } + writer.endList(expressionFrame); + } + } + /** Unparses offset/fetch using "LIMIT fetch OFFSET offset" syntax. */ protected static void unparseFetchUsingLimit(SqlWriter writer, @Nullable SqlNode offset, @Nullable SqlNode fetch) { @@ -1106,12 +1118,12 @@ protected static void unparseFetchUsingLimit(SqlWriter writer, @Nullable SqlNode } /** Unparses offset/fetch using "LIMIT fetch OFFSET offset" syntax, - * optionally allowing a scalar expression as fetch. */ + * optionally allowing scalar expressions as fetch and offset. */ protected static void unparseFetchUsingLimit(SqlWriter writer, @Nullable SqlNode offset, @Nullable SqlNode fetch, boolean allowExpression) { checkArgument(fetch != null || offset != null); unparseLimit(writer, fetch, allowExpression); - unparseOffset(writer, offset); + unparseOffset(writer, offset, allowExpression); } protected static void unparseLimit(SqlWriter writer, @Nullable SqlNode fetch) { @@ -1138,7 +1150,19 @@ private static void unparseLimit(SqlWriter writer, @Nullable SqlNode fetch, } protected static void unparseOffset(SqlWriter writer, @Nullable SqlNode offset) { + unparseOffset(writer, offset, false); + } + + private static void unparseOffset(SqlWriter writer, @Nullable SqlNode offset, + boolean allowExpression) { if (offset != null) { + if (!allowExpression + && !(offset instanceof SqlLiteral) + && !(offset instanceof SqlDynamicParam)) { + throw new IllegalArgumentException( + "LIMIT dialect does not support OFFSET expressions that cannot " + + "be reduced to a literal"); + } writer.newlineAndIndent(); final SqlWriter.Frame offsetFrame = writer.startList(SqlWriter.FrameTypeEnum.OFFSET); diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java index 7b4475283ea0..6432058d5f7f 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java @@ -137,7 +137,15 @@ public MysqlSqlDialect(Context context) { @Override public void unparseOffsetFetch(SqlWriter writer, @Nullable SqlNode offset, @Nullable SqlNode fetch) { - unparseFetchUsingLimit(writer, offset, fetch); + if (offset != null && fetch == null) { + // MySQL has no OFFSET-only syntax. Its documented unlimited-row form + // uses the maximum unsigned BIGINT value as LIMIT. + final SqlNode unlimited = + SqlLiteral.createExactNumeric("18446744073709551615", SqlParserPos.ZERO); + unparseFetchUsingLimit(writer, offset, unlimited); + } else { + unparseFetchUsingLimit(writer, offset, fetch); + } } @Override public @Nullable SqlNode emulateNullDirection(SqlNode node, diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/SqliteSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/SqliteSqlDialect.java index f31276413600..833ac5c10d96 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/SqliteSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/SqliteSqlDialect.java @@ -90,7 +90,14 @@ public SqliteSqlDialect(SqlDialect.Context context) { @Override public void unparseOffsetFetch(SqlWriter writer, @Nullable SqlNode offset, @Nullable SqlNode fetch) { - unparseFetchUsingLimit(writer, offset, fetch, true); + if (offset != null && fetch == null) { + // SQLite has no OFFSET-only syntax. LIMIT -1 means no upper bound. + final SqlNode unlimited = + SqlLiteral.createExactNumeric("-1", SqlParserPos.ZERO); + unparseFetchUsingLimit(writer, offset, unlimited, true); + } else { + unparseFetchUsingLimit(writer, offset, fetch, true); + } } @Override public void unparseCall(SqlWriter writer, SqlCall call, diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java index 02a044820f49..3ca0325a931d 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java @@ -1771,31 +1771,34 @@ private void handleOffsetFetch(@Nullable SqlNode offset, @Nullable SqlNode fetch } } - private void validateFetchExpression(@Nullable SqlNode fetch) { - if (fetch == null || fetch instanceof SqlDynamicParam) { + private void validateOffsetFetchExpression(@Nullable SqlNode node, + String kind) { + if (node == null || node instanceof SqlDynamicParam) { return; } - if (SqlUtil.isNullLiteral(fetch, true)) { - throw newValidationError(fetch, - RESOURCE.fetchExpressionEvaluatedToNull()); + if (SqlUtil.isNullLiteral(node, true)) { + throw newValidationError(node, + RESOURCE.offsetFetchExpressionEvaluatedToNull(kind)); } - validateNoAggs(aggOrOverFinder, fetch, "FETCH"); - fetch.accept(new SqlBasicVisitor() { + validateNoAggs(aggOrOverFinder, node, kind); + node.accept(new SqlBasicVisitor() { @Override public Void visit(SqlIdentifier id) { if (makeNullaryCall(id) != null) { return null; } throw newValidationError(id, - RESOURCE.fetchExpressionCannotReferenceColumn(id.toString())); + RESOURCE.offsetFetchExpressionCannotReferenceColumn(kind, + id.toString())); } }); final SqlValidatorScope scope = getEmptyScope(); - inferUnknownTypes(typeFactory.createSqlType(SqlTypeName.DECIMAL), scope, fetch); - validateExpr(fetch, scope); - final RelDataType type = getValidatedNodeType(fetch); + inferUnknownTypes(typeFactory.createSqlType(SqlTypeName.DECIMAL), scope, node); + validateExpr(node, scope); + final RelDataType type = getValidatedNodeType(node); if (!SqlTypeUtil.isNumeric(type)) { - throw newValidationError(fetch, - RESOURCE.fetchExpressionMustBeNumeric(type.getFullTypeString())); + throw newValidationError(node, + RESOURCE.offsetFetchExpressionMustBeNumeric(kind, + type.getFullTypeString())); } } @@ -4497,7 +4500,8 @@ protected void validateSelect( validateWindowClause(select); validateQualifyClause(select); handleOffsetFetch(select.getOffset(), select.getFetch()); - validateFetchExpression(select.getFetch()); + validateOffsetFetchExpression(select.getOffset(), "OFFSET"); + validateOffsetFetchExpression(select.getFetch(), "FETCH"); // Validate the SELECT clause late, because a select item might // depend on the GROUP BY list, or the window function might reference diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java index b4999b455bef..b3fb6398d5ac 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java @@ -1142,10 +1142,14 @@ private static void shiftMapping(Map mapping, int startIndex, } static boolean canDecorrelateOffsetFetch(Sort sort) { + final @Nullable RexLiteral offset = sort.offset == null + ? null + : RexUtil.reduceOffsetToLiteral(sort.getCluster(), sort.offset); final @Nullable RexLiteral fetch = sort.fetch == null ? null : RexUtil.reduceFetchToLiteral(sort.getCluster(), sort.fetch); - return isNonNegativeIntegralLiteral(sort.offset) + return (sort.offset == null + || offset != null && isNonNegativeIntegralLiteral(offset)) && (sort.fetch == null || fetch != null && isNonNegativeIntegralLiteral(fetch)); } diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java index 2309102ff826..92c5a4141583 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -3801,29 +3801,14 @@ public RelBuilder sortLimit(Number offset, Number fetch, /** Creates a {@link Sort} by a list of expressions, with limitNode and offsetNode. * - * @param offsetNode RexLiteral means number of rows to skip is deterministic, - * RexDynamicParam means number of rows to skip is dynamic. + * @param offsetNode Number of rows to skip * @param fetchNode Maximum number of rows to fetch * @param nodes Sort expressions */ public RelBuilder sortLimit(@Nullable RexNode offsetNode, @Nullable RexNode fetchNode, Iterable nodes) { - if (offsetNode != null) { - if (!(offsetNode instanceof RexLiteral || offsetNode instanceof RexDynamicParam)) { - throw new IllegalArgumentException("OFFSET node must be RexLiteral or RexDynamicParam"); - } - } - if (fetchNode != null && !isValidFetchExpression(fetchNode)) { - throw new IllegalArgumentException( - "FETCH node must not reference input fields or contain aggregate functions, " - + "window functions, or subqueries"); - } - if (fetchNode != null - && !SqlTypeUtil.isNumeric(fetchNode.getType())) { - throw new IllegalArgumentException( - "FETCH node must have a numeric type; actual type is " - + fetchNode.getType().getFullTypeString()); - } + validateOffsetFetchExpression(offsetNode, "OFFSET"); + validateOffsetFetchExpression(fetchNode, "FETCH"); final Registrar registrar = new Registrar(fields(), ImmutableList.of()); final List fieldCollations = registrar.registerFieldCollations(nodes); @@ -3890,14 +3875,31 @@ public RelBuilder sortLimit(@Nullable RexNode offsetNode, @Nullable RexNode fetc return this; } - private static boolean isValidFetchExpression(RexNode node) { - return Boolean.TRUE.equals(node.accept(new FetchExpressionVisitor())); + private static void validateOffsetFetchExpression(@Nullable RexNode node, + String kind) { + if (node == null) { + return; + } + if (!isValidOffsetFetchExpression(node)) { + throw new IllegalArgumentException( + kind + " node must not reference input fields or contain aggregate functions, " + + "window functions, or subqueries"); + } + if (!SqlTypeUtil.isNumeric(node.getType())) { + throw new IllegalArgumentException( + kind + " node must have a numeric type; actual type is " + + node.getType().getFullTypeString()); + } + } + + private static boolean isValidOffsetFetchExpression(RexNode node) { + return Boolean.TRUE.equals(node.accept(new OffsetFetchExpressionVisitor())); } - /** Visitor that validates FETCH expressions. */ - private static class FetchExpressionVisitor + /** Visitor that validates OFFSET and FETCH expressions. */ + private static class OffsetFetchExpressionVisitor extends RexVisitorImpl<@Nullable Boolean> { - FetchExpressionVisitor() { + OffsetFetchExpressionVisitor() { super(false); } 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 19f3ef47a8d3..3f0100a10263 100644 --- a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties +++ b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties @@ -61,9 +61,9 @@ ValidatorContext=From line {0,number,#}, column {1,number,#} to line {2,number,# CannotCastValue=Cast function cannot convert value of type {0} to type {1} UnknownDatatypeName=Unknown datatype name ''{0}'' IncompatibleValueType=Values passed to {0} operator must have compatible types -FetchExpressionMustBeNumeric=FETCH expression must have a numeric type; actual type is ''{0}'' -FetchExpressionCannotReferenceColumn=FETCH expression cannot reference table column ''{0}'' -FetchExpressionEvaluatedToNull=FETCH expression evaluated to NULL +OffsetFetchExpressionMustBeNumeric={0} expression must have a numeric type; actual type is ''{1}'' +OffsetFetchExpressionCannotReferenceColumn={0} expression cannot reference table column ''{1}'' +OffsetFetchExpressionEvaluatedToNull={0} expression evaluated to NULL IncompatibleTypesInList=Values in expression list must have compatible types IncompatibleCharset=Cannot apply operation ''{0}'' to strings with different charsets ''{1}'' and ''{2}'' InvalidOrderByPos=ORDER BY is only allowed on top-level SELECT diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 44dc09078d02..1685bb3cae26 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -4931,6 +4931,17 @@ private SqlDialect nonOrdinalDialect() { sql(query).withMysql().ok(expected); } + @Test void testOffsetExpressionWithLimitDialect() { + final String query = "select \"product_id\"\n" + + "from \"product\"\n" + + "offset 1 + 2 rows"; + final String expected = "SELECT `product_id`\n" + + "FROM `foodmart`.`product`\n" + + "LIMIT 18446744073709551615\n" + + "OFFSET 3"; + sql(query).withMysql().ok(expected); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -4945,6 +4956,17 @@ private SqlDialect nonOrdinalDialect() { sql(query).withSQLite().throws_(error); } + @Test void testNegativeOffsetExpressionIsRejectedBeforeSqlGeneration() { + final String query = "select \"product_id\"\n" + + "from \"product\"\n" + + "offset 0 - 1 rows"; + final String error = + "OFFSET value -1 is out of range; expected a non-negative value"; + sql(query).throws_(error); + sql(query).withMysql().throws_(error); + sql(query).withSQLite().throws_(error); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -4957,6 +4979,15 @@ private SqlDialect nonOrdinalDialect() { + "be reduced to a literal"); } + @Test void testParameterizedOffsetExpressionWithLimitDialect() { + final String query = "select \"product_id\"\n" + + "from \"product\"\n" + + "offset ? + 1 rows"; + sql(query).withMysql().throws_( + "LIMIT dialect does not support OFFSET expressions that cannot " + + "be reduced to a literal"); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -4970,6 +5001,31 @@ private SqlDialect nonOrdinalDialect() { sql(query).withSQLite().ok(expected); } + @Test void testParameterizedOffsetExpressionWithSQLite() { + final String query = "select \"product_id\"\n" + + "from \"product\"\n" + + "offset ? + 1 rows"; + final String expected = "SELECT \"product_id\"\n" + + "FROM \"foodmart\".\"product\"\n" + + "LIMIT -1\n" + + "OFFSET ? + 1"; + sql(query).withSQLite().ok(expected); + } + + @Test void testDynamicOffsetExpressionIsNotReduced() { + final String query = "select \"product_id\"\n" + + "from \"product\"\n" + + "offset extract(day from current_date) rows"; + final String expected = "SELECT \"product_id\"\n" + + "FROM \"foodmart\".\"product\"\n" + + "OFFSET EXTRACT(DAY FROM CURRENT_DATE) ROWS"; + sql(query).ok(expected); + sql(query).withPostgresql().ok(expected); + sql(query).withMysql().throws_( + "LIMIT dialect does not support OFFSET expressions that cannot " + + "be reduced to a literal"); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java index bf506ceb121a..51b738017c30 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -3623,6 +3623,17 @@ private void assertTypeAndToString( containsString("FETCH value -1.5 is out of range")); } + @Test void testValidateOffsetValueAllowsFractionalBigDecimal() { + assertThat(RexUtil.validateOffsetValue(new BigDecimal("1.5")), + is(new BigDecimal("1.5"))); + + final IllegalArgumentException e = + assertThrows(IllegalArgumentException.class, + () -> RexUtil.validateOffsetValue(new BigDecimal("-1.5"))); + assertThat(e.getMessage(), + containsString("OFFSET value -1.5 is out of range")); + } + @Test void testConstantMap() { final RelDataType intType = typeFactory.createSqlType(SqlTypeName.INTEGER); final RelDataType bigintType = typeFactory.createSqlType(SqlTypeName.BIGINT); diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java index f213b12dcb7e..9385d4a28d1b 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java @@ -3593,6 +3593,17 @@ public void checkOrderBy(final boolean desc, + "X=3\n"); } + @Test void testOffsetExpression() { + final CalciteAssert.AssertThat with = CalciteAssert.that(); + final String values = "select * from (values (1), (2), (3), (4)) as t(x)\n"; + with.query(values + "offset 1 + abs(-1) rows") + .returns("X=3\n" + + "X=4\n"); + with.query(values + "order by x desc offset 1 + abs(-1) rows") + .returns("X=2\n" + + "X=1\n"); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -3618,6 +3629,17 @@ public void checkOrderBy(final boolean desc, } } + @Test void testBindableOffsetExpression() { + try (Hook.Closeable ignored = Hook.ENABLE_BINDABLE.addThread(Hook.propertyJ(true))) { + CalciteAssert.that() + .query("select * from (values (1), (2), (3), (4)) as t(x)\n" + + "offset rand_integer(1) + 2 rows") + .explainContains("BindableSort(offset=[+(RAND_INTEGER(1), 2)])") + .returns("X=3\n" + + "X=4\n"); + } + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -3632,6 +3654,15 @@ public void checkOrderBy(final boolean desc, + "X=2\n"); } + @Test void testOffsetExpressionFunctionArguments() { + final CalciteAssert.AssertThat with = CalciteAssert.that(); + final String values = "select * from (values (1), (2), (3)) as t(x)\n"; + with.query(values + "offset abs(2) rows") + .returns("X=3\n"); + with.query(values + "offset abs(-2) rows") + .returns("X=3\n"); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -3647,6 +3678,17 @@ public void checkOrderBy(final boolean desc, .throws_("FETCH expression evaluated to NULL"); } + @Test void testOffsetExpressionInvalidValue() { + final CalciteAssert.AssertThat with = CalciteAssert.that(); + final String values = "select * from (values (1), (2), (3)) as t(x)\n"; + with.query(values + "offset 0 - 1 rows") + .throws_("OFFSET must not be negative"); + with.query(values + "offset -1 rows") + .throws_("OFFSET must not be negative"); + with.query(values + "offset cast(null as integer) rows") + .throws_("OFFSET expression evaluated to NULL"); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -3681,6 +3723,9 @@ public void checkOrderBy(final boolean desc, with.query(sqlPrefix + "fetch next (0.5 + 1) rows only" + sqlSuffix) .returns("DNAME=Sales; ENAME=Bill\n" + "DNAME=Sales; ENAME=Theodore\n"); + with.query(sqlPrefix + "offset 0.5 + 1 rows fetch next 1 row only" + + sqlSuffix) + .returns("DNAME=Sales; ENAME=Sebastian\n"); with.query(sqlPrefix + "offset 1.5 rows fetch next 1 row only" + sqlSuffix) .returns("DNAME=Sales; ENAME=Sebastian\n"); } @@ -3690,20 +3735,24 @@ public void checkOrderBy(final boolean desc, * [CALCITE-7592] * Add expression support for FETCH. */ @Test void testCorrelatedPreparedFractionalOffset() throws Exception { - final String sql = "select d.\"name\" as dname, e.\"name\" as ename\n" - + "from \"hr\".\"depts\" d,\n" - + "lateral (select \"empid\", \"name\" from \"hr\".\"emps\"\n" - + " where \"deptno\" = d.\"deptno\"\n" - + " order by \"empid\" offset ? rows fetch next 1 row only) e\n" - + "order by e.\"empid\""; - for (boolean topDown : new boolean[] {false, true}) { - CalciteAssert.hr() - .with(CalciteConnectionProperty.TOPDOWN_GENERAL_DECORRELATION_ENABLED, topDown) - .doWithConnection(connection -> { - checkPreparedBigDecimalParameter(connection, sql, - new BigDecimal("1.5"), - "DNAME=Sales; ENAME=Sebastian\n"); - }); + for (String offset + : new String[] {"?", "cast(? as decimal(2, 1)) + 0"}) { + final String sql = "select d.\"name\" as dname, e.\"name\" as ename\n" + + "from \"hr\".\"depts\" d,\n" + + "lateral (select \"empid\", \"name\" from \"hr\".\"emps\"\n" + + " where \"deptno\" = d.\"deptno\"\n" + + " order by \"empid\" offset " + offset + + " rows fetch next 1 row only) e\n" + + "order by e.\"empid\""; + for (boolean topDown : new boolean[] {false, true}) { + CalciteAssert.hr() + .with(CalciteConnectionProperty.TOPDOWN_GENERAL_DECORRELATION_ENABLED, topDown) + .doWithConnection(connection -> { + checkPreparedBigDecimalParameter(connection, sql, + new BigDecimal("1.5"), + "DNAME=Sales; ENAME=Sebastian\n"); + }); + } } } @@ -3739,6 +3788,34 @@ public void checkOrderBy(final boolean desc, } } + @Test void testCorrelatedPreparedOffsetExpression() throws Exception { + for (String offset : new String[] {"?", "? + 0"}) { + final String sql = "select d.\"name\" as dname, e.\"name\" as ename\n" + + "from \"hr\".\"depts\" d,\n" + + "lateral (select \"empid\", \"name\" from \"hr\".\"emps\"\n" + + " where \"deptno\" = d.\"deptno\"\n" + + " order by \"empid\" offset " + offset + " rows) e\n" + + "order by e.\"empid\""; + for (boolean topDown : new boolean[] {false, true}) { + CalciteAssert.hr() + .with(CalciteConnectionProperty.TOPDOWN_GENERAL_DECORRELATION_ENABLED, topDown) + .doWithConnection(connection -> { + checkPreparedFetchRepeated(connection, sql, + new int[] {1, 2}, + new String[] { + "DNAME=Sales; ENAME=Theodore\n" + + "DNAME=Sales; ENAME=Sebastian\n", + "DNAME=Sales; ENAME=Sebastian\n" + }); + checkPreparedParameterFails(connection, sql, -1, + "OFFSET must not be negative"); + checkPreparedParameterNullFails(connection, sql, + "OFFSET expression evaluated to NULL"); + }); + } + } + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -3756,6 +3833,19 @@ public void checkOrderBy(final boolean desc, .returns(expected); } + @Test void testOffsetExpressionBeyondLong() { + final CalciteAssert.AssertThat with = CalciteAssert.that(); + final String values = "select * from (values (1), (2), (3), (4)) as t(x)\n"; + with.query(values + "offset 9223372036854775808 rows") + .returns(""); + with.query(values + "offset " + + "cast(9223372036854775808 as decimal(20, 0)) + 1 rows") + .returns(""); + with.query(values + "order by x offset " + + "cast(9223372036854775808 as decimal(20, 0)) + 1 rows") + .returns(""); + } + /** Tests ORDER BY ... OFFSET ... FETCH. */ @Test void testOrderByOffsetFetch() { CalciteAssert.that() @@ -6277,6 +6367,34 @@ private CalciteAssert.AssertQuery withEmpDept(String sql) { }); } + @Test void testPreparedOffsetExpression() throws Exception { + CalciteAssert.that() + .doWithConnection(connection -> { + final String values = + "select * from (values (1), (2), (3), (4)) as t(x)\n"; + checkPreparedFetch(connection, values + "offset ? + 1 rows", + 1, "X=3\nX=4\n"); + checkPreparedFetch(connection, + values + "order by x desc offset ? + 1 rows", + 1, "X=2\nX=1\n"); + checkPreparedBigDecimalParameter(connection, + values + "offset cast(? as decimal(2, 1)) + 0 rows", + new BigDecimal("1.5"), "X=3\nX=4\n"); + checkPreparedFetch(connection, + values + "offset abs(cast(? as integer)) rows", + -2, "X=3\nX=4\n"); + checkPreparedBigDecimalParameter(connection, + values + "offset cast(? as decimal(20, 0)) rows", + new BigDecimal("9223372036854775808"), ""); + checkPreparedParameterFails(connection, + values + "offset ? + 1 rows", -2, + "OFFSET must not be negative"); + checkPreparedParameterNullFails(connection, + values + "offset ? + 1 rows", + "OFFSET expression evaluated to NULL"); + }); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -6311,6 +6429,9 @@ private CalciteAssert.AssertQuery withEmpDept(String sql) { .doWithConnection(connection -> { final String values = "select * from (values (1), (2), (3), (4)) as t(x)\n"; + checkPreparedFetch(connection, + values + "offset ? + 1 rows", + 1, "X=3\nX=4\n"); final String offset = values + "offset ? rows"; checkPreparedBigDecimalParameter(connection, offset, new BigDecimal("1.5"), diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java index 30aa2ff77fdf..d7d6e7f963bc 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -5600,6 +5600,19 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build ImmutableList.of())); } + @Test void testOffsetExpressionCannotReferenceInputField() { + final RelBuilder builder = RelBuilder.create(config().build()); + builder.scan("DEPT"); + final RexNode field = builder.field("DEPTNO"); + + assertThrows(IllegalArgumentException.class, + () -> builder.sortLimit(field, null, ImmutableList.of())); + assertThrows(IllegalArgumentException.class, + () -> builder.sortLimit( + builder.call(SqlStdOperatorTable.PLUS, builder.literal(1), field), + null, ImmutableList.of())); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -5613,6 +5626,16 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build ImmutableList.of()); } + @Test void testOffsetExpressionMustHaveNumericType() { + final RelBuilder builder = RelBuilder.create(config().build()); + builder.scan("DEPT"); + + assertThrows(IllegalArgumentException.class, + () -> builder.sortLimit(builder.literal("x"), null, ImmutableList.of())); + builder.sortLimit(builder.literal(new BigDecimal("1.5")), null, + ImmutableList.of()); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -5632,6 +5655,22 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build + " LogicalTableScan(table=[[scott, DEPT]])\n")); } + @Test void testOffsetExpressionAllowsScalarCallAndDynamicParameter() { + final RelBuilder builder = RelBuilder.create(config().build()); + final RelDataType intType = + builder.getTypeFactory().createSqlType(SqlTypeName.INTEGER); + builder.scan("DEPT") + .sortLimit( + builder.call(SqlStdOperatorTable.PLUS, + builder.getRexBuilder().makeDynamicParam(intType, 0), + builder.literal(1)), + null, ImmutableList.of()); + + assertThat( + builder.build(), hasTree("LogicalSort(offset=[+(?0, 1)])\n" + + " LogicalTableScan(table=[[scott, DEPT]])\n")); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ @@ -5662,6 +5701,33 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build () -> builder.sortLimit(null, subQuery, ImmutableList.of())); } + @Test void testOffsetExpressionCannotContainAggregateWindowOrSubQuery() { + final RelBuilder builder = RelBuilder.create(config().build()); + final RelDataType intType = + builder.getTypeFactory().createSqlType(SqlTypeName.INTEGER); + builder.scan("DEPT"); + final RexNode aggregate = + builder.call(SqlStdOperatorTable.SUM, builder.literal(1)); + assertThrows(IllegalArgumentException.class, + () -> builder.sortLimit(aggregate, null, ImmutableList.of())); + + final RexNode over = + builder.getRexBuilder().makeOver(intType, + SqlStdOperatorTable.ROW_NUMBER, ImmutableList.of(), + ImmutableList.of(), ImmutableList.of(), + RexWindowBounds.UNBOUNDED_PRECEDING, + RexWindowBounds.UNBOUNDED_FOLLOWING, + true, true, false, false, false); + assertThrows(IllegalArgumentException.class, + () -> builder.sortLimit(over, null, ImmutableList.of())); + + final RelBuilder subQueryBuilder = RelBuilder.create(config().build()); + final RexNode subQuery = + RexSubQuery.scalar(subQueryBuilder.values(new String[] {"N"}, 1).build()); + assertThrows(IllegalArgumentException.class, + () -> builder.sortLimit(subQuery, null, ImmutableList.of())); + } + /** Test case for * [CALCITE-7592] * Add expression support for FETCH. */ diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java index 460e066051fe..7232c40bcdd8 100644 --- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java @@ -1527,6 +1527,34 @@ void testColumnOriginsUnion() { .assertThatRowCount(is(2D), is(0D), is(2D)); } + @Test void testMinRowCountOffsetExpression() { + final String sql = "select * from (values (1), (2)) as t(x)\n" + + "offset 2 - 2 rows"; + final RelMetadataFixture fixture = sql(sql); + fixture.assertThatRowCount(is(2D), is(0D), is(2D)); + + fixture + .withCluster(cluster -> { + final RelOptPlanner planner = new VolcanoPlanner(); + planner.addRule(EnumerableRules.ENUMERABLE_VALUES_RULE); + planner.addRule(EnumerableRules.ENUMERABLE_PROJECT_RULE); + planner.addRule(EnumerableRules.ENUMERABLE_LIMIT_RULE); + planner.addRelTraitDef(ConventionTraitDef.INSTANCE); + return RelOptCluster.create(planner, cluster.getRexBuilder()); + }) + .withRelTransform(rel -> { + final RelOptPlanner planner = rel.getCluster().getPlanner(); + planner.setRoot(rel); + final RelTraitSet requiredOutputTraits = + rel.getCluster().traitSet().replace(EnumerableConvention.INSTANCE); + final RelNode root = planner.changeTraits(rel, requiredOutputTraits); + planner.setRoot(root); + return planner.findBestExp(); + }) + .assertThatRel(is(instanceOf(EnumerableLimit.class))) + .assertThatRowCount(is(2D), is(0D), is(2D)); + } + @Test void testRowCountSortLimitOffset() { final String sql = "select * from emp order by ename limit 10 offset 5"; /* 14 - 5 */ diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index 52c1875e3696..8e7e2945f14a 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -1766,6 +1766,39 @@ private void checkJoinProjectTransposeDoesNotMatch(JoinRelType type) { .check(); } + @Test void testSortUnionTransposePushesLiteralOffset() { + final String sql = "select a.name from dept a\n" + + "union all\n" + + "select b.name from dept b\n" + + "order by name offset 2 rows fetch next 3 rows only"; + sql(sql) + .withPreRule(CoreRules.PROJECT_SET_OP_TRANSPOSE) + .withRule(CoreRules.SORT_UNION_TRANSPOSE) + .check(); + } + + @Test void testSortUnionTransposePushesParameterizedOffsetExpression() { + final String sql = "select a.name from dept a\n" + + "union all\n" + + "select b.name from dept b\n" + + "order by name offset ? + 1 rows fetch next 2 rows only"; + sql(sql) + .withPreRule(CoreRules.PROJECT_SET_OP_TRANSPOSE) + .withRule(CoreRules.SORT_UNION_TRANSPOSE) + .check(); + } + + @Test void testSortUnionTransposeWithNonDeterministicOffset() { + final String sql = "select a.name from dept a\n" + + "union all\n" + + "select b.name from dept b\n" + + "order by name offset rand_integer(10) rows fetch next 2 rows only"; + sql(sql) + .withPreRule(CoreRules.PROJECT_SET_OP_TRANSPOSE) + .withRule(CoreRules.SORT_UNION_TRANSPOSE) + .checkUnchanged(); + } + @Test void testSortRemovalAllKeysConstant() { final String sql = "select count(*) as c\n" + "from sales.emp\n" @@ -12506,6 +12539,22 @@ private void checkNondeterministicFetchPreventsDecorrelation(boolean enableTopDo .check(); } + @Test void testTopDownGeneralDecorrelateForSubqueryWithOffsetExpression() { + final String sql = "select empno from emp where " + + "sal > SOME(select sal from emp_b where emp.deptno = emp_b.deptno " + + "order by emp_b.sal offset 1 + 1 rows " + + "fetch next (1 + 1) rows only)"; + + sql(sql) + .withRule( + CoreRules.FILTER_SUB_QUERY_TO_MARK_CORRELATE, + CoreRules.PROJECT_MERGE, + CoreRules.PROJECT_REMOVE) + .withLateDecorrelate(true) + .withTopDownGeneralDecorrelate(true) + .check(); + } + @Test void testTopDownGeneralDecorrelateForSubqueryWithCube() { final String sql = "select empno from emp where " + "sal < SOME(select avg(sal) from emp_b where emp.job = emp_b.job group by cube(deptno))"; 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 6ce401502c93..ec34053df9e6 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -1253,6 +1253,12 @@ public static void checkActualAndReferenceFiles() { sql(sql).ok(); } + @Test void testOffsetWithExpression() { + final String sql = + "select empno from emp offset 1 + abs(-2) rows"; + sql(sql).ok(); + } + @Test void testFetch() { final String sql = "select empno from emp fetch next 5 rows only"; sql(sql).ok(); 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 9b0d0de70221..eca160ce491c 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -10723,6 +10723,22 @@ void testGroupExpressionEquivalenceParams() { .fails("Windowed aggregate expression is illegal in FETCH clause"); } + @Test void testOffsetExpressionType() { + sql("select name from dept offset ^upper('x')^ rows") + .fails("OFFSET expression must have a numeric type; " + + "actual type is 'CHAR\\(1\\) NOT NULL'"); + sql("select name from dept offset ^'x'^ rows") + .fails("OFFSET expression must have a numeric type; " + + "actual type is 'CHAR\\(1\\) NOT NULL'"); + sql("select name from dept offset 1.5 rows").ok(); + sql("select name from dept offset ^deptno^ rows") + .fails("OFFSET expression cannot reference table column 'DEPTNO'"); + sql("select name from dept offset ^cast(null as integer)^ rows") + .fails("OFFSET expression evaluated to NULL"); + sql("select name from dept offset ^row_number() over ()^ rows") + .fails("Windowed aggregate expression is illegal in OFFSET clause"); + } + @Test void testRewriteWithOffsetWithoutOrderBy() { final String sql = "select name from dept offset 2"; final String expected = "SELECT `NAME`\n" @@ -10739,14 +10755,12 @@ void testGroupExpressionEquivalenceParams() { @Test void testNegativeFetchOffsetLimit() { sql("select name from dept limit ^-^1") .fails("(?s).*Encountered \"-\".*"); - sql("select name from dept offset ^-^1") - .fails("(?s).*Encountered \"-\".*"); + sql("select name from dept offset -1").ok(); sql("select name from dept fetch next ^-^1 rows only") .fails("(?s).*Encountered \"-\".*"); sql("select name from dept order by name limit ^-^1") .fails("(?s).*Encountered \"-\".*"); - sql("select name from dept order by name offset ^-^1") - .fails("(?s).*Encountered \"-\".*"); + sql("select name from dept order by name offset -1").ok(); sql("select name from dept order by name fetch next ^-^1 rows only") .fails("(?s).*Encountered \"-\".*"); } diff --git a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java index 44055f707462..1fb766223ea2 100644 --- a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java +++ b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test; +import java.math.BigDecimal; import java.util.function.Consumer; /** @@ -105,7 +106,74 @@ class EnumerableMergeUnionTest { .explainContains("EnumerableLimit(fetch=[+(?0, 1)])\n" + " EnumerableMergeUnion(all=[true])\n" + " EnumerableLimitSort(sort0=[$0], dir0=[ASC], " - + "fetch=[+(?0, 1)])\n"); + + "fetch=[+(?0, 1)])\n") + .consumesPreparedStatement(p -> p.setInt(1, 1)) + .returnsOrdered( + "empid=1; name=Bill", + "empid=1; name=Bill"); + } + + @Test void mergeUnionPushesParameterizedOffsetExpression() { + tester(false, + new HrSchemaBig(), + "select * from (select empid, name from emps " + + "union all select empid, name from emps) " + + "order by empid offset ? + 1 rows fetch next 2 rows only") + .explainContains("EnumerableLimit(offset=[+(?0, 1)], fetch=[2])\n" + + " EnumerableMergeUnion(all=[true])\n" + + " EnumerableLimitSort(sort0=[$0], dir0=[ASC], " + + "fetch=[+(CEIL(+(?0, 1)), 2)])\n") + .consumesPreparedStatement(p -> p.setInt(1, 1)) + .returnsOrdered( + "empid=2; name=Eric", + "empid=2; name=Eric"); + } + + @Test void mergeUnionRoundsOffsetAndFetchSeparatelyWhenPushingLimit() { + tester(false, + new HrSchemaBig(), + "select * from (select empid from emps where empid <= 3 " + + "union all select empid from emps where empid >= 40) " + + "order by empid offset ? rows fetch next ? rows only") + .explainContains("EnumerableLimit(offset=[?0], fetch=[?1])\n" + + " EnumerableMergeUnion(all=[true])\n" + + " EnumerableLimitSort(sort0=[$0], dir0=[ASC], " + + "fetch=[+(CEIL(?0), CEIL(?1))])\n") + .consumesPreparedStatement(p -> { + p.setBigDecimal(1, new BigDecimal("0.5")); + p.setBigDecimal(2, new BigDecimal("0.5")); + }) + .returnsOrdered("empid=2"); + } + + @Test void mergeUnionDistinctDoesNotPushLimitBeforeDeduplication() { + tester(false, + new HrSchemaBig(), + "select deptno from emps where empid <= 3 " + + "union select deptno from emps where empid between 40 and 41 " + + "order by deptno offset ? rows fetch next ? rows only") + .explainContains("EnumerableLimitSort(sort0=[$0], dir0=[ASC], " + + "offset=[?0], fetch=[?1])\n" + + " EnumerableMergeUnion(all=[false])\n" + + " EnumerableSort(sort0=[$0], dir0=[ASC])\n") + .consumesPreparedStatement(p -> { + p.setBigDecimal(1, new BigDecimal("0.5")); + p.setBigDecimal(2, new BigDecimal("0.5")); + }) + .returnsOrdered("deptno=20"); + } + + @Test void mergeUnionDoesNotPushNonDeterministicOffset() { + tester(false, + new HrSchemaBig(), + "select * from (select empid, name from emps " + + "union all select empid, name from emps) " + + "order by empid offset rand_integer(10) rows " + + "fetch next 2 rows only") + .explainContains("EnumerableLimitSort(sort0=[$0], dir0=[ASC], " + + "offset=[RAND_INTEGER(10)], fetch=[2])\n" + + " EnumerableMergeUnion(all=[true])\n" + + " EnumerableSort(sort0=[$0], dir0=[ASC])\n"); } @Test void mergeUnionAllOrderByName() { @@ -176,16 +244,14 @@ class EnumerableMergeUnionTest { tester(false, new HrSchemaBig(), "select * from (select empid, name from emps where name like 'G%' union select empid, name from emps where name like '%l') order by name limit 3") - .explainContains("EnumerableLimit(fetch=[3])\n" + .explainContains("EnumerableLimitSort(sort0=[$1], dir0=[ASC], fetch=[3])\n" + " EnumerableMergeUnion(all=[false])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" - + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[3])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" - + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[3])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n") + + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n" + + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n") .returnsOrdered( "empid=29; name=Anibal", "empid=1; name=Bill", @@ -218,14 +284,12 @@ class EnumerableMergeUnionTest { "select * from (select empid, name from emps where name like 'G%' union select empid, name from emps where name like '%l') order by name limit 3 offset 2") .explainContains("EnumerableLimit(offset=[2], fetch=[3])\n" + " EnumerableMergeUnion(all=[false])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" - + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[5])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" - + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[5])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n") + + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n" + + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n") .returnsOrdered( "empid=40; name=Emmanuel", "empid=10; name=Gabriel", diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml index e2b725a7d66d..2b110fab3bb5 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -20029,6 +20029,36 @@ LogicalSort(sort0=[$0], dir0=[ASC], fetch=[0]) LogicalSort(sort0=[$0], dir0=[ASC], fetch=[0]) LogicalProject(NAME=[$1]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + + + + + + + + + + + @@ -20059,6 +20089,36 @@ LogicalSort(sort0=[$0], dir0=[ASC], fetch=[+(?0, 1)]) LogicalSort(sort0=[$0], dir0=[ASC], fetch=[+(?0, 1)]) LogicalProject(NAME=[$1]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + + + + + + + + + + + @@ -20077,6 +20137,24 @@ LogicalSort(sort0=[$0], dir0=[ASC], fetch=[RAND_INTEGER(10)]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) LogicalProject(NAME=[$1]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + + + + + + + + @@ -21092,6 +21170,45 @@ LogicalProject(EMPNO=[$0]) LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]]) LogicalProject(EMPNO=[$0], DEPTNO=[$7]) LogicalTableScan(table=[[CATALOG, SALES, EMP_B]]) +]]> + + + + + SOME(select sal from emp_b where emp.deptno = emp_b.deptno order by emp_b.sal offset 1 + 1 rows fetch next (1 + 1) rows only)]]> + + + SOME($5, { +LogicalSort(sort0=[$0], dir0=[ASC], offset=[+(1, 1)], fetch=[+(1, 1)]) + LogicalProject(SAL=[$5]) + LogicalFilter(condition=[=($cor0.DEPTNO, $7)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP_B]]) +})], variablesSet=[[$cor0]]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + + + ($5, $9)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalSort(sort0=[$0], dir0=[ASC], offset=[+(1, 1)], fetch=[+(1, 1)]) + LogicalProject(SAL=[$5]) + LogicalFilter(condition=[=($cor0.DEPTNO, $7)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP_B]]) +]]> + + + ($5, $9), IS NOT DISTINCT FROM($7, $10))], joinType=[semi]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalFilter(condition=[AND(>($2, +(1, 1)), <=($2, +(+(1, 1), +(1, 1))))]) + LogicalProject(SAL=[$5], DEPTNO=[$7], $f2=[ROW_NUMBER() OVER (PARTITION BY $7 ORDER BY $5 NULLS LAST)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP_B]]) ]]> 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 1aa3c3f66134..5f1c7f2ee819 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -6441,6 +6441,18 @@ LogicalSort(offset=[?0], fetch=[?1]) LogicalSort(offset=[?0]) LogicalProject(EMPNO=[$0]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + + + + + + + + diff --git a/core/src/test/resources/sql/offset.iq b/core/src/test/resources/sql/offset.iq new file mode 100644 index 000000000000..3f4769132184 --- /dev/null +++ b/core/src/test/resources/sql/offset.iq @@ -0,0 +1,186 @@ +# offset.iq +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +!use post +!set outputformat mysql + +# OFFSET accepts an arithmetic expression without parentheses. +select * +from (values (1), (2), (3), (4)) as t(x) +offset 1 + abs(-1) rows; ++---+ +| X | ++---+ +| 3 | +| 4 | ++---+ +(2 rows) + +!ok + +# OFFSET also accepts a parenthesized scalar expression. +select * +from (values (1), (2), (3), (4)) as t(x) +offset (abs(2)) rows; ++---+ +| X | ++---+ +| 3 | +| 4 | ++---+ +(2 rows) + +!ok + +# OFFSET values are not restricted to the BIGINT range. +select * +from (values (1), (2), (3), (4)) as t(x) +offset cast(9223372036854775808 as decimal(20, 0)) + 1 rows; ++---+ +| X | ++---+ ++---+ +(0 rows) + +!ok + +# OFFSET expression cannot be negative. +select * +from (values (1), (2), (3)) as t(x) +offset 0 - 1 rows; +OFFSET must not be negative +!error + +# OFFSET expression cannot evaluate to NULL. +select * +from (values (1), (2), (3)) as t(x) +offset cast(null as integer) rows; +OFFSET expression evaluated to NULL +!error + +# OFFSET expression may have a fractional numeric type. +select * +from (values (1), (2), (3)) as t(x) +offset 1.5 rows; ++---+ +| X | ++---+ +| 3 | ++---+ +(1 row) + +!ok + +# OFFSET expression cannot reference input columns. +select * +from (values (1), (2), (3)) as t(x) +offset x rows; +OFFSET expression cannot reference table column 'X' +!error + +# Parentheses are not required around an OFFSET expression. +select * +from (values (1), (2), (3)) as t(x) +offset 1 + 1 rows; ++---+ +| X | ++---+ +| 3 | ++---+ +(1 row) + +!ok + +# OFFSET expression works with a table source. +select deptno, dname +from dept +order by deptno +offset 1 + 1 rows; ++--------+-------------+ +| DEPTNO | DNAME | ++--------+-------------+ +| 30 | Engineering | +| 40 | Empty | ++--------+-------------+ +(2 rows) + +!ok + +# OFFSET expression works together with FETCH on a table source. +select deptno, dname +from dept +order by deptno +offset 1 + 1 rows +fetch next (1 + 1) rows only; ++--------+-------------+ +| DEPTNO | DNAME | ++--------+-------------+ +| 30 | Engineering | +| 40 | Empty | ++--------+-------------+ +(2 rows) + +!ok + +# OFFSET expression may contain a scalar function on a table source. +select deptno +from dept +order by deptno +offset abs(-2) rows; ++--------+ +| DEPTNO | ++--------+ +| 30 | +| 40 | ++--------+ +(2 rows) + +!ok + +# OFFSET expression cannot reference columns of a table source. +select deptno, dname +from dept +order by deptno +offset deptno rows; +OFFSET expression cannot reference table column 'DEPTNO' +!error + +# OFFSET expression cannot reference columns even inside a larger expression. +select deptno, dname +from dept +order by deptno +offset deptno + 1 rows; +OFFSET expression cannot reference table column 'DEPTNO' +!error + +# OFFSET expression may be zero on a table source. +select deptno +from dept +order by deptno +offset 2 - 2 rows; ++--------+ +| DEPTNO | ++--------+ +| 10 | +| 20 | +| 30 | +| 40 | ++--------+ +(4 rows) + +!ok diff --git a/server/src/test/java/org/apache/calcite/test/ServerTest.java b/server/src/test/java/org/apache/calcite/test/ServerTest.java index 39d434f23ae9..0d6b4520b14c 100644 --- a/server/src/test/java/org/apache/calcite/test/ServerTest.java +++ b/server/src/test/java/org/apache/calcite/test/ServerTest.java @@ -489,6 +489,28 @@ static Connection connect() throws SQLException { } } + @Test void testOffsetExpressionCannotReferenceInputColumn() throws Exception { + try (Connection c = connect(); + Statement s = c.createStatement()) { + s.execute("create table person (id int not null, name varchar(20))"); + try (PreparedStatement p = + c.prepareStatement("insert into person (id, name) values (?, ?)")) { + p.setInt(1, 1); + p.setString(2, "foo"); + assertThat(p.executeUpdate(), is(1)); + } + + for (String offset : new String[] {"id", "(id)", "1 + id"}) { + final SQLException e = + assertThrows( + SQLException.class, () -> s.executeQuery("select * from person " + + "offset " + offset + " rows")); + assertThat(e.getMessage(), + containsString("OFFSET expression cannot reference table column 'ID'")); + } + } + } + /** Test case for * [CALCITE-6022] * Support "CREATE TABLE ... LIKE" DDL in server module. */ diff --git a/site/_docs/reference.md b/site/_docs/reference.md index b1f67eeef1ee..04f0fa08de41 100644 --- a/site/_docs/reference.md +++ b/site/_docs/reference.md @@ -193,8 +193,8 @@ query: } [ ORDER BY { ALL [ ASC | DESC ] [ NULLS FIRST | NULLS LAST ] | orderItem [, orderItem]* } ] [ LIMIT [ start, ] { count | ALL } ] - [ OFFSET start { ROW | ROWS } ] - [ FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } ONLY ] + [ OFFSET { start | expression } { ROW | ROWS } ] + [ FETCH { FIRST | NEXT } [ count | '(' expression ')' ] { ROW | ROWS } ONLY ] withItem: name @@ -215,7 +215,7 @@ select: [ WINDOW windowName AS windowSpec [, windowName AS windowSpec ]* ] [ QUALIFY booleanExpression ] [ ORDER BY orderItem [, orderItem ]* ] - [ LIMIT expression [ OFFSET expression ] ] + [ LIMIT expression [ OFFSET { start | expression } ] ] The optional, non-standard `BY` clause groups and orders the query by the specified expressions, and automatically adds them to the SELECT list @@ -429,11 +429,13 @@ An optional trailing ASC / DESC and NULLS FIRST / NULLS LAST applies to all keys In *query*, *start* may be either an unsigned numeric literal or a dynamic parameter whose value is numeric. The *count* in a LIMIT clause may be either -an unsigned numeric literal or a dynamic parameter whose value is numeric. The -*count* in a FETCH clause may be an unsigned numeric literal, a dynamic -parameter whose value is numeric, or a scalar expression enclosed in -parentheses. A FETCH *count* expression cannot reference columns from the query -input, and cannot contain aggregate functions, window functions, or sub-queries. +an unsigned numeric literal or a dynamic parameter whose value is numeric. An +OFFSET clause may contain an unsigned numeric literal, a dynamic parameter whose +value is numeric, or a scalar expression with optional parentheses. The *count* +in a FETCH clause may also be a scalar expression, but it must be enclosed in +parentheses. An OFFSET expression or FETCH *count* expression cannot reference +columns from the query input, and cannot contain aggregate functions, window +functions, or sub-queries. Support for decimal or non-integer values is adapter-dependent. An aggregate query is a query that contains a GROUP BY or a HAVING diff --git a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java index 372cd1189d28..e6e694c1e0bd 100644 --- a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java +++ b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java @@ -4075,7 +4075,16 @@ void checkPeriodPredicate(Checker checker) { + "FROM `FOO`\n" + "OFFSET ? ROWS\n" + "FETCH NEXT ? ROWS ONLY"); - // CALCITE-7592: Arithmetic and scalar expressions are allowed within parentheses. + // Arithmetic and scalar expressions are allowed within parentheses. + sql("select a from foo offset 1 + abs(-2) rows") + .ok("SELECT `A`\n" + + "FROM `FOO`\n" + + "OFFSET 1 + ABS(-2) ROWS"); + // Parentheses remain optional in OFFSET. + sql("select a from foo offset (1 + abs(-2)) rows") + .ok("SELECT `A`\n" + + "FROM `FOO`\n" + + "OFFSET 1 + ABS(-2) ROWS"); sql("select a from foo fetch next (1 + abs(-2)) rows only") .ok("SELECT `A`\n" + "FROM `FOO`\n" @@ -4091,7 +4100,9 @@ void checkPeriodPredicate(Checker checker) { // FETCH before OFFSET is illegal sql("select a from foo fetch next 3 rows only ^offset^ 1") .fails("(?s).*Encountered \"offset\" at .*"); - // Subqueries are not allowed in FETCH + // Subqueries are not allowed in OFFSET or FETCH + sql("select a from foo offset ^(^select 2) rows") + .fails("Query expression encountered in illegal context"); sql("select a from foo fetch next ^select^ 2 rows only") .fails("(?s).*Encountered \"select\" at .*"); sql("select a from foo fetch next (^select^ 2) rows only") @@ -4108,6 +4119,12 @@ void checkPeriodPredicate(Checker checker) { * SQL:2008. */ @Test void testLimit() { + sql("select a from foo order by b, c limit 2 offset 1 + abs(-2)") + .ok("SELECT `A`\n" + + "FROM `FOO`\n" + + "ORDER BY `B`, `C`\n" + + "OFFSET 1 + ABS(-2) ROWS\n" + + "FETCH NEXT 2 ROWS ONLY"); sql("select a from foo order by b, c limit 2 offset 1") .ok("SELECT `A`\n" + "FROM `FOO`\n" From 2d6b4693a5636b466e0f6af754b3e0b576de526e Mon Sep 17 00:00:00 2001 From: Kirill Tkalenko Date: Mon, 20 Jul 2026 22:01:07 +0300 Subject: [PATCH 2/5] [CALCITE-7662] Wip --- .../enumerable/EnumerableMergeUnionRule.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java index 9315f62a8a5a..a787bb73ff97 100644 --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java @@ -93,11 +93,14 @@ public EnumerableMergeUnionRule(Config config) { final boolean safeToReevaluate = RexUtil.isDeterministic(sort.fetch) && (sort.offset == null || RexUtil.isDeterministic(sort.offset)); - if (sort.offset == null && safeToReevaluate) { - inputFetch = sort.fetch; - } else if (safeToReevaluate) { - inputFetch = - RexUtil.makeOffsetFetchSum(sort.getCluster().getRexBuilder(), sort.offset, sort.fetch); + if (safeToReevaluate) { + if (sort.offset == null) { + inputFetch = sort.fetch; + } else { + inputFetch = + RexUtil.makeOffsetFetchSum( + sort.getCluster().getRexBuilder(), sort.offset, sort.fetch); + } } } From ab8db4fe073dbd4615196aa0a126dbfb1d334cf7 Mon Sep 17 00:00:00 2001 From: Kirill Tkalenko Date: Tue, 21 Jul 2026 07:33:19 +0300 Subject: [PATCH 3/5] [CALCITE-7662] Wip --- .../enumerable/EnumerableMergeUnionRule.java | 6 +-- .../enumerable/EnumerableMergeUnionTest.java | 47 +++++++------------ 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java index a787bb73ff97..3f2bdc5fe1f9 100644 --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java @@ -85,11 +85,9 @@ public EnumerableMergeUnionRule(Config config) { final Union union = call.rel(1); final int unionInputsSize = union.getInputs().size(); - // Push down sort limit, if possible. Do not push it through UNION DISTINCT: - // duplicate rows do not count towards the final limit, so limiting an input - // before duplicate elimination could discard a row needed in the result. + // Push down sort limit, if possible. RexNode inputFetch = null; - if (union.all && sort.fetch != null) { + if (sort.fetch != null) { final boolean safeToReevaluate = RexUtil.isDeterministic(sort.fetch) && (sort.offset == null || RexUtil.isDeterministic(sort.offset)); diff --git a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java index 1fb766223ea2..1fc0db8c44f4 100644 --- a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java +++ b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java @@ -146,23 +146,6 @@ class EnumerableMergeUnionTest { .returnsOrdered("empid=2"); } - @Test void mergeUnionDistinctDoesNotPushLimitBeforeDeduplication() { - tester(false, - new HrSchemaBig(), - "select deptno from emps where empid <= 3 " - + "union select deptno from emps where empid between 40 and 41 " - + "order by deptno offset ? rows fetch next ? rows only") - .explainContains("EnumerableLimitSort(sort0=[$0], dir0=[ASC], " - + "offset=[?0], fetch=[?1])\n" - + " EnumerableMergeUnion(all=[false])\n" - + " EnumerableSort(sort0=[$0], dir0=[ASC])\n") - .consumesPreparedStatement(p -> { - p.setBigDecimal(1, new BigDecimal("0.5")); - p.setBigDecimal(2, new BigDecimal("0.5")); - }) - .returnsOrdered("deptno=20"); - } - @Test void mergeUnionDoesNotPushNonDeterministicOffset() { tester(false, new HrSchemaBig(), @@ -244,14 +227,16 @@ class EnumerableMergeUnionTest { tester(false, new HrSchemaBig(), "select * from (select empid, name from emps where name like 'G%' union select empid, name from emps where name like '%l') order by name limit 3") - .explainContains("EnumerableLimitSort(sort0=[$1], dir0=[ASC], fetch=[3])\n" + .explainContains("EnumerableLimit(fetch=[3])\n" + " EnumerableMergeUnion(all=[false])\n" - + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n" - + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n") + + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" + + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[3])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" + + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[3])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n") .returnsOrdered( "empid=29; name=Anibal", "empid=1; name=Bill", @@ -284,12 +269,14 @@ class EnumerableMergeUnionTest { "select * from (select empid, name from emps where name like 'G%' union select empid, name from emps where name like '%l') order by name limit 3 offset 2") .explainContains("EnumerableLimit(offset=[2], fetch=[3])\n" + " EnumerableMergeUnion(all=[false])\n" - + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n" - + " EnumerableSort(sort0=[$1], dir0=[ASC])\n" - + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], empid=[$t0], name=[$t2], $condition=[$t6])\n" - + " EnumerableTableScan(table=[[s, emps]])\n") + + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" + + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[5])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['G%'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2])\n" + + " EnumerableLimitSort(sort0=[$2], dir0=[ASC], fetch=[5])\n" + + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=['%l'], expr#6=[LIKE($t2, $t5)], proj#0..4=[{exprs}], $condition=[$t6])\n" + + " EnumerableTableScan(table=[[s, emps]])\n") .returnsOrdered( "empid=40; name=Emmanuel", "empid=10; name=Gabriel", From affd17842a2f667f36d6d743cae352dd9b40f50c Mon Sep 17 00:00:00 2001 From: Kirill Tkalenko Date: Tue, 21 Jul 2026 07:43:59 +0300 Subject: [PATCH 4/5] [CALCITE-7662] Wip --- .../rel/rel2sql/RelToSqlConverterTest.java | 15 +++++++++++++ .../apache/calcite/rex/RexProgramTest.java | 3 +++ .../org/apache/calcite/test/JdbcTest.java | 21 +++++++++++++++++++ .../apache/calcite/test/RelBuilderTest.java | 12 +++++++++++ .../apache/calcite/test/RelMetadataTest.java | 3 +++ .../apache/calcite/test/RelOptRulesTest.java | 12 +++++++++++ .../calcite/test/SqlToRelConverterTest.java | 3 +++ .../apache/calcite/test/SqlValidatorTest.java | 3 +++ .../enumerable/EnumerableMergeUnionTest.java | 9 ++++++++ .../org/apache/calcite/test/ServerTest.java | 3 +++ 10 files changed, 84 insertions(+) diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 1685bb3cae26..f48685837b22 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -4931,6 +4931,9 @@ private SqlDialect nonOrdinalDialect() { sql(query).withMysql().ok(expected); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionWithLimitDialect() { final String query = "select \"product_id\"\n" + "from \"product\"\n" @@ -4956,6 +4959,9 @@ private SqlDialect nonOrdinalDialect() { sql(query).withSQLite().throws_(error); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testNegativeOffsetExpressionIsRejectedBeforeSqlGeneration() { final String query = "select \"product_id\"\n" + "from \"product\"\n" @@ -4979,6 +4985,9 @@ private SqlDialect nonOrdinalDialect() { + "be reduced to a literal"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testParameterizedOffsetExpressionWithLimitDialect() { final String query = "select \"product_id\"\n" + "from \"product\"\n" @@ -5001,6 +5010,9 @@ private SqlDialect nonOrdinalDialect() { sql(query).withSQLite().ok(expected); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testParameterizedOffsetExpressionWithSQLite() { final String query = "select \"product_id\"\n" + "from \"product\"\n" @@ -5012,6 +5024,9 @@ private SqlDialect nonOrdinalDialect() { sql(query).withSQLite().ok(expected); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testDynamicOffsetExpressionIsNotReduced() { final String query = "select \"product_id\"\n" + "from \"product\"\n" diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java index 51b738017c30..2c2801c7ab16 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java @@ -3623,6 +3623,9 @@ private void assertTypeAndToString( containsString("FETCH value -1.5 is out of range")); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testValidateOffsetValueAllowsFractionalBigDecimal() { assertThat(RexUtil.validateOffsetValue(new BigDecimal("1.5")), is(new BigDecimal("1.5"))); diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java index 9385d4a28d1b..f3720e7bc15a 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java @@ -3593,6 +3593,9 @@ public void checkOrderBy(final boolean desc, + "X=3\n"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpression() { final CalciteAssert.AssertThat with = CalciteAssert.that(); final String values = "select * from (values (1), (2), (3), (4)) as t(x)\n"; @@ -3629,6 +3632,9 @@ public void checkOrderBy(final boolean desc, } } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testBindableOffsetExpression() { try (Hook.Closeable ignored = Hook.ENABLE_BINDABLE.addThread(Hook.propertyJ(true))) { CalciteAssert.that() @@ -3654,6 +3660,9 @@ public void checkOrderBy(final boolean desc, + "X=2\n"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionFunctionArguments() { final CalciteAssert.AssertThat with = CalciteAssert.that(); final String values = "select * from (values (1), (2), (3)) as t(x)\n"; @@ -3678,6 +3687,9 @@ public void checkOrderBy(final boolean desc, .throws_("FETCH expression evaluated to NULL"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionInvalidValue() { final CalciteAssert.AssertThat with = CalciteAssert.that(); final String values = "select * from (values (1), (2), (3)) as t(x)\n"; @@ -3788,6 +3800,9 @@ public void checkOrderBy(final boolean desc, } } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testCorrelatedPreparedOffsetExpression() throws Exception { for (String offset : new String[] {"?", "? + 0"}) { final String sql = "select d.\"name\" as dname, e.\"name\" as ename\n" @@ -3833,6 +3848,9 @@ public void checkOrderBy(final boolean desc, .returns(expected); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionBeyondLong() { final CalciteAssert.AssertThat with = CalciteAssert.that(); final String values = "select * from (values (1), (2), (3), (4)) as t(x)\n"; @@ -6367,6 +6385,9 @@ private CalciteAssert.AssertQuery withEmpDept(String sql) { }); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testPreparedOffsetExpression() throws Exception { CalciteAssert.that() .doWithConnection(connection -> { diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java index d7d6e7f963bc..290c26199ffd 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -5600,6 +5600,9 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build ImmutableList.of())); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionCannotReferenceInputField() { final RelBuilder builder = RelBuilder.create(config().build()); builder.scan("DEPT"); @@ -5626,6 +5629,9 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build ImmutableList.of()); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionMustHaveNumericType() { final RelBuilder builder = RelBuilder.create(config().build()); builder.scan("DEPT"); @@ -5655,6 +5661,9 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build + " LogicalTableScan(table=[[scott, DEPT]])\n")); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionAllowsScalarCallAndDynamicParameter() { final RelBuilder builder = RelBuilder.create(config().build()); final RelDataType intType = @@ -5701,6 +5710,9 @@ private static RelNode buildCorrelateWithJoin(JoinRelType type, RelBuilder build () -> builder.sortLimit(null, subQuery, ImmutableList.of())); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionCannotContainAggregateWindowOrSubQuery() { final RelBuilder builder = RelBuilder.create(config().build()); final RelDataType intType = diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java index 7232c40bcdd8..a40927c37a04 100644 --- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java @@ -1527,6 +1527,9 @@ void testColumnOriginsUnion() { .assertThatRowCount(is(2D), is(0D), is(2D)); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testMinRowCountOffsetExpression() { final String sql = "select * from (values (1), (2)) as t(x)\n" + "offset 2 - 2 rows"; diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index 8e7e2945f14a..c427dadea203 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -1766,6 +1766,9 @@ private void checkJoinProjectTransposeDoesNotMatch(JoinRelType type) { .check(); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testSortUnionTransposePushesLiteralOffset() { final String sql = "select a.name from dept a\n" + "union all\n" @@ -1777,6 +1780,9 @@ private void checkJoinProjectTransposeDoesNotMatch(JoinRelType type) { .check(); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testSortUnionTransposePushesParameterizedOffsetExpression() { final String sql = "select a.name from dept a\n" + "union all\n" @@ -1788,6 +1794,9 @@ private void checkJoinProjectTransposeDoesNotMatch(JoinRelType type) { .check(); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testSortUnionTransposeWithNonDeterministicOffset() { final String sql = "select a.name from dept a\n" + "union all\n" @@ -12539,6 +12548,9 @@ private void checkNondeterministicFetchPreventsDecorrelation(boolean enableTopDo .check(); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testTopDownGeneralDecorrelateForSubqueryWithOffsetExpression() { final String sql = "select empno from emp where " + "sal > SOME(select sal from emp_b where emp.deptno = emp_b.deptno " 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 ec34053df9e6..451e446ed196 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -1253,6 +1253,9 @@ public static void checkActualAndReferenceFiles() { sql(sql).ok(); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetWithExpression() { final String sql = "select empno from emp offset 1 + abs(-2) rows"; 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 eca160ce491c..7000048a37c3 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -10723,6 +10723,9 @@ void testGroupExpressionEquivalenceParams() { .fails("Windowed aggregate expression is illegal in FETCH clause"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionType() { sql("select name from dept offset ^upper('x')^ rows") .fails("OFFSET expression must have a numeric type; " diff --git a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java index 1fc0db8c44f4..5ca85c077758 100644 --- a/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java +++ b/core/src/test/java/org/apache/calcite/test/enumerable/EnumerableMergeUnionTest.java @@ -113,6 +113,9 @@ class EnumerableMergeUnionTest { "empid=1; name=Bill"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void mergeUnionPushesParameterizedOffsetExpression() { tester(false, new HrSchemaBig(), @@ -129,6 +132,9 @@ class EnumerableMergeUnionTest { "empid=2; name=Eric"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void mergeUnionRoundsOffsetAndFetchSeparatelyWhenPushingLimit() { tester(false, new HrSchemaBig(), @@ -146,6 +152,9 @@ class EnumerableMergeUnionTest { .returnsOrdered("empid=2"); } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void mergeUnionDoesNotPushNonDeterministicOffset() { tester(false, new HrSchemaBig(), diff --git a/server/src/test/java/org/apache/calcite/test/ServerTest.java b/server/src/test/java/org/apache/calcite/test/ServerTest.java index 0d6b4520b14c..0be7fb16efa1 100644 --- a/server/src/test/java/org/apache/calcite/test/ServerTest.java +++ b/server/src/test/java/org/apache/calcite/test/ServerTest.java @@ -489,6 +489,9 @@ static Connection connect() throws SQLException { } } + /** Test case for + * [CALCITE-7662] + * Add expression support for OFFSET. */ @Test void testOffsetExpressionCannotReferenceInputColumn() throws Exception { try (Connection c = connect(); Statement s = c.createStatement()) { From 745aaf1d8cfbbd39f396afa39e25ebda1ca49a4b Mon Sep 17 00:00:00 2001 From: Kirill Tkalenko Date: Tue, 21 Jul 2026 13:55:44 +0300 Subject: [PATCH 5/5] [CALCITE-7662] Wip --- .../calcite/runtime/CalciteResource.java | 3 ++ .../org/apache/calcite/sql/SqlDialect.java | 32 +++++++++---------- .../sql/validate/SqlValidatorImpl.java | 7 ++++ .../runtime/CalciteResource.properties | 1 + .../org/apache/calcite/test/JdbcTest.java | 14 ++++---- .../apache/calcite/test/SqlValidatorTest.java | 6 ++-- 6 files changed, 38 insertions(+), 25 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 6252728bf799..eb18b51e1bf7 100644 --- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java +++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java @@ -175,6 +175,9 @@ ExInst offsetFetchExpressionCannotReferenceColumn( @BaseMessage("{0} expression evaluated to NULL") ExInst offsetFetchExpressionEvaluatedToNull(String kind); + @BaseMessage("{0} must not be negative") + ExInst offsetFetchValueMustNotBeNegative(String kind); + @BaseMessage("Values in expression list must have compatible types") ExInst incompatibleTypesInList(); diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java index 1cbf52298639..994b7418452d 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java @@ -1081,7 +1081,7 @@ protected static void unparseFetchUsingAnsi(SqlWriter writer, @Nullable SqlNode writer.startList(SqlWriter.FrameTypeEnum.FETCH); writer.keyword("FETCH"); writer.keyword("NEXT"); - unparseOffsetFetchExpression(writer, fetch); + unparseFetchExpression(writer, fetch); writer.keyword("ROWS"); writer.keyword("ONLY"); writer.endList(fetchFrame); @@ -1089,25 +1089,25 @@ protected static void unparseFetchUsingAnsi(SqlWriter writer, @Nullable SqlNode } private static void unparseOffsetExpression(SqlWriter writer, SqlNode offset) { - if (offset instanceof SqlCall) { - writer.getDialect().unparseCall(writer, (SqlCall) offset, 0, 0); - } else { - offset.unparse(writer, 0, 0); + unparseExpression(writer, offset); + } + + private static void unparseFetchExpression(SqlWriter writer, SqlNode fetch) { + if (fetch instanceof SqlLiteral + || fetch instanceof SqlDynamicParam) { + fetch.unparse(writer, -1, -1); + return; } + final SqlWriter.Frame expressionFrame = writer.startList("(", ")"); + unparseExpression(writer, fetch); + writer.endList(expressionFrame); } - private static void unparseOffsetFetchExpression(SqlWriter writer, SqlNode node) { - if (node instanceof SqlLiteral - || node instanceof SqlDynamicParam) { - node.unparse(writer, -1, -1); + private static void unparseExpression(SqlWriter writer, SqlNode node) { + if (node instanceof SqlCall) { + writer.getDialect().unparseCall(writer, (SqlCall) node, 0, 0); } else { - final SqlWriter.Frame expressionFrame = writer.startList("(", ")"); - if (node instanceof SqlCall) { - writer.getDialect().unparseCall(writer, (SqlCall) node, 0, 0); - } else { - node.unparse(writer, 0, 0); - } - writer.endList(expressionFrame); + node.unparse(writer, 0, 0); } } diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java index 3ca0325a931d..fae4a0c53db5 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java @@ -70,6 +70,7 @@ import org.apache.calcite.sql.SqlMerge; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlNumericLiteral; import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.SqlOperatorTable; import org.apache.calcite.sql.SqlOrderBy; @@ -1780,6 +1781,12 @@ private void validateOffsetFetchExpression(@Nullable SqlNode node, throw newValidationError(node, RESOURCE.offsetFetchExpressionEvaluatedToNull(kind)); } + if (node instanceof SqlNumericLiteral + && requireNonNull(((SqlNumericLiteral) node).bigDecimalValue()) + .signum() < 0) { + throw newValidationError(node, + RESOURCE.offsetFetchValueMustNotBeNegative(kind)); + } validateNoAggs(aggOrOverFinder, node, kind); node.accept(new SqlBasicVisitor() { @Override public Void visit(SqlIdentifier id) { 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 3f0100a10263..bfbf8a78d2bc 100644 --- a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties +++ b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties @@ -64,6 +64,7 @@ IncompatibleValueType=Values passed to {0} operator must have compatible types OffsetFetchExpressionMustBeNumeric={0} expression must have a numeric type; actual type is ''{1}'' OffsetFetchExpressionCannotReferenceColumn={0} expression cannot reference table column ''{1}'' OffsetFetchExpressionEvaluatedToNull={0} expression evaluated to NULL +OffsetFetchValueMustNotBeNegative={0} must not be negative IncompatibleTypesInList=Values in expression list must have compatible types IncompatibleCharset=Cannot apply operation ''{0}'' to strings with different charsets ''{1}'' and ''{2}'' InvalidOrderByPos=ORDER BY is only allowed on top-level SELECT diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java index f3720e7bc15a..d235fa327c77 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java @@ -3709,13 +3709,13 @@ public void checkOrderBy(final boolean desc, + "from \"hr\".\"depts\" d,\n" + "lateral (select \"name\" from \"hr\".\"emps\"\n" + " where \"deptno\" = d.\"deptno\"\n"; - for (String fetch : new String[] {"(0 - 1)", "(-1)"}) { - for (boolean topDown : new boolean[] {false, true}) { - CalciteAssert.hr() - .with(CalciteConnectionProperty.TOPDOWN_GENERAL_DECORRELATION_ENABLED, topDown) - .query(sqlPrefix + " fetch next " + fetch + " rows only) e") - .throws_("FETCH value -1 is out of range"); - } + for (boolean topDown : new boolean[] {false, true}) { + final CalciteAssert.AssertThat with = CalciteAssert.hr() + .with(CalciteConnectionProperty.TOPDOWN_GENERAL_DECORRELATION_ENABLED, topDown); + with.query(sqlPrefix + " fetch next (0 - 1) rows only) e") + .throws_("FETCH value -1 is out of range"); + with.query(sqlPrefix + " fetch next (-1) rows only) e") + .throws_("FETCH must not be negative"); } } 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 7000048a37c3..97d55abd0195 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -10758,12 +10758,14 @@ void testGroupExpressionEquivalenceParams() { @Test void testNegativeFetchOffsetLimit() { sql("select name from dept limit ^-^1") .fails("(?s).*Encountered \"-\".*"); - sql("select name from dept offset -1").ok(); + sql("select name from dept offset ^-1^") + .fails("OFFSET must not be negative"); sql("select name from dept fetch next ^-^1 rows only") .fails("(?s).*Encountered \"-\".*"); sql("select name from dept order by name limit ^-^1") .fails("(?s).*Encountered \"-\".*"); - sql("select name from dept order by name offset -1").ok(); + sql("select name from dept order by name offset ^-1^") + .fails("OFFSET must not be negative"); sql("select name from dept order by name fetch next ^-^1 rows only") .fails("(?s).*Encountered \"-\".*"); }