You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by ru...@apache.org on 2022/04/18 11:00:19 UTC
[calcite] branch master updated: [CALCITE-5048] Query with parameterized LIMIT and correlated sub-query throws AssertionError "not a literal"
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 <ji...@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";