This is an automated email from the ASF dual-hosted git repository.
dlych pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
The following commit(s) were added to refs/heads/master by this push:
new c5589f6 [ASTERIXDB-2612][COMP] Incorrect result with two-step
aggregate
c5589f6 is described below
commit c5589f6b2022d66a885df26cbac6932ff52ef6f5
Author: Dmitry Lychagin <[email protected]>
AuthorDate: Mon Jul 15 11:41:01 2019 -0700
[ASTERIXDB-2612][COMP] Incorrect result with two-step aggregate
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Restrict two-step aggregate rewriting based on
operators in the GROUP BY subplan
Change-Id: Iffae44842d4aad594c4252218837fb2d244ede36
Reviewed-on: https://asterix-gerrit.ics.uci.edu/3494
Contrib: Jenkins <[email protected]>
Sonar-Qube: Jenkins <[email protected]>
Integration-Tests: Jenkins <[email protected]>
Tested-by: Jenkins <[email protected]>
Reviewed-by: Dmitry Lychagin <[email protected]>
Reviewed-by: Ali Alsuliman <[email protected]>
---
.../two-step-agg-01/two-step-agg-01.1.ddl.sqlpp | 46 ++++++++++++++++++++++
.../two-step-agg-01/two-step-agg-01.2.update.sqlpp | 22 +++++++++++
.../two-step-agg-01/two-step-agg-01.3.query.sqlpp | 29 ++++++++++++++
.../group-by/two-step-agg-01/two-step-agg-01.3.adm | 2 +
.../test/resources/runtimets/testsuite_sqlpp.xml | 5 +++
.../rules/AbstractIntroduceCombinerRule.java | 18 ++++++---
6 files changed, 117 insertions(+), 5 deletions(-)
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.1.ddl.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.1.ddl.sqlpp
new file mode 100644
index 0000000..dd75e79
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.1.ddl.sqlpp
@@ -0,0 +1,46 @@
+/*
+ * 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.
+ */
+
+drop dataverse test if exists;
+create dataverse test;
+
+use test;
+
+
+create type test.Schema as
+ closed {
+ unique1 : bigint,
+ unique2 : bigint,
+ two : bigint,
+ four : bigint,
+ ten : bigint,
+ twenty : bigint,
+ onePercent : bigint,
+ tenPercent : bigint,
+ twentyPercent : bigint,
+ fiftyPercent : bigint,
+ unique3 : bigint,
+ evenOnePercent : bigint,
+ oddOnePercent : bigint,
+ stringu1 : string,
+ stringu2 : string,
+ string4 : string
+};
+
+create dataset onektup(Schema) primary key unique2;
\ No newline at end of file
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.2.update.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.2.update.sqlpp
new file mode 100644
index 0000000..e5d85fe
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.2.update.sqlpp
@@ -0,0 +1,22 @@
+/*
+ * 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;
+
+load dataset onektup using localfs
((`path`=`asterix_nc1://data/wisc/onektup.adm`),(`format`=`delimited-text`),(`delimiter`=`|`))
pre-sorted;
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.3.query.sqlpp
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.3.query.sqlpp
new file mode 100644
index 0000000..4841d45
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/two-step-agg-01/two-step-agg-01.3.query.sqlpp
@@ -0,0 +1,29 @@
+/*
+ * 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 m, array_count((from g select value g.t.unique1 limit 3)) cnt
+from onektup t
+group by unique2 % 2 as m group as g
+order by m
+;
+
+
+
diff --git
a/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/two-step-agg-01/two-step-agg-01.3.adm
b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/two-step-agg-01/two-step-agg-01.3.adm
new file mode 100644
index 0000000..a5f6e8d
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/resources/runtimets/results/group-by/two-step-agg-01/two-step-agg-01.3.adm
@@ -0,0 +1,2 @@
+{ "m": 0, "cnt": 3 }
+{ "m": 1, "cnt": 3 }
\ 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 4b6d797..5d461f8 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -5018,6 +5018,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="group-by">
+ <compilation-unit name="two-step-agg-01">
+ <output-dir compare="Text">two-step-agg-01</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="group-by">
<compilation-unit name="null">
<output-dir compare="Text">null</output-dir>
</compilation-unit>
diff --git
a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java
b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java
index cb290a4..8ffbdac 100644
---
a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java
+++
b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceCombinerRule.java
@@ -31,7 +31,6 @@ import org.apache.hyracks.algebricks.common.utils.Pair;
import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression;
import org.apache.hyracks.algebricks.core.algebra.base.ILogicalOperator;
import org.apache.hyracks.algebricks.core.algebra.base.IOptimizationContext;
-import org.apache.hyracks.algebricks.core.algebra.base.LogicalOperatorTag;
import org.apache.hyracks.algebricks.core.algebra.base.LogicalVariable;
import
org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression;
import
org.apache.hyracks.algebricks.core.algebra.expressions.AggregateFunctionCallExpression;
@@ -125,13 +124,13 @@ public abstract class AbstractIntroduceCombinerRule
implements IAlgebraicRewrite
if (newGbyOp != null) {
// Cut and paste nested input pipelines of initAgg to
pushedAgg's input
Mutable<ILogicalOperator> inputRef =
initAgg.getInputs().get(0);
- if (!isPushableInput(inputRef.getValue())) {
+ if (!isPushableInputInGroupBySubplan(inputRef.getValue())) {
return new Pair<>(false, null);
}
Mutable<ILogicalOperator> bottomRef = inputRef;
while (!bottomRef.getValue().getInputs().isEmpty()) {
bottomRef = bottomRef.getValue().getInputs().get(0);
- if (!isPushableInput(bottomRef.getValue())) {
+ if
(!isPushableInputInGroupBySubplan(bottomRef.getValue())) {
return new Pair<>(false, null);
}
}
@@ -160,8 +159,17 @@ public abstract class AbstractIntroduceCombinerRule
implements IAlgebraicRewrite
}
}
- protected boolean isPushableInput(ILogicalOperator op) {
- return op.getOperatorTag() != LogicalOperatorTag.DISTINCT;
+ protected boolean isPushableInputInGroupBySubplan(ILogicalOperator op) {
+ switch (op.getOperatorTag()) {
+ case ASSIGN:
+ case NESTEDTUPLESOURCE:
+ case ORDER:
+ case PROJECT:
+ case SELECT:
+ return true;
+ default:
+ return false;
+ }
}
protected class SimilarAggregatesInfo {
