asterixdb-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wangs...@apache.org
Subject asterixdb git commit: ASTERIXDB-1727: Fix an issue with multiple aggregates in one group-by
Date Fri, 11 Nov 2016 01:39:23 GMT
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 <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Yingyi Bu <buyingyi@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 <wangsaeu@yahoo.com>
Authored: Thu Nov 10 14:39:37 2016 -0800
Committer: Taewoo Kim <wangsaeu@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();


Mime
View raw message