>From Peeyush Gupta <[email protected]>: Peeyush Gupta has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19103 )
Change subject: [NO ISSUE][COMP] Exponential recursion in OperatorManipulationUtil.substituteVarRec method ...................................................................... [NO ISSUE][COMP] Exponential recursion in OperatorManipulationUtil.substituteVarRec method - user model changes: no - storage format changes: no - interface changes: no Ext-ref: MB-64295 Change-Id: Iefe7859bb6b83f59e735d041ef1765e8741d0c5e Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19095 Tested-by: Jenkins <[email protected]> Reviewed-by: Peeyush Gupta <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> (cherry picked from commit 4dde600f6c30259add99cbd47759d08a255aa548) --- M hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggregateIntoNestedSubplanRule.java M hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractDecorrelationRule.java M hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/IntroduceGroupByForSubplanRule.java M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java 5 files changed, 50 insertions(+), 10 deletions(-) git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/03/19103/1 diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggregateIntoNestedSubplanRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggregateIntoNestedSubplanRule.java index d2ac8e5..378c758 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggregateIntoNestedSubplanRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushAggregateIntoNestedSubplanRule.java @@ -398,14 +398,16 @@ if (!pushableNestedSubplan) { return false; } - + Set<ILogicalOperator> visited = new HashSet<>(); for (int i = 0; i < nspOp.getNestedPlans().size(); i++) { Mutable<ILogicalOperator> nspAggRef = nspOp.getNestedPlans().get(i).getRoots().get(0); AggregateOperator nspAgg = (AggregateOperator) nspAggRef.getValue(); Mutable<ILogicalOperator> nspAggChildRef = nspAgg.getInputs().get(0); LogicalVariable listifyVar = findListifiedVariable(nspAgg, varFromNestedAgg); if (listifyVar != null) { - OperatorManipulationUtil.substituteVarRec(aggInSubplanOp, unnestVar, listifyVar, true, context); + OperatorManipulationUtil.substituteVarRec(aggInSubplanOp, unnestVar, listifyVar, true, context, + visited); + visited.clear(); nspAgg.getVariables().addAll(aggInSubplanOp.getVariables()); nspAgg.getExpressions().addAll(aggInSubplanOp.getExpressions()); for (LogicalVariable v : aggInSubplanOp.getVariables()) { diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java index 6d92b51..28166a9 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushGroupByThroughProduct.java @@ -120,12 +120,14 @@ AbstractBinaryJoinOperator join = (AbstractBinaryJoinOperator) opRefJoin.getValue(); gby.getDecorList().clear(); gby.getDecorList().addAll(decorToPush); + Set<ILogicalOperator> visited = new HashSet<>(); for (Pair<LogicalVariable, Mutable<ILogicalExpression>> p : decorNotToPush) { LogicalVariable v1 = p.first; if (v1 != null) { VariableReferenceExpression varRef = (VariableReferenceExpression) p.second.getValue(); LogicalVariable v2 = varRef.getVariableReference(); - OperatorManipulationUtil.substituteVarRec(join, v2, v1, true, context); + OperatorManipulationUtil.substituteVarRec(join, v2, v1, true, context, visited); + visited.clear(); } } Mutable<ILogicalOperator> branchRef = join.getInputs().get(branch); diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java index 2413ed2..f8fb6c2 100644 --- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java +++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java @@ -181,17 +181,24 @@ } public static void substituteVarRec(AbstractLogicalOperator op, LogicalVariable v1, LogicalVariable v2, - boolean goThroughNts, ITypingContext ctx) throws AlgebricksException { + boolean goThroughNts, ITypingContext ctx, Set<ILogicalOperator> visited) throws AlgebricksException { VariableUtilities.substituteVariables(op, v1, v2, goThroughNts, ctx); for (Mutable<ILogicalOperator> opRef2 : op.getInputs()) { - substituteVarRec((AbstractLogicalOperator) opRef2.getValue(), v1, v2, goThroughNts, ctx); + if (visited.contains(opRef2.getValue())) { + continue; + } + visited.add(opRef2.getValue()); + substituteVarRec((AbstractLogicalOperator) opRef2.getValue(), v1, v2, goThroughNts, ctx, visited); } if (op.getOperatorTag() == LogicalOperatorTag.NESTEDTUPLESOURCE && goThroughNts) { NestedTupleSourceOperator nts = (NestedTupleSourceOperator) op; if (nts.getDataSourceReference() != null) { AbstractLogicalOperator op2 = (AbstractLogicalOperator) nts.getDataSourceReference().getValue().getInputs().get(0).getValue(); - substituteVarRec(op2, v1, v2, goThroughNts, ctx); + if (!visited.contains(op2)) { + visited.add(op2); + substituteVarRec(op2, v1, v2, goThroughNts, ctx, visited); + } } } if (op.hasNestedPlans()) { @@ -199,7 +206,11 @@ for (ILogicalPlan p : aonp.getNestedPlans()) { for (Mutable<ILogicalOperator> ref : p.getRoots()) { AbstractLogicalOperator aop = (AbstractLogicalOperator) ref.getValue(); - substituteVarRec(aop, v1, v2, goThroughNts, ctx); + if (visited.contains(aop)) { + continue; + } + visited.add(aop); + substituteVarRec(aop, v1, v2, goThroughNts, ctx, visited); } } } diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractDecorrelationRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractDecorrelationRule.java index 55c8400..dbdf6c4 100644 --- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractDecorrelationRule.java +++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractDecorrelationRule.java @@ -92,6 +92,7 @@ protected void buildVarExprList(Collection<LogicalVariable> vars, IOptimizationContext context, GroupByOperator g, List<Pair<LogicalVariable, Mutable<ILogicalExpression>>> outVeList) throws AlgebricksException { SourceLocation sourceLoc = g.getSourceLocation(); + Set<ILogicalOperator> visited = new HashSet<>(); for (LogicalVariable ov : vars) { LogicalVariable newVar = context.newVar(); VariableReferenceExpression varExpr = new VariableReferenceExpression(newVar); @@ -101,7 +102,8 @@ for (ILogicalPlan p : g.getNestedPlans()) { for (Mutable<ILogicalOperator> r : p.getRoots()) { OperatorManipulationUtil.substituteVarRec((AbstractLogicalOperator) r.getValue(), ov, newVar, true, - context); + context, visited); + visited.clear(); } } } diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/IntroduceGroupByForSubplanRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/IntroduceGroupByForSubplanRule.java index 391d03a..36cad77 100644 --- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/IntroduceGroupByForSubplanRule.java +++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/IntroduceGroupByForSubplanRule.java @@ -331,6 +331,7 @@ List<Pair<LogicalVariable, Mutable<ILogicalExpression>>> outVeList) throws AlgebricksException { SourceLocation sourceLoc = g.getSourceLocation(); Map<LogicalVariable, LogicalVariable> m = new HashMap<LogicalVariable, LogicalVariable>(); + Set<ILogicalOperator> visited = new HashSet<>(); for (LogicalVariable ov : vars) { LogicalVariable newVar = context.newVar(); ILogicalExpression varExpr = new VariableReferenceExpression(newVar); @@ -340,11 +341,13 @@ for (ILogicalPlan p : g.getNestedPlans()) { for (Mutable<ILogicalOperator> r : p.getRoots()) { OperatorManipulationUtil.substituteVarRec((AbstractLogicalOperator) r.getValue(), ov, newVar, true, - context); + context, visited); + visited.clear(); } } AbstractLogicalOperator opUnder = (AbstractLogicalOperator) g.getInputs().get(0).getValue(); - OperatorManipulationUtil.substituteVarRec(opUnder, ov, newVar, true, context); + OperatorManipulationUtil.substituteVarRec(opUnder, ov, newVar, true, context, visited); + visited.clear(); m.put(ov, newVar); } return m; -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19103 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: stabilization-c8b0f90c72 Gerrit-Change-Id: Iefe7859bb6b83f59e735d041ef1765e8741d0c5e Gerrit-Change-Number: 19103 Gerrit-PatchSet: 1 Gerrit-Owner: Peeyush Gupta <[email protected]> Gerrit-MessageType: newchange
