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 4dde600f6c30259add99cbd47759d08a255aa548 Author: Peeyush Gupta <[email protected]> AuthorDate: Tue Nov 19 13:33:06 2024 -0800 [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]> --- .../rules/PushAggregateIntoNestedSubplanRule.java | 6 ++++-- .../optimizer/rules/PushGroupByThroughProduct.java | 4 +++- .../core/algebra/util/OperatorManipulationUtil.java | 19 +++++++++++++++---- .../rewriter/rules/AbstractDecorrelationRule.java | 4 +++- .../rules/subplan/IntroduceGroupByForSubplanRule.java | 7 +++++-- 5 files changed, 30 insertions(+), 10 deletions(-) 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 d2ac8e57ec..378c75851d 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 @@ public class PushAggregateIntoNestedSubplanRule implements IAlgebraicRewriteRule 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 6d92b515e4..28166a98d5 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 @@ public class PushGroupByThroughProduct implements IAlgebraicRewriteRule { 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 2413ed250e..f8fb6c252e 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 class OperatorManipulationUtil { } 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 @@ public class OperatorManipulationUtil { 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 55c8400e3d..dbdf6c48a3 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 @@ public abstract class AbstractDecorrelationRule implements IAlgebraicRewriteRule 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 @@ public abstract class AbstractDecorrelationRule implements IAlgebraicRewriteRule 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 391d03a8d8..36cad77208 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 @@ public class IntroduceGroupByForSubplanRule implements IAlgebraicRewriteRule { 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 @@ public class IntroduceGroupByForSubplanRule implements IAlgebraicRewriteRule { 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;
