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 <[email protected]> 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 <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Dmitry Lychagin <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> --- .../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; }
