You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2023/01/16 07:22:17 UTC
[GitHub] [ignite-3] zstan opened a new pull request, #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
zstan opened a new pull request, #1524:
URL: https://github.com/apache/ignite-3/pull/1524
…ith additional sort
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1083907403
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##########
@@ -83,6 +84,10 @@ public SortAggregateNode(
this.rowFactory = rowFactory;
this.grpSet = grpSet;
this.comp = comp;
+
+ if ((type == AggregateType.REDUCE || type == AggregateType.SINGLE) && accFactory != null && grpSet.isEmpty()) {
Review Comment:
what about rewind? (<-- let's add execution test for this case as well)
--
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
[GitHub] [ignite-3] zstan closed pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan closed pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
URL: https://github.com/apache/ignite-3/pull/1524
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082427170
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##########
@@ -159,6 +161,10 @@ public void end() throws Exception {
waiting = -1;
+ if (grp == null && accFactory != null) {
Review Comment:
this will produce empty row when none is expected in query like `SELECT min(c) FROM t GROUP BY a` when table is empty. BTW, we need execution test for this case
to initialise group properly, please, take a look at `org.apache.ignite.internal.sql.engine.exec.rel.HashAggregateNode.Grouping#init`
--
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
[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
Posted by GitBox <gi...@apache.org>.
zstan commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073476395
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java:
##########
@@ -389,52 +390,45 @@ public void mapAggregateWithoutDistinctAcc() throws Exception {
publicSchema.addTable(tbl);
- // TODO: https://issues.apache.org/jira/browse/IGNITE-18464 Colocated sort aggregates need to compose a plans with additional sort
- //checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
-
- checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(*) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1, grp0", publicSchema);
checkDistinctInMapAggNode("SELECT val1, COUNT(DISTINCT val0) as v1 FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT val0 FROM test WHERE VAL1 = ANY(SELECT DISTINCT val1 FROM test)", publicSchema);
}
/**
- * Check that plan does not contain distinct accumulators on map nodes.
+ * Check that plan does not contain distinct accumulators on map nodes with additional expectations.
*
* @param sql Request string.
* @param publicSchema Schema.
* @throws Exception If failed.
*/
private void checkDistinctInMapAggNode(String sql, IgniteSchema publicSchema) throws Exception {
- List<Pair<String[], Boolean>> disabledRules = List.of(new Pair<>(new String[]{""}, false),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule"}, true),
+ log.info("++check: " + sql);
Review Comment:
yep, but in case of exception someone need to append the same logging for understanding the case. Do you still think i need to remove it ?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java:
##########
@@ -389,52 +390,45 @@ public void mapAggregateWithoutDistinctAcc() throws Exception {
publicSchema.addTable(tbl);
- // TODO: https://issues.apache.org/jira/browse/IGNITE-18464 Colocated sort aggregates need to compose a plans with additional sort
- //checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
-
- checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(*) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1, grp0", publicSchema);
checkDistinctInMapAggNode("SELECT val1, COUNT(DISTINCT val0) as v1 FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT val0 FROM test WHERE VAL1 = ANY(SELECT DISTINCT val1 FROM test)", publicSchema);
}
/**
- * Check that plan does not contain distinct accumulators on map nodes.
+ * Check that plan does not contain distinct accumulators on map nodes with additional expectations.
*
* @param sql Request string.
* @param publicSchema Schema.
* @throws Exception If failed.
*/
private void checkDistinctInMapAggNode(String sql, IgniteSchema publicSchema) throws Exception {
- List<Pair<String[], Boolean>> disabledRules = List.of(new Pair<>(new String[]{""}, false),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule"}, true),
+ log.info("++check: " + sql);
+ List<Pair<String[], Predicate<IgniteRel>>> disabledRules = List.of(
new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceSortAggregateConverterRule"}, true),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceHashAggregateConverterRule"}, true)
+ "MapReduceSortAggregateConverterRule"}, node -> !findNodes(node, byClass(IgniteMapAggregateBase.class)).isEmpty()),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedHashAggregateConverterRule"}, node -> true),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedSortAggregateConverterRule"}, node -> true)
);
- for (Pair<String[], Boolean> rules : disabledRules) {
+ for (Pair<String[], Predicate<IgniteRel>> rules : disabledRules) {
+ log.info("+++disable rules: " + Arrays.toString(rules.getFirst()));
Review Comment:
yep, but in case of exception someone need to append the same logging for understanding the case. Do you still think i need to remove it ?
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1083913680
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##########
@@ -67,8 +70,134 @@ 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') */ ";
Review Comment:
the same question here
--
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
[GitHub] [ignite-3] zstan commented on pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#issuecomment-1399288641
@korlov42 i fix all your comments, plz check ?
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1080923365
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java:
##########
@@ -389,52 +390,45 @@ public void mapAggregateWithoutDistinctAcc() throws Exception {
publicSchema.addTable(tbl);
- // TODO: https://issues.apache.org/jira/browse/IGNITE-18464 Colocated sort aggregates need to compose a plans with additional sort
- //checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
-
- checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(*) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1, grp0", publicSchema);
checkDistinctInMapAggNode("SELECT val1, COUNT(DISTINCT val0) as v1 FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT val0 FROM test WHERE VAL1 = ANY(SELECT DISTINCT val1 FROM test)", publicSchema);
}
/**
- * Check that plan does not contain distinct accumulators on map nodes.
+ * Check that plan does not contain distinct accumulators on map nodes with additional expectations.
*
* @param sql Request string.
* @param publicSchema Schema.
* @throws Exception If failed.
*/
private void checkDistinctInMapAggNode(String sql, IgniteSchema publicSchema) throws Exception {
- List<Pair<String[], Boolean>> disabledRules = List.of(new Pair<>(new String[]{""}, false),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule"}, true),
+ log.info("++check: " + sql);
+ List<Pair<String[], Predicate<IgniteRel>>> disabledRules = List.of(
new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceSortAggregateConverterRule"}, true),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceHashAggregateConverterRule"}, true)
+ "MapReduceSortAggregateConverterRule"}, node -> !findNodes(node, byClass(IgniteMapAggregateBase.class)).isEmpty()),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedHashAggregateConverterRule"}, node -> true),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedSortAggregateConverterRule"}, node -> true)
);
- for (Pair<String[], Boolean> rules : disabledRules) {
+ for (Pair<String[], Predicate<IgniteRel>> rules : disabledRules) {
+ log.info("+++disable rules: " + Arrays.toString(rules.getFirst()));
Review Comment:
You can add a message to the next assertTrue() call.
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073229917
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSetOpTest.java:
##########
@@ -105,7 +105,7 @@ public void testExceptNested() {
}
@Test
- @Disabled("https://issues.apache.org/jira/browse/IGNITE-18469")
+ //@Disabled("https://issues.apache.org/jira/browse/IGNITE-18469")
Review Comment:
Let's either uncomment or remove the comment
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073302342
##########
modules/runner/src/integrationTest/sql/aggregate/aggregates/test_aggr_string.test:
##########
@@ -80,3 +117,6 @@ SELECT MAX(s), MIN(s) FROM test_strings2;
aa A
+
+
+
Review Comment:
```suggestion
```
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082303971
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/AbstractIgniteConverterRule.java:
##########
@@ -24,6 +24,7 @@
import org.apache.calcite.rel.convert.ConverterRule;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.ignite.internal.sql.engine.rel.IgniteConvention;
+import org.checkerframework.checker.nullness.qual.Nullable;
Review Comment:
I believe it has to be `org.jetbrains.annotations`
--
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
[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
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1083310431
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##########
@@ -159,6 +161,10 @@ public void end() throws Exception {
waiting = -1;
+ if (grp == null && accFactory != null) {
Review Comment:
got it!
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1083908015
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##########
@@ -119,7 +124,11 @@ public void push(RowT row) throws Exception {
waiting--;
if (grp != null) {
- int cmp = comp.compare(row, prevRow);
+ int cmp = 0;
+
+ if (comp != null) {
Review Comment:
comparator is never null
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073311278
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java:
##########
@@ -389,52 +390,45 @@ public void mapAggregateWithoutDistinctAcc() throws Exception {
publicSchema.addTable(tbl);
- // TODO: https://issues.apache.org/jira/browse/IGNITE-18464 Colocated sort aggregates need to compose a plans with additional sort
- //checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
-
- checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(*) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1, grp0", publicSchema);
checkDistinctInMapAggNode("SELECT val1, COUNT(DISTINCT val0) as v1 FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT val0 FROM test WHERE VAL1 = ANY(SELECT DISTINCT val1 FROM test)", publicSchema);
}
/**
- * Check that plan does not contain distinct accumulators on map nodes.
+ * Check that plan does not contain distinct accumulators on map nodes with additional expectations.
*
* @param sql Request string.
* @param publicSchema Schema.
* @throws Exception If failed.
*/
private void checkDistinctInMapAggNode(String sql, IgniteSchema publicSchema) throws Exception {
- List<Pair<String[], Boolean>> disabledRules = List.of(new Pair<>(new String[]{""}, false),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule"}, true),
+ log.info("++check: " + sql);
+ List<Pair<String[], Predicate<IgniteRel>>> disabledRules = List.of(
new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceSortAggregateConverterRule"}, true),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceHashAggregateConverterRule"}, true)
+ "MapReduceSortAggregateConverterRule"}, node -> !findNodes(node, byClass(IgniteMapAggregateBase.class)).isEmpty()),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedHashAggregateConverterRule"}, node -> true),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedSortAggregateConverterRule"}, node -> true)
);
- for (Pair<String[], Boolean> rules : disabledRules) {
+ for (Pair<String[], Predicate<IgniteRel>> rules : disabledRules) {
+ log.info("+++disable rules: " + Arrays.toString(rules.getFirst()));
Review Comment:
```suggestion
```
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073322335
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java:
##########
@@ -389,52 +390,45 @@ public void mapAggregateWithoutDistinctAcc() throws Exception {
publicSchema.addTable(tbl);
- // TODO: https://issues.apache.org/jira/browse/IGNITE-18464 Colocated sort aggregates need to compose a plans with additional sort
- //checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
-
- checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(*) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1, grp0", publicSchema);
checkDistinctInMapAggNode("SELECT val1, COUNT(DISTINCT val0) as v1 FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT val0 FROM test WHERE VAL1 = ANY(SELECT DISTINCT val1 FROM test)", publicSchema);
}
/**
- * Check that plan does not contain distinct accumulators on map nodes.
+ * Check that plan does not contain distinct accumulators on map nodes with additional expectations.
*
* @param sql Request string.
* @param publicSchema Schema.
* @throws Exception If failed.
*/
private void checkDistinctInMapAggNode(String sql, IgniteSchema publicSchema) throws Exception {
- List<Pair<String[], Boolean>> disabledRules = List.of(new Pair<>(new String[]{""}, false),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule"}, true),
+ log.info("++check: " + sql);
+ List<Pair<String[], Predicate<IgniteRel>>> disabledRules = List.of(
new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceSortAggregateConverterRule"}, true),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceHashAggregateConverterRule"}, true)
+ "MapReduceSortAggregateConverterRule"}, node -> !findNodes(node, byClass(IgniteMapAggregateBase.class)).isEmpty()),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedHashAggregateConverterRule"}, node -> true),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedSortAggregateConverterRule"}, node -> true)
Review Comment:
I've just found we use same rule-sets in different tests to disable some features.
We can extract e.g. 'enum' with these rules, which forces planner to force/disable features.
Please take a look at my PR, maybe we can go the better way here and there.
https://github.com/apache/ignite-3/pull/1536
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082404654
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##########
@@ -76,7 +75,6 @@ public SortAggregateNode(
Comparator<RowT> comp
) {
super(ctx);
- assert Objects.nonNull(comp);
Review Comment:
I would prefer to leave this assertion as is.
Without this assertion, a situation in which we would have previously obtained either an assertion error or an NPE would now lead to the wrong result.
Perhaps, we could cover single group case by providing comparator returning 0 every time, WDYT?
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1083924778
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##########
@@ -67,8 +70,134 @@ 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");
+ }
+ }
+
+ @WithSystemProperty(key = "IMPLICIT_PK_ENABLED", value = "true")
+ @Test
+ public void checkEmptyTable() {
Review Comment:
does it make sense to verify both colocated and 2-phase aggregates?
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073235716
##########
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:
Maybe, it would be better to extend parent `sql` method with disabled rules instead of this one. WDYT?
```
String sql(String sql, String... rules) {
```
or
```
private String appendDisabledRules(String sql, List<String> rules) {
```
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073235716
##########
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:
Maybe, it would be better to extend parent `sql` method with disabled rules instead of this one. WDYT?
`String sql(String sql, String... rules) {`
or
`String sql(String sql, List<String> rules) {`
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082304386
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/SortAggregateConverterRule.java:
##########
@@ -36,6 +36,7 @@
import org.apache.ignite.internal.sql.engine.trait.IgniteDistributions;
import org.apache.ignite.internal.sql.engine.trait.TraitUtils;
import org.apache.ignite.internal.sql.engine.util.HintUtils;
+import org.checkerframework.checker.nullness.qual.Nullable;
Review Comment:
the same
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073351703
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/agg/IgniteColocatedSortAggregate.java:
##########
@@ -57,9 +57,6 @@ public IgniteColocatedSortAggregate(
) {
super(cluster, traitSet, input, groupSet, groupSets, aggCalls);
- assert !TraitUtils.collation(traitSet).isDefault();
- assert !groupSet.isEmpty() && groupSets.size() == 1;
Review Comment:
Are these assertion totally incorrect? or just another case become possible and assert(s) can be changed?
--
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
[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
Posted by 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
[GitHub] [ignite-3] zstan commented on pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#issuecomment-1401656343
@korlov42 all done
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073310602
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java:
##########
@@ -389,52 +390,45 @@ public void mapAggregateWithoutDistinctAcc() throws Exception {
publicSchema.addTable(tbl);
- // TODO: https://issues.apache.org/jira/browse/IGNITE-18464 Colocated sort aggregates need to compose a plans with additional sort
- //checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
-
- checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(*) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1, grp0", publicSchema);
checkDistinctInMapAggNode("SELECT val1, COUNT(DISTINCT val0) as v1 FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT val0 FROM test WHERE VAL1 = ANY(SELECT DISTINCT val1 FROM test)", publicSchema);
}
/**
- * Check that plan does not contain distinct accumulators on map nodes.
+ * Check that plan does not contain distinct accumulators on map nodes with additional expectations.
*
* @param sql Request string.
* @param publicSchema Schema.
* @throws Exception If failed.
*/
private void checkDistinctInMapAggNode(String sql, IgniteSchema publicSchema) throws Exception {
- List<Pair<String[], Boolean>> disabledRules = List.of(new Pair<>(new String[]{""}, false),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule"}, true),
+ log.info("++check: " + sql);
Review Comment:
```suggestion
```
--
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
[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
Posted by GitBox <gi...@apache.org>.
zstan commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073484635
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java:
##########
@@ -389,52 +390,45 @@ public void mapAggregateWithoutDistinctAcc() throws Exception {
publicSchema.addTable(tbl);
- // TODO: https://issues.apache.org/jira/browse/IGNITE-18464 Colocated sort aggregates need to compose a plans with additional sort
- //checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
- //checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
-
- checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(*) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test", publicSchema);
+ checkDistinctInMapAggNode("SELECT COUNT(DISTINCT val0) FROM test GROUP BY val1, grp0", publicSchema);
checkDistinctInMapAggNode("SELECT val1, COUNT(DISTINCT val0) as v1 FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT AVG(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT SUM(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MIN(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT MAX(DISTINCT val0) FROM test GROUP BY val1", publicSchema);
-
checkDistinctInMapAggNode("SELECT val0 FROM test WHERE VAL1 = ANY(SELECT DISTINCT val1 FROM test)", publicSchema);
}
/**
- * Check that plan does not contain distinct accumulators on map nodes.
+ * Check that plan does not contain distinct accumulators on map nodes with additional expectations.
*
* @param sql Request string.
* @param publicSchema Schema.
* @throws Exception If failed.
*/
private void checkDistinctInMapAggNode(String sql, IgniteSchema publicSchema) throws Exception {
- List<Pair<String[], Boolean>> disabledRules = List.of(new Pair<>(new String[]{""}, false),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule"}, true),
+ log.info("++check: " + sql);
+ List<Pair<String[], Predicate<IgniteRel>>> disabledRules = List.of(
new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceSortAggregateConverterRule"}, true),
- new Pair<>(new String[]{"ColocatedHashAggregateConverterRule", "ColocatedSortAggregateConverterRule",
- "MapReduceHashAggregateConverterRule"}, true)
+ "MapReduceSortAggregateConverterRule"}, node -> !findNodes(node, byClass(IgniteMapAggregateBase.class)).isEmpty()),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedHashAggregateConverterRule"}, node -> true),
+ new Pair<>(new String[]{"MapReduceHashAggregateConverterRule", "MapReduceSortAggregateConverterRule",
+ "ColocatedSortAggregateConverterRule"}, node -> true)
Review Comment:
thanks, i check your PR and have no clear ideas here, probably after your and my issues have been merged we make additional issue and introduce such an enum ?
--
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
[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
Posted by GitBox <gi...@apache.org>.
zstan commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1073477767
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/agg/IgniteColocatedSortAggregate.java:
##########
@@ -57,9 +57,6 @@ public IgniteColocatedSortAggregate(
) {
super(cluster, traitSet, input, groupSet, groupSets, aggCalls);
- assert !TraitUtils.collation(traitSet).isDefault();
- assert !groupSet.isEmpty() && groupSets.size() == 1;
Review Comment:
another case become possible. for example select *
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1083913316
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##########
@@ -48,12 +53,10 @@ static void initTestData() {
@Test
public void mapReduceAggregate() {
+ String disabledRules = " /*+ DISABLE_RULE('MapReduceHashAggregateConverterRule', 'ColocatedSortAggregateConverterRule') */ ";
Review Comment:
does it make sense to disable colocated hash aggregate rule as well?
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1083927134
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##########
@@ -159,6 +161,10 @@ public void end() throws Exception {
waiting = -1;
+ if (grp == null && accFactory != null) {
Review Comment:
let's add _execution_ test for this case
--
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
[GitHub] [ignite-3] ygerzhedovich commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
Posted by "ygerzhedovich (via GitHub)" <gi...@apache.org>.
ygerzhedovich commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1085616852
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##########
@@ -67,8 +71,142 @@ 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', 'ColocatedHashAggregateConverterRule', "
+ + "'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)
Review Comment:
The main branch already fixed and use querySingleAsync method
--
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