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";

Reply via email to