You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by wa...@apache.org on 2016/11/11 01:39:23 UTC
asterixdb git commit: ASTERIXDB-1727: Fix an issue with multiple
aggregates in one group-by
Repository: asterixdb
Updated Branches:
refs/heads/master e4d919e52 -> f40081f5a
ASTERIXDB-1727: Fix an issue with multiple aggregates in one group-by
- Although the hash group-by hint is given, if multiple aggregate operators
exist in the subplan of group-by, the physical operator of the given group-by
should not be set as EXTERNAL_GROUP_BY since we don't support multiple aggregates
in the external group by physical operator.
Change-Id: Ifb5619cc3ece00ab83962d53e004f203684df9ee
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1343
Sonar-Qube: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Yingyi Bu <bu...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo
Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/f40081f5
Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/f40081f5
Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/f40081f5
Branch: refs/heads/master
Commit: f40081f5ad0146885e9f9a416598bf46661a2f54
Parents: e4d919e
Author: Taewoo Kim <wa...@yahoo.com>
Authored: Thu Nov 10 14:39:37 2016 -0800
Committer: Taewoo Kim <wa...@yahoo.com>
Committed: Thu Nov 10 17:38:45 2016 -0800
----------------------------------------------------------------------
.../rules/SetAsterixPhysicalOperatorsRule.java | 14 ++++-
.../query-ASTERIXDB-1727.1.query.aql | 64 ++++++++++++++++++++
.../query-ASTERIXDB-1727.1.adm | 2 +
.../src/test/resources/runtimets/testsuite.xml | 5 ++
.../SetAlgebricksPhysicalOperatorsRule.java | 10 +++
5 files changed, 94 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/f40081f5/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java
index 677b9a7..473a2a5 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java
@@ -139,7 +139,19 @@ public class SetAsterixPhysicalOperatorsRule implements IAlgebraicRewriteRule {
}
}
- if (hasIntermediateAgg) {
+ // Check whether there are multiple aggregates in the sub plan.
+ // Currently, we don't support multiple aggregates in one external group-by.
+ boolean multipleAggOpsFound = false;
+ ILogicalOperator r1Logical = aggOp;
+ while (r1Logical.hasInputs()) {
+ r1Logical = r1Logical.getInputs().get(0).getValue();
+ if (r1Logical.getOperatorTag() == LogicalOperatorTag.AGGREGATE) {
+ multipleAggOpsFound = true;
+ break;
+ }
+ }
+
+ if (hasIntermediateAgg && !multipleAggOpsFound) {
for (int i = 0; i < aggNum; i++) {
AbstractFunctionCallExpression expr = (AbstractFunctionCallExpression) aggExprs
.get(i).getValue();
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/f40081f5/asterixdb/asterix-app/src/test/resources/runtimets/queries/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.query.aql
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.query.aql b/asterixdb/asterix-app/src/test/resources/runtimets/queries/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.query.aql
new file mode 100644
index 0000000..955f820
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.query.aql
@@ -0,0 +1,64 @@
+/*
+ * 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 : Tests that the exception in ASTERIDXDB-1727 issue is not reproduced.
+ * Because of the current limitation of the system (see the generateMergeAggregationExpressions
+ method of the SetAlgebricksPhysicalOperatorsRule class for more details.), hash hint will be
+ ignored.
+ * Success : Yes
+ */
+
+let $customer := {{ {"cid" : 1}, {"cid" : 2} }}
+
+let $orders := {{
+ {"oid": 100,
+ "ocid" : 1,
+ "priority" : 10,
+ "class" : "A",
+ "items" : [{"price" : 1000}, { "price" : 2000}]
+ },
+ {"oid": 200,
+ "ocid" : 2,
+ "priority" : 20,
+ "class" : "A",
+ "items" : [{"price" : 2000}, {"price" : 3000}]
+ }
+}}
+
+for $c in $customer
+for $o in $orders
+where
+ $c.cid = $o.ocid
+for $i in $o.items
+/*+ hash */
+group by $o_orderid := $o.oid, $o_class := $o.class, $o_priority := $o.priority
+ with $i
+let $price := sum (
+ for $t in $i
+ return
+ $t.price
+)
+order by $price desc, $o_class
+return {
+ "o_orderkey": $o_orderid,
+ "price": $price,
+ "o_class": $o_class,
+ "o_priority": $o_priority
+}
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/f40081f5/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.adm
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.adm
new file mode 100644
index 0000000..46c80f8
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/aggregate/query-ASTERIXDB-1727/query-ASTERIXDB-1727.1.adm
@@ -0,0 +1,2 @@
+{ "o_orderkey": 200, "price": 5000, "o_class": "A", "o_priority": 20 }
+{ "o_orderkey": 100, "price": 3000, "o_class": "A", "o_priority": 10 }
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/f40081f5/asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml
index dd859c4..5664c72 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml
@@ -613,6 +613,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="aggregate">
+ <compilation-unit name="query-ASTERIXDB-1727">
+ <output-dir compare="Text">query-ASTERIXDB-1727</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="aggregate">
<compilation-unit name="group_only">
<output-dir compare="Text">group_only</output-dir>
</compilation-unit>
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/f40081f5/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java
----------------------------------------------------------------------
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java
index 0c09fc0..5240781 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java
@@ -438,6 +438,16 @@ public class SetAlgebricksPhysicalOperatorsRule implements IAlgebraicRewriteRule
if (r0Logical.getOperatorTag() != LogicalOperatorTag.AGGREGATE) {
return false;
}
+
+ // Check whether there are multiple aggregates in the sub plan.
+ ILogicalOperator r1Logical = r0Logical;
+ while (r1Logical.hasInputs()) {
+ r1Logical = r1Logical.getInputs().get(0).getValue();
+ if (r1Logical.getOperatorTag() == LogicalOperatorTag.AGGREGATE) {
+ return false;
+ }
+ }
+
AggregateOperator aggOp = (AggregateOperator) r0.getValue();
List<Mutable<ILogicalExpression>> aggFuncRefs = aggOp.getExpressions();
List<LogicalVariable> originalAggVars = aggOp.getVariables();