You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by mb...@apache.org on 2022/02/02 18:55:35 UTC
[asterixdb] 06/12: [ASTERIXDB-3007][COMP] Fix ConsolidateWindowOperatorsRule
This is an automated email from the ASF dual-hosted git repository.
mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 11a30f10ab84c523cbea10b879bd1c2a245dda00
Author: Dmitry Lychagin <dm...@couchbase.com>
AuthorDate: Mon Jan 31 14:42:57 2022 -0800
[ASTERIXDB-3007][COMP] Fix ConsolidateWindowOperatorsRule
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Fix ConsolidateWindowOperatorsRule to correclty merge window
operator with subplans into window operator without subplans
- Fix deep copy visitors for window operator with subplans
- Add compiler sanity check code to verify that each nested tuple
source operator correctly points to its datasource operator
Change-Id: Ib9077a0331ab57cdd449426be77f05741d0778cc
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15063
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
Reviewed-by: Ali Alsuliman <al...@gmail.com>
---
.../queries/window/win_opt_02/win_opt_02_1.sqlpp | 35 ++++++++++++++++++++
.../results/window/win_opt_02/win_opt_02_1.plan | 23 ++++++++++++++
.../window/win_opt_02/win_opt_02.10.query.sqlpp | 37 ++++++++++++++++++++++
.../results/window/win_opt_02/win_opt_02.10.adm | 10 ++++++
...calOperatorDeepCopyWithNewVariablesVisitor.java | 4 +--
.../logical/visitors/OperatorDeepCopyVisitor.java | 4 +--
.../core/algebra/plan/PlanStructureVerifier.java | 35 +++++++++++++++++++-
.../rules/ConsolidateWindowOperatorsRule.java | 20 ++++++++++--
8 files changed, 161 insertions(+), 7 deletions(-)
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/window/win_opt_02/win_opt_02_1.sqlpp b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/window/win_opt_02/win_opt_02_1.sqlpp
new file mode 100644
index 0000000..def3a02
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/window/win_opt_02/win_opt_02_1.sqlpp
@@ -0,0 +1,35 @@
+/*
+ * 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 : Test fix for ASTERIXDB-3007
+ * Expected Res : SUCCESS
+ */
+
+with ds1 as (
+ select r as t, r*r as x
+ from range(1, 10) r
+)
+
+select t, x, dt, dx, int(v) as v, int(a) as a
+from ds1
+let dt = t - lag(t) over (order by t),
+ dx = x - lag(x) over (order by t),
+ v = dx/dt,
+ a = v - lag(v) over (order by t)
+order by t;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/window/win_opt_02/win_opt_02_1.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/window/win_opt_02/win_opt_02_1.plan
new file mode 100644
index 0000000..931e417
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/window/win_opt_02/win_opt_02_1.plan
@@ -0,0 +1,23 @@
+-- DISTRIBUTE_RESULT |LOCAL|
+ -- ONE_TO_ONE_EXCHANGE |LOCAL|
+ -- STREAM_PROJECT |LOCAL|
+ -- ASSIGN |LOCAL|
+ -- STREAM_PROJECT |LOCAL|
+ -- ASSIGN |LOCAL|
+ -- STREAM_PROJECT |LOCAL|
+ -- WINDOW |LOCAL|
+ {
+ -- AGGREGATE |LOCAL|
+ -- NESTED_TUPLE_SOURCE |LOCAL|
+ }
+ -- WINDOW |LOCAL|
+ {
+ -- AGGREGATE |LOCAL|
+ -- NESTED_TUPLE_SOURCE |LOCAL|
+ }
+ -- WINDOW_STREAM |LOCAL|
+ -- ONE_TO_ONE_EXCHANGE |LOCAL|
+ -- STABLE_SORT [$$r(ASC)] |LOCAL|
+ -- ONE_TO_ONE_EXCHANGE |UNPARTITIONED|
+ -- UNNEST |UNPARTITIONED|
+ -- EMPTY_TUPLE_SOURCE |UNPARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/window/win_opt_02/win_opt_02.10.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/window/win_opt_02/win_opt_02.10.query.sqlpp
new file mode 100644
index 0000000..a6f448c
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/window/win_opt_02/win_opt_02.10.query.sqlpp
@@ -0,0 +1,37 @@
+/*
+ * 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 : Test fix for ASTERIXDB-3007
+ * Expected Res : SUCCESS
+ */
+
+use test;
+
+with ds1 as (
+ select r as t, r*r as x
+ from range(1, 10) r
+)
+
+select t, x, dt, dx, int(v) as v, int(a) as a
+from ds1
+let dt = t - lag(t) over (order by t),
+ dx = x - lag(x) over (order by t),
+ v = dx/dt,
+ a = v - lag(v) over (order by t)
+order by t;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/window/win_opt_02/win_opt_02.10.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/window/win_opt_02/win_opt_02.10.adm
new file mode 100644
index 0000000..29322df
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/window/win_opt_02/win_opt_02.10.adm
@@ -0,0 +1,10 @@
+{ "t": 1, "x": 1, "dt": null, "dx": null, "v": null, "a": null }
+{ "t": 2, "x": 4, "dt": 1, "dx": 3, "v": 3, "a": null }
+{ "t": 3, "x": 9, "dt": 1, "dx": 5, "v": 5, "a": 2 }
+{ "t": 4, "x": 16, "dt": 1, "dx": 7, "v": 7, "a": 2 }
+{ "t": 5, "x": 25, "dt": 1, "dx": 9, "v": 9, "a": 2 }
+{ "t": 6, "x": 36, "dt": 1, "dx": 11, "v": 11, "a": 2 }
+{ "t": 7, "x": 49, "dt": 1, "dx": 13, "v": 13, "a": 2 }
+{ "t": 8, "x": 64, "dt": 1, "dx": 15, "v": 15, "a": 2 }
+{ "t": 9, "x": 81, "dt": 1, "dx": 17, "v": 17, "a": 2 }
+{ "t": 10, "x": 100, "dt": 1, "dx": 19, "v": 19, "a": 2 }
\ No newline at end of file
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/LogicalOperatorDeepCopyWithNewVariablesVisitor.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/LogicalOperatorDeepCopyWithNewVariablesVisitor.java
index 199c2e1..e242531 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/LogicalOperatorDeepCopyWithNewVariablesVisitor.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/LogicalOperatorDeepCopyWithNewVariablesVisitor.java
@@ -619,11 +619,11 @@ public class LogicalOperatorDeepCopyWithNewVariablesVisitor
List<LogicalVariable> varCopy = deepCopyVariableList(op.getVariables());
List<Mutable<ILogicalExpression>> exprCopy =
exprDeepCopyVisitor.deepCopyExpressionReferenceList(op.getExpressions());
- List<ILogicalPlan> nestedPlansCopy = new ArrayList<>();
WindowOperator opCopy = new WindowOperator(partitionExprCopy, orderExprCopy, frameValueExprCopy,
frameStartExprCopy, frameStartValidationExprCopy, frameEndExprCopy, frameEndValidationExprCopy,
frameExcludeExprCopy, op.getFrameExcludeNegationStartIdx(), frameExcludeUnaryExprCopy,
- frameOffsetExprCopy, op.getFrameMaxObjects(), varCopy, exprCopy, nestedPlansCopy);
+ frameOffsetExprCopy, op.getFrameMaxObjects(), varCopy, exprCopy, null);
+ List<ILogicalPlan> nestedPlansCopy = opCopy.getNestedPlans();
deepCopyInputsAnnotationsAndExecutionMode(op, arg, opCopy);
deepCopyPlanList(op.getNestedPlans(), nestedPlansCopy, opCopy);
return opCopy;
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/OperatorDeepCopyVisitor.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/OperatorDeepCopyVisitor.java
index 7b67af1..c2ee661 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/OperatorDeepCopyVisitor.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/OperatorDeepCopyVisitor.java
@@ -448,11 +448,11 @@ public class OperatorDeepCopyVisitor implements ILogicalOperatorVisitor<ILogical
deepCopyVars(newVariables, op.getVariables());
List<Mutable<ILogicalExpression>> newExpressions = new ArrayList<>();
deepCopyExpressionRefs(newExpressions, op.getExpressions());
- List<ILogicalPlan> newNestedPlans = new ArrayList<>();
WindowOperator newWinOp = new WindowOperator(newPartitionExprs, newOrderExprs, newFrameValueExprs,
newFrameStartExprs, newFrameStartValidationExprs, newFrameEndExprs, newFrameEndValidationExprs,
newFrameExclusionExprs, op.getFrameExcludeNegationStartIdx(), newFrameExcludeUnaryExpr,
- newFrameOffsetExpr, op.getFrameMaxObjects(), newVariables, newExpressions, newNestedPlans);
+ newFrameOffsetExpr, op.getFrameMaxObjects(), newVariables, newExpressions, null);
+ List<ILogicalPlan> newNestedPlans = newWinOp.getNestedPlans();
for (ILogicalPlan nestedPlan : op.getNestedPlans()) {
newNestedPlans.add(OperatorManipulationUtil.deepCopy(nestedPlan, newWinOp));
}
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/plan/PlanStructureVerifier.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/plan/PlanStructureVerifier.java
index a072d11..2b40114 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/plan/PlanStructureVerifier.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/plan/PlanStructureVerifier.java
@@ -46,9 +46,11 @@ 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.IVariableTypeEnvironment;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractOperatorWithNestedPlans;
+import org.apache.hyracks.algebricks.core.algebra.operators.logical.NestedTupleSourceOperator;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.VariableUtilities;
import org.apache.hyracks.algebricks.core.algebra.prettyprint.IPlanPrettyPrinter;
import org.apache.hyracks.algebricks.core.algebra.typing.ITypingContext;
+import org.apache.hyracks.algebricks.core.algebra.util.OperatorManipulationUtil;
import org.apache.hyracks.algebricks.core.algebra.util.OperatorPropertiesUtil;
import org.apache.hyracks.algebricks.core.algebra.visitors.ILogicalExpressionReferenceTransform;
@@ -76,6 +78,11 @@ public final class PlanStructureVerifier {
private static final String ERROR_MESSAGE_TEMPLATE_6 = "undefined used variables %s in %s";
+ private static final String ERROR_MESSAGE_TEMPLATE_7 =
+ "unexpected source operator in NestedTupleSourceOperator: %s. Expected source operator %s";
+
+ private static final String ERROR_MESSAGE_TEMPLATE_8 = "unexpected leaf operator in nested plan: %s";
+
public static final Comparator<LogicalVariable> VARIABLE_CMP = Comparator.comparing(LogicalVariable::toString);
private final ExpressionReferenceVerifierVisitor exprVisitor = new ExpressionReferenceVerifierVisitor();
@@ -185,7 +192,10 @@ public final class PlanStructureVerifier {
if (op instanceof AbstractOperatorWithNestedPlans) {
children = new ArrayList<>(children);
for (ILogicalPlan nestedPlan : ((AbstractOperatorWithNestedPlans) op).getNestedPlans()) {
- children.addAll(nestedPlan.getRoots());
+ for (Mutable<ILogicalOperator> nestedRootRef : nestedPlan.getRoots()) {
+ checkLeafOperatorsInNestedPlan(op, nestedRootRef);
+ children.add(nestedRootRef);
+ }
}
}
return children;
@@ -262,6 +272,29 @@ public final class PlanStructureVerifier {
}
}
+ private void checkLeafOperatorsInNestedPlan(ILogicalOperator op, Mutable<ILogicalOperator> rootRef)
+ throws AlgebricksException {
+ for (Mutable<ILogicalOperator> leafRef : OperatorManipulationUtil.findLeafDescendantsOrSelf(rootRef)) {
+ ILogicalOperator leafOp = leafRef.getValue();
+ switch (leafOp.getOperatorTag()) {
+ case EMPTYTUPLESOURCE:
+ break;
+ case NESTEDTUPLESOURCE:
+ NestedTupleSourceOperator ntsOp = (NestedTupleSourceOperator) leafOp;
+ ILogicalOperator ntsSrcOp = ntsOp.getDataSourceReference().getValue();
+ if (ntsSrcOp != op) {
+ throw new AlgebricksException(String.format(ERROR_MESSAGE_TEMPLATE_7,
+ PlanStabilityVerifier.printOperator(ntsSrcOp, prettyPrinter),
+ PlanStabilityVerifier.printOperator(op, prettyPrinter)));
+ }
+ break;
+ default:
+ throw new AlgebricksException(String.format(ERROR_MESSAGE_TEMPLATE_8,
+ PlanStabilityVerifier.printOperator(leafOp, prettyPrinter)));
+ }
+ }
+ }
+
private void raiseException(String sharedReferenceKind, String sharedEntity, ILogicalOperator firstOp,
ILogicalOperator secondOp) throws AlgebricksException {
String errorMessage;
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateWindowOperatorsRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateWindowOperatorsRule.java
index 2ec0654..317dabb 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateWindowOperatorsRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateWindowOperatorsRule.java
@@ -33,10 +33,12 @@ 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.operators.logical.AbstractLogicalOperator;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.AggregateOperator;
+import org.apache.hyracks.algebricks.core.algebra.operators.logical.NestedTupleSourceOperator;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.WindowOperator;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.IsomorphismOperatorVisitor;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.IsomorphismUtilities;
import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.VariableUtilities;
+import org.apache.hyracks.algebricks.core.algebra.util.OperatorManipulationUtil;
import org.apache.hyracks.algebricks.core.algebra.util.OperatorPropertiesUtil;
import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
@@ -82,7 +84,9 @@ public class ConsolidateWindowOperatorsRule implements IAlgebraicRewriteRule {
Set<LogicalVariable> used1 = new HashSet<>();
VariableUtilities.getUsedVariables(winOp1, used1);
- if (!OperatorPropertiesUtil.disjoint(winOp2.getVariables(), used1)) {
+ Set<LogicalVariable> produced2 = new HashSet<>();
+ VariableUtilities.getProducedVariables(winOp2, produced2);
+ if (!OperatorPropertiesUtil.disjoint(produced2, used1)) {
return false;
}
@@ -130,7 +134,6 @@ public class ConsolidateWindowOperatorsRule implements IAlgebraicRewriteRule {
aggTo.getExpressions().addAll(aggFrom.getExpressions());
context.computeAndSetTypeEnvironmentForOperator(aggTo);
} else {
- setAll(winOpTo.getNestedPlans(), winOpFrom.getNestedPlans());
setAll(winOpTo.getFrameValueExpressions(), winOpFrom.getFrameValueExpressions());
setAll(winOpTo.getFrameStartExpressions(), winOpFrom.getFrameStartExpressions());
setAll(winOpTo.getFrameStartValidationExpressions(), winOpFrom.getFrameStartValidationExpressions());
@@ -141,6 +144,19 @@ public class ConsolidateWindowOperatorsRule implements IAlgebraicRewriteRule {
winOpTo.getFrameExcludeUnaryExpression().setValue(winOpFrom.getFrameExcludeUnaryExpression().getValue());
winOpTo.getFrameOffsetExpression().setValue(winOpFrom.getFrameOffsetExpression().getValue());
winOpTo.setFrameMaxObjects(winOpFrom.getFrameMaxObjects());
+ // move nested plans
+ for (ILogicalPlan fromNestedPlan : winOpFrom.getNestedPlans()) {
+ for (Mutable<ILogicalOperator> rootRef : fromNestedPlan.getRoots()) {
+ for (Mutable<ILogicalOperator> leafRef : OperatorManipulationUtil
+ .findLeafDescendantsOrSelf(rootRef)) {
+ ILogicalOperator leafOp = leafRef.getValue();
+ if (leafOp.getOperatorTag() == LogicalOperatorTag.NESTEDTUPLESOURCE) {
+ ((NestedTupleSourceOperator) leafOp).getDataSourceReference().setValue(winOpTo);
+ }
+ }
+ }
+ winOpTo.getNestedPlans().add(fromNestedPlan);
+ }
}
return true;
}