This is an automated email from the ASF dual-hosted git repository. rubenql pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push: new 5eb97411e [CALCITE-5048] Query with parameterized LIMIT and correlated sub-query throws AssertionError "not a literal" 5eb97411e is described below commit 5eb97411ecb5df1c0fdfa5d3568d93468bff0fa1 Author: xiejiajun <jiajunbernou...@foxmail.com> AuthorDate: Tue Apr 5 09:04:07 2022 +0800 [CALCITE-5048] Query with parameterized LIMIT and correlated sub-query throws AssertionError "not a literal" --- .../calcite/rel/metadata/RelMdMaxRowCount.java | 26 ++++++-------- .../calcite/rel/metadata/RelMdMinRowCount.java | 26 ++++++-------- .../org/apache/calcite/rel/metadata/RelMdUtil.java | 3 +- .../apache/calcite/sql2rel/RelDecorrelator.java | 9 ++--- .../java/org/apache/calcite/tools/RelBuilder.java | 32 +++++++++++++++-- .../apache/calcite/rel/metadata/RelMdUtilTest.java | 29 +++++++++++++++ .../java/org/apache/calcite/test/JdbcTest.java | 22 ++++++++++++ .../org/apache/calcite/test/RelBuilderTest.java | 41 ++++++++++++++++++++++ .../org/apache/calcite/test/RelMetadataTest.java | 9 +++++ 9 files changed, 155 insertions(+), 42 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMaxRowCount.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMaxRowCount.java index e4801b64c..a3c4b18af 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMaxRowCount.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdMaxRowCount.java @@ -109,16 +109,13 @@ public class RelMdMaxRowCount if (rowCount == null) { rowCount = Double.POSITIVE_INFINITY; } - final int offset = rel.offset == null ? 0 : RexLiteral.intValue(rel.offset); + + final int offset = rel.offset instanceof RexLiteral ? RexLiteral.intValue(rel.offset) : 0; rowCount = Math.max(rowCount - offset, 0D); - if (rel.fetch != null) { - final int limit = RexLiteral.intValue(rel.fetch); - if (limit < rowCount) { - return (double) limit; - } - } - return rowCount; + final double limit = + rel.fetch instanceof RexLiteral ? RexLiteral.intValue(rel.fetch) : rowCount; + return limit < rowCount ? limit : rowCount; } public Double getMaxRowCount(EnumerableLimit rel, RelMetadataQuery mq) { @@ -126,16 +123,13 @@ public class RelMdMaxRowCount if (rowCount == null) { rowCount = Double.POSITIVE_INFINITY; } - final int offset = rel.offset == null ? 0 : RexLiteral.intValue(rel.offset); + + final int offset = rel.offset instanceof RexLiteral ? RexLiteral.intValue(rel.offset) : 0; rowCount = Math.max(rowCount - offset, 0D); - if (rel.fetch != null) { - final int limit = RexLiteral.intValue(rel.fetch); - if (limit < rowCount) { - return (double) limit; - } - } - return rowCount; + final double limit = + rel.fetch instanceof RexLiteral ? RexLiteral.intValue(rel.fetch) : rowCount; + return limit < rowCount ? limit : rowCount; } public @Nullable Double getMaxRowCount(Aggregate rel, RelMetadataQuery mq) { 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 21d55ad8b..131fec5de 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 @@ -108,16 +108,13 @@ public class RelMdMinRowCount if (rowCount == null) { rowCount = 0D; } - final int offset = rel.offset == null ? 0 : RexLiteral.intValue(rel.offset); + + final int offset = rel.offset instanceof RexLiteral ? RexLiteral.intValue(rel.offset) : 0; rowCount = Math.max(rowCount - offset, 0D); - if (rel.fetch != null) { - final int limit = RexLiteral.intValue(rel.fetch); - if (limit < rowCount) { - return (double) limit; - } - } - return rowCount; + final double limit = + rel.fetch instanceof RexLiteral ? RexLiteral.intValue(rel.fetch) : rowCount; + return limit < rowCount ? limit : rowCount; } public Double getMinRowCount(EnumerableLimit rel, RelMetadataQuery mq) { @@ -125,16 +122,13 @@ public class RelMdMinRowCount if (rowCount == null) { rowCount = 0D; } - final int offset = rel.offset == null ? 0 : RexLiteral.intValue(rel.offset); + + final int offset = rel.offset instanceof RexLiteral ? RexLiteral.intValue(rel.offset) : 0; rowCount = Math.max(rowCount - offset, 0D); - if (rel.fetch != null) { - final int limit = RexLiteral.intValue(rel.fetch); - if (limit < rowCount) { - return (double) limit; - } - } - return rowCount; + final double limit = + rel.fetch instanceof RexLiteral ? RexLiteral.intValue(rel.fetch) : rowCount; + return limit < rowCount ? limit : rowCount; } public Double getMinRowCount(Aggregate rel, RelMetadataQuery mq) { 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 8370bab15..107776294 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 @@ -28,6 +28,7 @@ import org.apache.calcite.rel.core.Project; 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; @@ -911,7 +912,7 @@ public class RelMdUtil { return true; } final Double rowCount = mq.getMaxRowCount(input); - if (rowCount == null) { + if (rowCount == null || offset instanceof RexDynamicParam || fetch instanceof RexDynamicParam) { // Cannot be determined return false; } 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 95b52a18d..7acfd46f2 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java @@ -477,13 +477,10 @@ public class RelDecorrelator implements ReflectiveVisitor { RelCollation oldCollation = rel.getCollation(); RelCollation newCollation = RexUtil.apply(mapping, oldCollation); - final int offset = rel.offset == null ? -1 : RexLiteral.intValue(rel.offset); - final int fetch = rel.fetch == null ? -1 : RexLiteral.intValue(rel.fetch); - final RelNode newSort = relBuilder - .push(newInput) - .sortLimit(offset, fetch, relBuilder.fields(newCollation)) - .build(); + .push(newInput) + .sortLimit(rel.offset, rel.fetch, relBuilder.fields(newCollation)) + .build(); // Sort does not change input ordering return register(rel, newSort, frame.oldToNewOutputs, frame.corDefOutputs); 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 f141968a1..968c8adf2 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -70,6 +70,7 @@ import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexCall; import org.apache.calcite.rex.RexCallBinding; import org.apache.calcite.rex.RexCorrelVariable; +import org.apache.calcite.rex.RexDynamicParam; import org.apache.calcite.rex.RexExecutor; import org.apache.calcite.rex.RexFieldCollation; import org.apache.calcite.rex.RexInputRef; @@ -3191,12 +3192,37 @@ public class RelBuilder { */ public RelBuilder sortLimit(int offset, int fetch, Iterable<? extends RexNode> nodes) { + final @Nullable RexNode offsetNode = offset <= 0 ? null : literal(offset); + final @Nullable RexNode fetchNode = fetch < 0 ? null : literal(fetch); + return sortLimit(offsetNode, fetchNode, nodes); + } + + /** 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 fetchNode RexLiteral means maximum number of rows to fetch is deterministic, + * RexDynamicParam mean maximum number is dynamic. + * @param nodes Sort expressions + */ + public RelBuilder sortLimit(@Nullable RexNode offsetNode, @Nullable RexNode fetchNode, + Iterable<? extends RexNode> nodes) { + if (offsetNode != null) { + if (!(offsetNode instanceof RexLiteral || offsetNode instanceof RexDynamicParam)) { + throw new IllegalArgumentException("OFFSET node must be RexLiteral or RexDynamicParam"); + } + } + if (fetchNode != null) { + if (!(fetchNode instanceof RexLiteral || fetchNode instanceof RexDynamicParam)) { + throw new IllegalArgumentException("FETCH node must be RexLiteral or RexDynamicParam"); + } + } + final Registrar registrar = new Registrar(fields(), ImmutableList.of()); final List<RelFieldCollation> fieldCollations = registrar.registerFieldCollations(nodes); - - final RexNode offsetNode = offset <= 0 ? null : literal(offset); - final RexNode fetchNode = fetch < 0 ? null : literal(fetch); + final int fetch = fetchNode instanceof RexLiteral + ? RexLiteral.intValue(fetchNode) : -1; if (offsetNode == null && fetch == 0 && config.simplifyLimit()) { return empty(); } diff --git a/core/src/test/java/org/apache/calcite/rel/metadata/RelMdUtilTest.java b/core/src/test/java/org/apache/calcite/rel/metadata/RelMdUtilTest.java index 617b9c658..dcfa6ecb8 100644 --- a/core/src/test/java/org/apache/calcite/rel/metadata/RelMdUtilTest.java +++ b/core/src/test/java/org/apache/calcite/rel/metadata/RelMdUtilTest.java @@ -16,9 +16,16 @@ */ package org.apache.calcite.rel.metadata; +import org.apache.calcite.rel.RelCollations; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Sort; +import org.apache.calcite.test.RelMetadataFixture; +import org.apache.calcite.tools.Frameworks; + import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -26,6 +33,15 @@ import static org.junit.jupiter.api.Assertions.assertTrue; */ public class RelMdUtilTest { + /** Creates a fixture. */ + protected RelMetadataFixture fixture() { + return RelMetadataFixture.DEFAULT; + } + + final RelMetadataFixture sql(String sql) { + return fixture().withSql(sql); + } + @Test void testNumDistinctVals() { // the first element must be distinct, the second one has half chance of being distinct assertEquals(1.5, RelMdUtil.numDistinctVals(2.0, 2.0), 1e-5); @@ -51,4 +67,17 @@ public class RelMdUtilTest { assertEquals(domainSize, RelMdUtil.numDistinctVals(domainSize, domainSize * 100), 1e-5); } + @Test void testDynamicParameterInLimitOffset() { + Frameworks.withPlanner((cluster, relOptSchema, rootSchema) -> { + RelMetadataQuery mq = cluster.getMetadataQuery(); + RelNode rel = sql("select * from emp limit ? offset ?").toRel(); + Sort sort = (Sort) rel; + assertFalse( + RelMdUtil.checkInputForCollationAndLimit(mq, sort.getInput(), RelCollations.EMPTY, + sort.offset, sort.fetch) + ); + return null; + }); + } + } 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 162d496f1..ddd35fa1a 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java @@ -5430,6 +5430,28 @@ public class JdbcTest { }); } + /** + * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5048">[CALCITE-5048] + * Query with parameterized LIMIT and correlated sub-query throws AssertionError "not a + * literal"</a>. + */ + @Test void testDynamicParameterInLimitOffset() { + CalciteAssert.hr() + .query("SELECT * FROM \"hr\".\"emps\" AS a " + + "WHERE \"deptno\" = " + + "(SELECT MAX(\"deptno\") " + + "FROM \"hr\".\"emps\" AS b " + + "WHERE a.\"empid\" = b.\"empid\"" + + ") ORDER BY \"salary\" LIMIT ? OFFSET ?") + .explainContains("EnumerableLimit(offset=[?1], fetch=[?0])") + .consumesPreparedStatement(p -> { + p.setInt(1, 2); + p.setInt(2, 1); + }) + .returns("empid=200; deptno=20; name=Eric; salary=8000.0; commission=500\n" + + "empid=100; deptno=10; name=Bill; salary=10000.0; commission=1000\n"); + } + /** Tests a JDBC connection that provides a model (a single schema based on * a JDBC database). */ @Test void testModel() { 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 5851dcbfe..e174fae8e 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -17,12 +17,14 @@ package org.apache.calcite.test; import org.apache.calcite.adapter.enumerable.EnumerableConvention; +import org.apache.calcite.adapter.enumerable.EnumerableRules; import org.apache.calcite.adapter.java.ReflectiveSchema; import org.apache.calcite.jdbc.CalciteConnection; import org.apache.calcite.plan.Contexts; import org.apache.calcite.plan.Convention; import org.apache.calcite.plan.RelOptTable; import org.apache.calcite.plan.RelTraitDef; +import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.RelCollations; import org.apache.calcite.rel.RelDistributions; import org.apache.calcite.rel.RelNode; @@ -37,6 +39,7 @@ import org.apache.calcite.rel.core.TableFunctionScan; import org.apache.calcite.rel.core.TableModify; import org.apache.calcite.rel.core.Window; import org.apache.calcite.rel.hint.RelHint; +import org.apache.calcite.rel.metadata.RelMetadataQuery; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.rel.type.RelDataTypeField; @@ -69,10 +72,13 @@ import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.validate.SqlUserDefinedTableFunction; import org.apache.calcite.test.schemata.hr.HrSchema; import org.apache.calcite.tools.Frameworks; +import org.apache.calcite.tools.Program; import org.apache.calcite.tools.Programs; import org.apache.calcite.tools.RelBuilder; import org.apache.calcite.tools.RelRunner; import org.apache.calcite.tools.RelRunners; +import org.apache.calcite.tools.RuleSet; +import org.apache.calcite.tools.RuleSets; import org.apache.calcite.util.Holder; import org.apache.calcite.util.ImmutableBitSet; import org.apache.calcite.util.Pair; @@ -4371,6 +4377,41 @@ public class RelBuilderTest { assertThat(root, hasTree(expected)); } + @Test void testDynamicParameterInLimitOffset() { + final RelBuilder relBuilder = RelBuilder.create(config().build()); + final RelDataType intType = relBuilder.getTypeFactory().createSqlType(SqlTypeName.INTEGER); + final RexBuilder rexBuilder = relBuilder.getRexBuilder(); + + RelNode planBefore = relBuilder + .scan("DEPT") + .sortLimit(rexBuilder.makeDynamicParam(intType, 1), + rexBuilder.makeDynamicParam(intType, 0), + ImmutableList.of()) + .build(); + String expectedLogicalPlan = "LogicalSort(offset=[?1], fetch=[?0])\n" + + " LogicalTableScan(table=[[scott, DEPT]])\n"; + assertThat(planBefore, hasTree(expectedLogicalPlan)); + + RuleSet prepareRules = + RuleSets.ofList( + EnumerableRules.ENUMERABLE_FILTER_RULE, + EnumerableRules.ENUMERABLE_SORT_RULE, + EnumerableRules.ENUMERABLE_LIMIT_RULE, + EnumerableRules.ENUMERABLE_TABLE_SCAN_RULE); + RelTraitSet desiredTraits = planBefore.getTraitSet() + .replace(EnumerableConvention.INSTANCE); + Program program = Programs.of(prepareRules); + RelNode planAfter = program.run(planBefore.getCluster().getPlanner(), planBefore, + desiredTraits, ImmutableList.of(), ImmutableList.of()); + String expectedEnumerablePlan = "EnumerableLimit(offset=[?1], fetch=[?0])\n" + + " EnumerableTableScan(table=[[scott, DEPT]])\n"; + assertThat(planAfter, hasTree(expectedEnumerablePlan)); + + RelMetadataQuery mq = planAfter.getCluster().getMetadataQuery(); + assertThat(mq.getMinRowCount(planAfter), is(0D)); + assertThat(mq.getMaxRowCount(planAfter), is(Double.POSITIVE_INFINITY)); + } + @Test void testAdoptConventionEnumerable() { final RelBuilder builder = RelBuilder.create(config().build()); RelNode root = builder 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 038547c21..fc5e66e16 100644 --- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java @@ -644,6 +644,15 @@ public class RelMetadataTest { fixture.assertThatRowCount(is(9D), is(0D), is(10d)); } + @Test void testRowCountSortLimitOffsetDynamic() { + sql("select * from emp order by ename limit ? offset ?") + .assertThatRowCount(is(EMP_SIZE), is(0D), is(Double.POSITIVE_INFINITY)); + sql("select * from emp order by ename limit 1 offset ?") + .assertThatRowCount(is(EMP_SIZE), is(0D), is(1D)); + sql("select * from emp order by ename limit ? offset 1") + .assertThatRowCount(is(EMP_SIZE - 1), is(0D), is(Double.POSITIVE_INFINITY)); + } + @Test void testRowCountSortLimitOffsetOnFinite() { final String sql = "select * from (select * from emp limit 12)\n" + "order by ename limit 20 offset 5";