This is an automated email from the ASF dual-hosted git repository.
korlov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 38b36cefc2 IGNITE-22204: Sql. Set operation. Incorrect query
transformation for a query with limit / offset and sort (#3857)
38b36cefc2 is described below
commit 38b36cefc285a44aa2f4cba14110ba3523561202
Author: Max Zhuravkov <[email protected]>
AuthorDate: Tue Jun 11 14:25:17 2024 +0400
IGNITE-22204: Sql. Set operation. Incorrect query transformation for a
query with limit / offset and sort (#3857)
---
.../ignite/internal/sql/engine/ItSetOpTest.java | 19 ++++++++++++++
.../sql/set/test_union_with_limit.test | 2 --
.../internal/sql/engine/rel/IgniteLimit.java | 5 ++--
.../ignite/internal/sql/engine/rel/IgniteSort.java | 14 +++++-----
.../sql/engine/rule/SortExchangeTransposeRule.java | 7 ++++-
.../ignite/internal/sql/engine/util/Commons.java | 2 +-
.../sql/engine/planner/LimitOffsetPlannerTest.java | 30 +++++++++++++++++++++-
7 files changed, 66 insertions(+), 13 deletions(-)
diff --git
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSetOpTest.java
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSetOpTest.java
index 1cb1e03d59..ada7368553 100644
---
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSetOpTest.java
+++
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSetOpTest.java
@@ -357,6 +357,25 @@ public class ItSetOpTest extends BaseSqlIntegrationTest {
assertEquals(3, rows.size());
}
+ @Test
+ public void testUnionNestedSortLimit() {
+ sql("CREATE TABLE test (a INTEGER PRIMARY KEY, v VARCHAR(100))");
+ sql("INSERT INTO test VALUES (1, 'a'), (2, 'b'), (3, 'c'), (4, 'd')");
+
+ assertQuery("SELECT a FROM\n"
+ + " (SELECT a FROM test ORDER BY a LIMIT 1 OFFSET 1) t(a)\n"
+ + "UNION ALL\n"
+ + "SELECT a FROM\n"
+ + " (SELECT a FROM\n"
+ + " (SELECT a FROM test ORDER BY a LIMIT 3 OFFSET 2) i(a)\n"
+ + " ORDER BY a OFFSET 1\n"
+ + " ) t(a)\n"
+ + "\n")
+ .returns(2)
+ .returns(4)
+ .check();
+ }
+
private static void createTable(String tableName) {
sql("CREATE TABLE " + tableName + "(id INT PRIMARY KEY, name VARCHAR,
salary DOUBLE)");
}
diff --git
a/modules/sql-engine/src/integrationTest/sql/set/test_union_with_limit.test
b/modules/sql-engine/src/integrationTest/sql/set/test_union_with_limit.test
index cd1b4eddb2..61513140de 100644
--- a/modules/sql-engine/src/integrationTest/sql/set/test_union_with_limit.test
+++ b/modules/sql-engine/src/integrationTest/sql/set/test_union_with_limit.test
@@ -49,8 +49,6 @@ SELECT a FROM
----
4
-skipif ignite3
-# https://issues.apache.org/jira/browse/IGNITE-22204 incorrect plan
transformation
query I rowsort
SELECT a FROM
(SELECT a FROM test ORDER BY a LIMIT 1 OFFSET 1) t(a)
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteLimit.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteLimit.java
index 50c777a997..55e3ae4976 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteLimit.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteLimit.java
@@ -38,6 +38,7 @@ import org.apache.calcite.util.Pair;
import org.apache.ignite.internal.sql.engine.metadata.cost.IgniteCost;
import org.apache.ignite.internal.sql.engine.trait.IgniteDistributions;
import org.apache.ignite.internal.sql.engine.trait.TraitUtils;
+import org.jetbrains.annotations.Nullable;
/** Relational expression that applies a limit and/or offset to its input. */
public class IgniteLimit extends SingleRel implements IgniteRel {
@@ -62,8 +63,8 @@ public class IgniteLimit extends SingleRel implements
IgniteRel {
RelOptCluster cluster,
RelTraitSet traits,
RelNode child,
- RexNode offset,
- RexNode fetch
+ @Nullable RexNode offset,
+ @Nullable RexNode fetch
) {
super(cluster, traits, child);
this.offset = offset;
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteSort.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteSort.java
index fbdcd3183e..ba30ebdae8 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteSort.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteSort.java
@@ -40,6 +40,7 @@ import
org.apache.ignite.internal.sql.engine.metadata.cost.IgniteCost;
import org.apache.ignite.internal.sql.engine.metadata.cost.IgniteCostFactory;
import org.apache.ignite.internal.sql.engine.trait.IgniteDistributions;
import org.apache.ignite.internal.sql.engine.trait.TraitUtils;
+import org.jetbrains.annotations.Nullable;
/**
* Ignite sort operator.
@@ -62,8 +63,8 @@ public class IgniteSort extends Sort implements IgniteRel {
RelTraitSet traits,
RelNode child,
RelCollation collation,
- RexNode offset,
- RexNode fetch
+ @Nullable RexNode offset,
+ @Nullable RexNode fetch
) {
super(cluster, traits, child, collation, offset, fetch);
}
@@ -85,8 +86,9 @@ public class IgniteSort extends Sort implements IgniteRel {
}
/**
- * Constructor.
- * TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
+ * Constructor used for deserialization.
+ *
+ * @param input Serialized representation.
*/
public IgniteSort(RelInput input) {
super(changeTraits(input, IgniteConvention.INSTANCE));
@@ -98,8 +100,8 @@ public class IgniteSort extends Sort implements IgniteRel {
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);
}
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/SortExchangeTransposeRule.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/SortExchangeTransposeRule.java
index fbf714a436..4350bfa05d 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/SortExchangeTransposeRule.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/SortExchangeTransposeRule.java
@@ -48,7 +48,7 @@ public class SortExchangeTransposeRule extends
RelRule<SortExchangeTransposeRule
IgniteExchange exchange = call.rel(1);
return hashAlike(distribution(exchange.getInput()))
- && exchange.distribution() == single();
+ && exchange.distribution() == single() &&
call.rel(0).isEnforcer();
}
private static boolean hashAlike(IgniteDistribution distribution) {
@@ -59,6 +59,9 @@ public class SortExchangeTransposeRule extends
RelRule<SortExchangeTransposeRule
@Override
public void onMatch(RelOptRuleCall call) {
IgniteSort sort = call.rel(0);
+
+ assert sort.isEnforcer() : "Non enforcer sort can not be pushed under
Exchanger";
+
IgniteExchange exchange = call.rel(1);
RelOptCluster cluster = sort.getCluster();
@@ -84,6 +87,8 @@ public class SortExchangeTransposeRule extends
RelRule<SortExchangeTransposeRule
.withDescription("SortExchangeTransposeRule")
.withOperandSupplier(o0 ->
o0.operand(IgniteSort.class)
+ // Only an enforcer sort (a sort operator w/o
fetch/offset parameters) can be pushed under exchange
+ .predicate(IgniteSort::isEnforcer)
.oneInput(o1 ->
o1.operand(IgniteExchange.class)
.anyInputs()))
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
index 3134b788f9..b63180efc5 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
@@ -155,7 +155,7 @@ public final class Commons {
.withTrimUnusedFields(true)
// Disable `RemoveSortInSubQuery` hint that causes
incorrect plan transformation
// because calcite does not distinguish between VIEWs and
nested subqueries.
- // TODO https://issues.apache.org/jira/browse/IGNITE-22204
+ // TODO https://issues.apache.org/jira/browse/IGNITE-22392
.withRemoveSortInSubQuery(false)
// currently SqlToRelConverter creates not optimal plan
for both optimization and execution
// so it's better to disable such rewriting right now
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/LimitOffsetPlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/LimitOffsetPlannerTest.java
index 5a0a82b957..776124ecb1 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/LimitOffsetPlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/LimitOffsetPlannerTest.java
@@ -196,7 +196,7 @@ public class LimitOffsetPlannerTest extends
AbstractPlannerTest {
@Test
public void testNestedOffset() throws Exception {
// Tests for planner for limit/sort in nested subqueries
- // See bug https://issues.apache.org/jira/browse/IGNITE-22204
+ // See bug https://issues.apache.org/jira/browse/IGNITE-21946
TableBuilder builder = TestBuilders.table()
.name("TEST")
@@ -242,6 +242,34 @@ public class LimitOffsetPlannerTest extends
AbstractPlannerTest {
);
}
+ @Test
+ public void testOffsetFetchPropagationForSort() throws Exception {
+ TableBuilder builder = TestBuilders.table()
+ .name("TEST")
+ .addKeyColumn("ID", NativeTypes.INT32)
+ .addColumn("A", NativeTypes.INT32)
+ .size(ROW_CNT)
+ .distribution(IgniteDistributions.affinity(0, 1, 1));
+
+ IgniteSchema publicSchema = createSchema(builder.build());
+
+ assertPlan("SELECT a FROM (SELECT a FROM test ORDER BY a LIMIT 3
OFFSET 2) i(a) ORDER BY a OFFSET 1",
+ publicSchema, isInstanceOf(IgniteLimit.class)
+ .and(l -> doubleFromRex(l.offset(), -1) == 1.0)
+ .and(input(isInstanceOf(IgniteLimit.class)
+ .and(l -> doubleFromRex(l.offset(), -1) == 2.0)
+ .and(l -> doubleFromRex(l.fetch(), -1) == 3.0)
+ .and(input(isInstanceOf(IgniteExchange.class)
+ .and(input(
+ isInstanceOf(IgniteSort.class)
+ .and(l ->
doubleFromRex(l.offset, -1) == 2.0)
+ .and(l ->
doubleFromRex(l.fetch, -1) == 3.0)
+ ))
+ ))
+ ))
+ );
+ }
+
/**
* Creates PUBLIC schema with one TEST table.
*/
