You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by am...@apache.org on 2023/01/23 10:38:34 UTC

[ignite-3] branch main updated: IGNITE-18207 Sql. Pushdown DISTINCT aggregate with no particular function (#1536)

This is an automated email from the ASF dual-hosted git repository.

amashenkov 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 e7a624c4ad IGNITE-18207 Sql. Pushdown DISTINCT aggregate with no particular function (#1536)
e7a624c4ad is described below

commit e7a624c4adda2e16296a83568456f6460ab02cc6
Author: Andrew V. Mashenkov <AM...@users.noreply.github.com>
AuthorDate: Mon Jan 23 13:38:28 2023 +0300

    IGNITE-18207 Sql. Pushdown DISTINCT aggregate with no particular function (#1536)
---
 .../sql/engine/AbstractBasicIntegrationTest.java   | 44 ++++++++++++++++++----
 .../internal/sql/engine/ItAggregatesTest.java      | 29 ++++++++++++++
 .../internal/sql/engine/util/QueryChecker.java     | 29 +++++++++++++-
 3 files changed, 93 insertions(+), 9 deletions(-)

diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/AbstractBasicIntegrationTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/AbstractBasicIntegrationTest.java
index 6696801757..2fda26f39f 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/AbstractBasicIntegrationTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/AbstractBasicIntegrationTest.java
@@ -31,14 +31,11 @@ import static org.junit.jupiter.api.Assertions.assertSame;
 
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
 import java.util.stream.IntStream;
-import java.util.stream.Stream;
 import org.apache.ignite.Ignite;
 import org.apache.ignite.IgnitionManager;
 import org.apache.ignite.internal.app.IgniteImpl;
@@ -244,9 +241,22 @@ public class AbstractBasicIntegrationTest extends BaseIgniteAbstractTest {
      * @param rules Additional rules need to be disabled.
      */
     static QueryChecker assertQuery(String qry, JoinType joinType, String... rules) {
-        return AbstractBasicIntegrationTest.assertQuery(qry.replaceAll("(?i)^select", "select "
-                + Stream.concat(Arrays.stream(joinType.disabledRules), Arrays.stream(rules))
-                .collect(Collectors.joining("','", "/*+ DISABLE_RULE('", "') */"))));
+        return assertQuery(qry)
+                .disableRules(joinType.disabledRules)
+                .disableRules(rules);
+    }
+
+    /**
+     * Used for query with aggregates checks, disables other aggregate rules for executing exact agregate algo.
+     *
+     * @param qry Query for check.
+     * @param aggregateType Type of aggregate algo.
+     * @param rules Additional rules need to be disabled.
+     */
+    static QueryChecker assertQuery(String qry, AggregateType aggregateType, String... rules) {
+        return assertQuery(qry)
+                .disableRules(aggregateType.disabledRules)
+                .disableRules(rules);
     }
 
     /**
@@ -289,6 +299,26 @@ public class AbstractBasicIntegrationTest extends BaseIgniteAbstractTest {
         }
     }
 
+    enum AggregateType {
+        SORT(
+                "ColocatedHashAggregateConverterRule",
+                "ColocatedSortAggregateConverterRule",
+                "MapReduceHashAggregateConverterRule"
+        ),
+
+        HASH(
+                "ColocatedHashAggregateConverterRule",
+                "ColocatedSortAggregateConverterRule",
+                "MapReduceSortAggregateConverterRule"
+        );
+
+        private final String[] disabledRules;
+
+        AggregateType(String... disabledRules) {
+            this.disabledRules = disabledRules;
+        }
+    }
+
     protected static void createAndPopulateTable() {
         createTable(DEFAULT_TABLE_NAME, 1, 8);
 
@@ -334,7 +364,7 @@ public class AbstractBasicIntegrationTest extends BaseIgniteAbstractTest {
 
             assert id != null : "Primary key cannot be null";
 
-            Tuple row  = view.get(null, Tuple.create().set(columnNames[0], id));
+            Tuple row = view.get(null, Tuple.create().set(columnNames[0], id));
 
             assertNotNull(row);
 
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItAggregatesTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItAggregatesTest.java
index 656467fe55..476741c487 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItAggregatesTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItAggregatesTest.java
@@ -216,4 +216,33 @@ public class ItAggregatesTest extends AbstractBasicIntegrationTest {
                 .returns("val1", 50L)
                 .check();
     }
+
+    @Test
+    public void distinctAggregateWithoutAggregateFunction() {
+        var sql = "select distinct name from person";
+
+        assertQuery(sql)
+                .matches(QueryChecker.matches(".*Colocated.*Aggregate.*Exchange.*"))
+                .returns("Igor")
+                .returns("Ilya")
+                .returns("Roma")
+                .returns(null)
+                .check();
+
+        assertQuery(sql, AggregateType.HASH)
+                .matches(QueryChecker.matches(".*ReduceHashAggregate.*Exchange.*MapHashAggregate.*"))
+                .returns("Igor")
+                .returns("Ilya")
+                .returns("Roma")
+                .returns(null)
+                .check();
+
+        assertQuery(sql, AggregateType.SORT)
+                .matches(QueryChecker.matches(".*ReduceSortAggregate.*Exchange.*MapSortAggregate.*"))
+                .returns("Igor")
+                .returns("Ilya")
+                .returns("Roma")
+                .returns(null)
+                .check();
+    }
 }
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java
index a1b0d50c6f..6dbdf90b88 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java
@@ -233,10 +233,12 @@ public abstract class QueryChecker {
         );
     }
 
-    private final String qry;
+    private final String originalQuery;
 
     private final ArrayList<Matcher<String>> planMatchers = new ArrayList<>();
 
+    private final ArrayList<String> disabledRules = new ArrayList<>();
+
     private List<List<?>> expectedResult;
 
     private List<String> expectedColumnNames;
@@ -257,7 +259,7 @@ public abstract class QueryChecker {
      * @param qry Query.
      */
     public QueryChecker(String qry) {
-        this.qry = qry;
+        this.originalQuery = qry;
     }
 
     /**
@@ -287,6 +289,20 @@ public abstract class QueryChecker {
         return this;
     }
 
+    /**
+     * Disables rules.
+     *
+     * @param rules Rules to disable.
+     * @return This.
+     */
+    public QueryChecker disableRules(String... rules) {
+        if (rules != null) {
+            Arrays.stream(rules).filter(Objects::nonNull).forEach(disabledRules::add);
+        }
+
+        return this;
+    }
+
     /**
      * This method add the given row to the list of expected, the order of enumeration does not matter unless {@link #ordered()} is set.
      *
@@ -370,6 +386,15 @@ public abstract class QueryChecker {
         // Check plan.
         QueryProcessor qryProc = getEngine();
 
+        String qry = originalQuery;
+
+        if (!disabledRules.isEmpty()) {
+            assert qry.matches("(?i)^select .*") : "SELECT query was expected: " + originalQuery;
+
+            qry = qry.replaceAll("(?i)^select", "select "
+                    + disabledRules.stream().collect(Collectors.joining("','", "/*+ DISABLE_RULE('", "') */")));
+        }
+
         var explainCursors = qryProc.queryAsync("PUBLIC", "EXPLAIN PLAN FOR " + qry, params);
 
         var explainCursor = explainCursors.get(0).join();