IGNITE-22967 SQL Calcite: Fix lost limit/offset on planning - Fixes #…

…11471. Signed-off-by: Aleksey Plekhanov <plehanov.alex@gmail.com>
apache · Aug 13, 2024 · 6864b43 · 6864b43
1 parent 7f35ec7
commit 6864b43
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 12 deletions.
diff --git a/.../main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java b/.../main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java
@@ -140,6 +140,7 @@ public class CalciteQueryProcessor extends GridProcessorAdapter implements Query
     public static final FrameworkConfig FRAMEWORK_CONFIG = Frameworks.newConfigBuilder()
         .executor(new RexExecutorImpl(DataContexts.EMPTY))
         .sqlToRelConverterConfig(SqlToRelConverter.config()
+            .withRemoveSortInSubQuery(false)
             .withTrimUnusedFields(true)
             // currently SqlToRelConverter creates not optimal plan for both optimization and execution
             // so it's better to disable such rewriting right now

diff --git a/...te/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteLimit.java b/...te/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteLimit.java
@@ -18,7 +18,6 @@
 package org.apache.ignite.internal.processors.query.calcite.rel;
 
 import java.util.List;
-
 import com.google.common.collect.ImmutableList;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptCost;
@@ -35,6 +34,7 @@
 import org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCost;
 import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
 import org.apache.ignite.internal.processors.query.calcite.trait.TraitUtils;
+import org.jetbrains.annotations.Nullable;
 
 import static org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCost.FETCH_IS_PARAM_FACTOR;
 import static org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCost.OFFSET_IS_PARAM_FACTOR;
@@ -43,10 +43,10 @@
 /** */
 public class IgniteLimit extends SingleRel implements IgniteRel {
     /** Offset. */
-    private final RexNode offset;
+    @Nullable private final RexNode offset;
 
     /** Fetches rows expression (limit) */
-    private final RexNode fetch;
+    @Nullable private final RexNode fetch;
 
     /**
      * Constructor.
@@ -61,8 +61,8 @@ public IgniteLimit(
         RelOptCluster cluster,
         RelTraitSet traits,
         RelNode child,
-        RexNode offset,
-        RexNode fetch
+        @Nullable RexNode offset,
+        @Nullable RexNode fetch
     ) {
         super(cluster, traits, child);
         this.offset = offset;
@@ -154,14 +154,14 @@ else if (!requiredCollation.satisfies(relCollation))
     /**
      * @return Offset.
      */
-    public RexNode offset() {
+    @Nullable public RexNode offset() {
         return offset;
     }
 
     /**
      * @return Fetches rows expression (limit)
      */
-    public RexNode fetch() {
+    @Nullable public RexNode fetch() {
         return fetch;
     }
 

diff --git a/...ite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteSort.java b/...ite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteSort.java
@@ -17,7 +17,6 @@
 package org.apache.ignite.internal.processors.query.calcite.rel;
 
 import java.util.List;
-
 import com.google.common.collect.ImmutableList;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptCost;
@@ -36,6 +35,7 @@
 import org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCostFactory;
 import org.apache.ignite.internal.processors.query.calcite.trait.IgniteDistributions;
 import org.apache.ignite.internal.processors.query.calcite.trait.TraitUtils;
+import org.jetbrains.annotations.Nullable;
 
 import static org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCost.FETCH_IS_PARAM_FACTOR;
 import static org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCost.OFFSET_IS_PARAM_FACTOR;
@@ -65,8 +65,8 @@ public IgniteSort(
         RelTraitSet traits,
         RelNode child,
         RelCollation collation,
-        RexNode offset,
-        RexNode fetch,
+        @Nullable RexNode offset,
+        @Nullable RexNode fetch,
         boolean enforcer
     ) {
         super(cluster, traits, child, collation, offset, fetch);
@@ -106,8 +106,8 @@ public IgniteSort(RelInput input) {
         RelTraitSet traitSet,
         RelNode newInput,
         RelCollation newCollation,
-        RexNode offset,
-        RexNode fetch
+        @Nullable RexNode offset,
+        @Nullable RexNode fetch
     ) {
         return new IgniteSort(getCluster(), traitSet, newInput, traitSet.getCollation(), offset, fetch, enforcer);
     }

diff --git a/...ache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java b/...ache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java
@@ -88,6 +88,16 @@ public class LimitOffsetIntegrationTest extends AbstractBasicIntegrationTest {
                     .setSqlSchema("PUBLIC"));
     }
 
+    /** */
+    @Test
+    public void testNestedLimitOffsetWithUnion() {
+        sql("INSERT into TEST_REPL VALUES (1, 'a'), (2, 'b'), (3, 'c'), (4, 'd')");
+
+        assertQuery("(SELECT id FROM TEST_REPL WHERE id = 2) UNION ALL " +
+            "SELECT id FROM (select id from (SELECT id FROM TEST_REPL OFFSET 2) order by id OFFSET 1)"
+        ).returns(2).returns(4).check();
+    }
+
     /** Tests correctness of fetch / offset params. */
     @Test
     public void testInvalidLimitOffset() {

diff --git a/...a/org/apache/ignite/internal/processors/query/calcite/planner/LimitOffsetPlannerTest.java b/...a/org/apache/ignite/internal/processors/query/calcite/planner/LimitOffsetPlannerTest.java
@@ -61,6 +61,18 @@ public void testLimit() throws Exception {
             .and(hasChildThat(isInstanceOf(IgniteSort.class)))));
     }
 
+    /** */
+    @Test
+    public void testNestedLimitOffsetWithUnion() throws Exception {
+        IgniteSchema publicSchema = createSchemaWithTable(IgniteDistributions.random());
+
+        assertPlan("(SELECT ID FROM TEST WHERE ID = 2) UNION ALL " +
+                "SELECT ID FROM (SELECT ID from (SELECT ID FROM TEST OFFSET 20) ORDER BY ID OFFSET 10)",
+            publicSchema,
+            nodeOrAnyChild(isInstanceOf(IgniteLimit.class).and(l -> doubleFromRex(l.offset(), -1) == 10d))
+        );
+    }
+
     /** */
     @Test
     public void testEstimateRows() throws Exception {