[GitHub] [ignite-3] zstan commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort

[GitBox <gi...@apache.org>]

zstan commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073472554


##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##########
@@ -67,8 +70,118 @@ public void mapReduceAggregate() {
 
     @Test
     public void correctCollationsOnMapReduceSortAgg() {
-        var cursors = sql("SELECT PK FROM TEST_ONE_COL_IDX WHERE col0 IN (SELECT col0 FROM TEST_ONE_COL_IDX)");
+        String disabledRules = " /*+ DISABLE_RULE('MapReduceHashAggregateConverterRule', 'ColocatedSortAggregateConverterRule') */ ";
+
+        var cursors = sql(
+                appendDisabledRules("SELECT PK FROM TEST_ONE_COL_IDX WHERE col0 IN (SELECT col0 FROM TEST_ONE_COL_IDX)", disabledRules));
 
         assertEquals(ROWS, cursors.size());
     }
+
+    @Test
+    @WithSystemProperty(key = "IMPLICIT_PK_ENABLED", value = "true")
+    //TODO: rewrite with QueryChecker after
+    // https://issues.apache.org/jira/browse/IGNITE-18501 (QueryChecker still use deprecated QueryProcessor#queryAsync method)
+    public void testDifferentCollocatedSortAgg() {
+        try {
+            sql("CREATE TABLE testMe (a INTEGER, b INTEGER, s VARCHAR);");
+            sql("INSERT INTO testMe VALUES (11, 1, 'hello'), (12, 2, 'world'), (11, 3, NULL)");
+            sql("INSERT INTO testMe VALUES (11, 3, 'hello'), (12, 2, 'world'), (10, 5, 'ahello'), (13, 6, 'world')");
+
+            String disabledRules = " /*+ DISABLE_RULE('MapReduceHashAggregateConverterRule', 'MapReduceSortAggregateConverterRule', "
+                    + "'ColocatedHashAggregateConverterRule') */ ";
+
+            List<List<Object>> res = sql(appendDisabledRules("SELECT DISTINCT(a) as a FROM testMe ORDER BY a", disabledRules));
+            List<Object> res0 = transformResults(res);
+            assertEquals(4, res.size());
+            assertEquals(List.of(10, 11, 12, 13), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(*) FROM testMe", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(1, res.size());
+            assertEquals(List.of(7L), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(a), COUNT(DISTINCT(b)) FROM testMe", disabledRules));
+
+            assertEquals(1, res.size());
+            assertEquals(List.of(7L, 5L), res.get(0));
+
+            res = sql(appendDisabledRules("SELECT COUNT(a) as a, s FROM testMe GROUP BY s ORDER BY a, s", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(4, res.size());
+            assertEquals(List.of(List.of(1L, "ahello"), Arrays.asList(1L, null), List.of(2L, "hello"), List.of(3L, "world")), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(a) as a, AVG(a) as b, MIN(a), MIN(b), s FROM testMe GROUP BY s ORDER BY a, b",
+                    disabledRules));
+            res0 = transformResults(res);
+            assertEquals(4, res.size());
+            assertEquals(List.of(List.of(1L, 10, 10, 5, "ahello"), Arrays.asList(1L, 11, 11, 3, null), List.of(2L, 11, 11, 1, "hello"),
+                    List.of(3L, 12, 12, 2, "world")), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(a) as a, AVG(a) as bb, MIN(a), MIN(b), s FROM testMe GROUP BY s, b ORDER BY a, s",
+                    disabledRules));
+            res0 = transformResults(res);
+            assertEquals(6, res.size());
+            assertEquals(List.of(List.of(1L, 10, 10, 5, "ahello"), List.of(1L, 11, 11, 1, "hello"), List.of(1L, 11, 11, 3, "hello"),
+                    List.of(1L, 13, 13, 6, "world"), Arrays.asList(1L, 11, 11, 3, null), List.of(2L, 12, 12, 2, "world")), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(a) FROM testMe", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(1, res.size());
+            assertEquals(List.of(7L), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(DISTINCT(a)) FROM testMe", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(1, res.size());
+            assertEquals(List.of(4L), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(a), COUNT(s), COUNT(*) FROM testMe", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(1, res.size());
+            assertEquals(List.of(7L, 6L, 7L), res0.get(0));
+
+            res = sql(appendDisabledRules("SELECT AVG(a) FROM testMe", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(1, res.size());
+            assertEquals(List.of(11), res0);
+
+            res = sql(appendDisabledRules("SELECT MIN(a) FROM testMe", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(1, res.size());
+            assertEquals(List.of(10), res0);
+
+            res = sql(appendDisabledRules("SELECT COUNT(a), COUNT(DISTINCT(a)) FROM testMe", disabledRules));
+            res0 = transformResults(res);
+            assertEquals(1, res.size());
+            assertEquals(List.of(7L, 4L), res0.get(0));
+        } finally {
+            sql("DROP TABLE testMe");
+        }
+    }
+
+    private String appendDisabledRules(String sql, String rules) {

Review Comment:
   I understand but seems that check output results with **sql()** is not comfortable as with QueryChecker (which need to be rafactored a bit, issue is linked above) let`s do it in checker ? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org