You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2021/06/28 18:03:46 UTC
Change in asterixdb[mad-hatter]: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
From Dmitry Lychagin <dm...@couchbase.com>:
Dmitry Lychagin has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104 )
Change subject: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
......................................................................
[ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Fix incorrect result caused PushGroupByThroughProduct rule
- Add testcase
Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
---
M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
6 files changed, 110 insertions(+), 7 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/04/12104/1
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
index 48c4324..6d92b51 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
@@ -84,24 +84,27 @@
Mutable<ILogicalOperator> opLeftRef = join.getInputs().get(0);
ILogicalOperator opLeft = opLeftRef.getValue();
+ Mutable<ILogicalOperator> opRightRef = join.getInputs().get(1);
+ ILogicalOperator opRight = opRightRef.getValue();
switch (canPushThrough(gby, opLeft, decorToPush, decorNotToPush)) {
case REPEATED_DECORS: {
return false;
}
case TRUE: {
- push(opRef, opRef2, 0, decorToPush, decorNotToPush, context);
- return true;
+ if (OperatorPropertiesUtil.isCardinalityZeroOrOne(opRight)) {
+ push(opRef, opRef2, 0, decorToPush, decorNotToPush, context);
+ return true;
+ }
+ return false;
}
case FALSE: {
decorToPush.clear();
- Mutable<ILogicalOperator> opRightRef = join.getInputs().get(1);
- ILogicalOperator opRight = opRightRef.getValue();
- if (canPushThrough(gby, opRight, decorToPush, decorNotToPush) == PushTestResult.TRUE) {
+ if (canPushThrough(gby, opRight, decorToPush, decorNotToPush) == PushTestResult.TRUE
+ && OperatorPropertiesUtil.isCardinalityZeroOrOne(opLeft)) {
push(opRef, opRef2, 1, decorToPush, decorNotToPush, context);
return true;
- } else {
- return false;
}
+ return false;
}
default: {
throw new IllegalStateException();
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
new file mode 100644
index 0000000..10f5546
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ * Description : Testing group by over cross product (ASTERIXDB-2917)
+ * Expected : SUCCESS
+ */
+
+
+drop dataverse test if exists;
+create dataverse test;
+
+use test;
+
+create dataset t1(id integer not unknown) open type primary key id;
+
+create dataset t2(id integer not unknown) open type primary key id;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
new file mode 100644
index 0000000..7fe3777
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use test;
+
+insert into t1 ([
+ { "id": 1, "x": "a" },
+ { "id": 2, "x": "a" },
+ { "id": 3, "x": "a" },
+ { "id": 4, "x": "b" },
+ { "id": 5, "x": "b" },
+ { "id": 6, "x": "b" }
+]);
+
+insert into t2 ([
+ { "id": 11, "y": "c" },
+ { "id": 12, "y": "c" }
+]);
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
new file mode 100644
index 0000000..34ec706
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use test;
+
+select t1.x, count(*) as cnt
+from t2, t1
+group by t1.x
+order by t1.x;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
new file mode 100644
index 0000000..e1e3cf1
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
@@ -0,0 +1,2 @@
+{ "cnt": 6, "x": "a" }
+{ "cnt": 6, "x": "b" }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index 4c034ad..8d06f71 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -5323,6 +5323,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="group-by">
+ <compilation-unit name="gby-cross-join">
+ <output-dir compare="Text">gby-cross-join</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="group-by">
<compilation-unit name="gby-nested-01">
<output-dir compare="Text">gby-nested-01</output-dir>
</compilation-unit>
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: mad-hatter
Gerrit-Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
Gerrit-Change-Number: 12104
Gerrit-PatchSet: 1
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-MessageType: newchange
Change in asterixdb[mad-hatter]: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Ali Alsuliman <al...@gmail.com>:
Ali Alsuliman has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104 )
Change subject: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: mad-hatter
Gerrit-Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
Gerrit-Change-Number: 12104
Gerrit-PatchSet: 1
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Comment-Date: Mon, 28 Jun 2021 21:35:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Change in asterixdb[mad-hatter]: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Dmitry Lychagin <dm...@couchbase.com>:
Dmitry Lychagin has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104 )
Change subject: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
......................................................................
[ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Fix incorrect result caused PushGroupByThroughProduct rule
- Add testcase
Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
---
M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
6 files changed, 110 insertions(+), 7 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/04/12104/1
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
index 48c4324..6d92b51 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
@@ -84,24 +84,27 @@
Mutable<ILogicalOperator> opLeftRef = join.getInputs().get(0);
ILogicalOperator opLeft = opLeftRef.getValue();
+ Mutable<ILogicalOperator> opRightRef = join.getInputs().get(1);
+ ILogicalOperator opRight = opRightRef.getValue();
switch (canPushThrough(gby, opLeft, decorToPush, decorNotToPush)) {
case REPEATED_DECORS: {
return false;
}
case TRUE: {
- push(opRef, opRef2, 0, decorToPush, decorNotToPush, context);
- return true;
+ if (OperatorPropertiesUtil.isCardinalityZeroOrOne(opRight)) {
+ push(opRef, opRef2, 0, decorToPush, decorNotToPush, context);
+ return true;
+ }
+ return false;
}
case FALSE: {
decorToPush.clear();
- Mutable<ILogicalOperator> opRightRef = join.getInputs().get(1);
- ILogicalOperator opRight = opRightRef.getValue();
- if (canPushThrough(gby, opRight, decorToPush, decorNotToPush) == PushTestResult.TRUE) {
+ if (canPushThrough(gby, opRight, decorToPush, decorNotToPush) == PushTestResult.TRUE
+ && OperatorPropertiesUtil.isCardinalityZeroOrOne(opLeft)) {
push(opRef, opRef2, 1, decorToPush, decorNotToPush, context);
return true;
- } else {
- return false;
}
+ return false;
}
default: {
throw new IllegalStateException();
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
new file mode 100644
index 0000000..10f5546
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ * Description : Testing group by over cross product (ASTERIXDB-2917)
+ * Expected : SUCCESS
+ */
+
+
+drop dataverse test if exists;
+create dataverse test;
+
+use test;
+
+create dataset t1(id integer not unknown) open type primary key id;
+
+create dataset t2(id integer not unknown) open type primary key id;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
new file mode 100644
index 0000000..7fe3777
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use test;
+
+insert into t1 ([
+ { "id": 1, "x": "a" },
+ { "id": 2, "x": "a" },
+ { "id": 3, "x": "a" },
+ { "id": 4, "x": "b" },
+ { "id": 5, "x": "b" },
+ { "id": 6, "x": "b" }
+]);
+
+insert into t2 ([
+ { "id": 11, "y": "c" },
+ { "id": 12, "y": "c" }
+]);
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
new file mode 100644
index 0000000..34ec706
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use test;
+
+select t1.x, count(*) as cnt
+from t2, t1
+group by t1.x
+order by t1.x;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
new file mode 100644
index 0000000..e1e3cf1
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
@@ -0,0 +1,2 @@
+{ "cnt": 6, "x": "a" }
+{ "cnt": 6, "x": "b" }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index 4c034ad..8d06f71 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -5323,6 +5323,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="group-by">
+ <compilation-unit name="gby-cross-join">
+ <output-dir compare="Text">gby-cross-join</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="group-by">
<compilation-unit name="gby-nested-01">
<output-dir compare="Text">gby-nested-01</output-dir>
</compilation-unit>
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: mad-hatter
Gerrit-Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
Gerrit-Change-Number: 12104
Gerrit-PatchSet: 1
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-MessageType: newchange
Change in asterixdb[mad-hatter]: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Jenkins <je...@fulliautomatix.ics.uci.edu>:
Jenkins has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104 )
Change subject: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
......................................................................
Patch Set 1: Integration-Tests+1
Integration Tests Successful
https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/12140/ : SUCCESS
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: mad-hatter
Gerrit-Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
Gerrit-Change-Number: 12104
Gerrit-PatchSet: 1
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-CC: Anon. E. Moose #1000171
Gerrit-Comment-Date: Mon, 28 Jun 2021 19:18:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Change in asterixdb[mad-hatter]: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Dmitry Lychagin <dm...@couchbase.com>:
Dmitry Lychagin has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104 )
Change subject: [ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
......................................................................
[ASTERIXDB-2917][COMP] Incorrect result with group by over cross product
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Fix incorrect result caused PushGroupByThroughProduct rule
- Add testcase
Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
Reviewed-by: Ali Alsuliman <al...@gmail.com>
---
M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
6 files changed, 110 insertions(+), 7 deletions(-)
Approvals:
Dmitry Lychagin: Looks good to me, but someone else must approve
Ali Alsuliman: Looks good to me, approved
Jenkins: Verified; Verified
Objections:
Anon. E. Moose #1000171: Violations found
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
index 48c4324..6d92b51 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java
@@ -84,24 +84,27 @@
Mutable<ILogicalOperator> opLeftRef = join.getInputs().get(0);
ILogicalOperator opLeft = opLeftRef.getValue();
+ Mutable<ILogicalOperator> opRightRef = join.getInputs().get(1);
+ ILogicalOperator opRight = opRightRef.getValue();
switch (canPushThrough(gby, opLeft, decorToPush, decorNotToPush)) {
case REPEATED_DECORS: {
return false;
}
case TRUE: {
- push(opRef, opRef2, 0, decorToPush, decorNotToPush, context);
- return true;
+ if (OperatorPropertiesUtil.isCardinalityZeroOrOne(opRight)) {
+ push(opRef, opRef2, 0, decorToPush, decorNotToPush, context);
+ return true;
+ }
+ return false;
}
case FALSE: {
decorToPush.clear();
- Mutable<ILogicalOperator> opRightRef = join.getInputs().get(1);
- ILogicalOperator opRight = opRightRef.getValue();
- if (canPushThrough(gby, opRight, decorToPush, decorNotToPush) == PushTestResult.TRUE) {
+ if (canPushThrough(gby, opRight, decorToPush, decorNotToPush) == PushTestResult.TRUE
+ && OperatorPropertiesUtil.isCardinalityZeroOrOne(opLeft)) {
push(opRef, opRef2, 1, decorToPush, decorNotToPush, context);
return true;
- } else {
- return false;
}
+ return false;
}
default: {
throw new IllegalStateException();
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
new file mode 100644
index 0000000..10f5546
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.1.ddl.sqlpp
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ * Description : Testing group by over cross product (ASTERIXDB-2917)
+ * Expected : SUCCESS
+ */
+
+
+drop dataverse test if exists;
+create dataverse test;
+
+use test;
+
+create dataset t1(id integer not unknown) open type primary key id;
+
+create dataset t2(id integer not unknown) open type primary key id;
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
new file mode 100644
index 0000000..7fe3777
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.2.update.sqlpp
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use test;
+
+insert into t1 ([
+ { "id": 1, "x": "a" },
+ { "id": 2, "x": "a" },
+ { "id": 3, "x": "a" },
+ { "id": 4, "x": "b" },
+ { "id": 5, "x": "b" },
+ { "id": 6, "x": "b" }
+]);
+
+insert into t2 ([
+ { "id": 11, "y": "c" },
+ { "id": 12, "y": "c" }
+]);
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
new file mode 100644
index 0000000..34ec706
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-cross-join/gby-cross-join.3.query.sqlpp
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use test;
+
+select t1.x, count(*) as cnt
+from t2, t1
+group by t1.x
+order by t1.x;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
new file mode 100644
index 0000000..e1e3cf1
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/gby-cross-join/gby-cross-join.3.adm
@@ -0,0 +1,2 @@
+{ "cnt": 6, "x": "a" }
+{ "cnt": 6, "x": "b" }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index 4c034ad..8d06f71 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -5323,6 +5323,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="group-by">
+ <compilation-unit name="gby-cross-join">
+ <output-dir compare="Text">gby-cross-join</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="group-by">
<compilation-unit name="gby-nested-01">
<output-dir compare="Text">gby-nested-01</output-dir>
</compilation-unit>
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/12104
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: mad-hatter
Gerrit-Change-Id: I85400289d3096c4da38ed73c7576e7c8b79a4909
Gerrit-Change-Number: 12104
Gerrit-PatchSet: 2
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-MessageType: merged