Yingyi Bu has submitted this change and it was merged. Change subject: ASTERIXDB-1322: fix variable visitors for SubplanOperator. ......................................................................
ASTERIXDB-1322: fix variable visitors for SubplanOperator. - Simplified the implementation of OperatorPropertiesUtil; - Fixed PushProjectDownRule to not introduce project operator into a subplan to project outer variables. Change-Id: I53ee1e27b41c9c80d51a7e1baf058d97338c18a9 Reviewed-on: https://asterix-gerrit.ics.uci.edu/662 Tested-by: Jenkins <[email protected]> Reviewed-by: Till Westmann <[email protected]> --- M algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/ProducedVariableVisitor.java M algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/SchemaVariableVisitor.java M algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java M algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ComplexUnnestToProductRule.java M algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java 5 files changed, 28 insertions(+), 60 deletions(-) Approvals: Till Westmann: Looks good to me, approved Jenkins: Verified diff --git a/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/ProducedVariableVisitor.java b/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/ProducedVariableVisitor.java index 0c1e3de..8a0f156 100644 --- a/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/ProducedVariableVisitor.java +++ b/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/ProducedVariableVisitor.java @@ -20,7 +20,9 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.commons.lang3.mutable.Mutable; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; @@ -187,11 +189,16 @@ @Override public Void visitSubplanOperator(SubplanOperator op, Void arg) throws AlgebricksException { + Set<LogicalVariable> producedVars = new HashSet<>(); + Set<LogicalVariable> liveVars = new HashSet<>(); for (ILogicalPlan p : op.getNestedPlans()) { for (Mutable<ILogicalOperator> r : p.getRoots()) { - VariableUtilities.getLiveVariables(r.getValue(), producedVariables); + VariableUtilities.getProducedVariablesInDescendantsAndSelf(r.getValue(), producedVars); + VariableUtilities.getSubplanLocalLiveVariables(r.getValue(), liveVars); } } + producedVars.retainAll(liveVars); + producedVariables.addAll(producedVars); return null; } @@ -283,8 +290,9 @@ producedVariables.addAll(op.getVariables()); LogicalVariable positionalVariable = op.getPositionalVariable(); if (positionalVariable != null) { - if (!producedVariables.contains(positionalVariable)) + if (!producedVariables.contains(positionalVariable)) { producedVariables.add(positionalVariable); + } } return null; } diff --git a/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/SchemaVariableVisitor.java b/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/SchemaVariableVisitor.java index c2a20df..7a2f229 100644 --- a/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/SchemaVariableVisitor.java +++ b/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/SchemaVariableVisitor.java @@ -42,8 +42,8 @@ import org.apache.hyracks.algebricks.core.algebra.operators.logical.GroupByOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.IndexInsertDeleteUpsertOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.InnerJoinOperator; -import org.apache.hyracks.algebricks.core.algebra.operators.logical.IntersectOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.InsertDeleteUpsertOperator; +import org.apache.hyracks.algebricks.core.algebra.operators.logical.IntersectOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.LeftOuterJoinOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.LimitOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.MaterializeOperator; @@ -213,7 +213,6 @@ for (Mutable<ILogicalOperator> c : op.getInputs()) { VariableUtilities.getLiveVariables(c.getValue(), schemaVariables); } - VariableUtilities.getProducedVariables(op, schemaVariables); for (ILogicalPlan p : op.getNestedPlans()) { for (Mutable<ILogicalOperator> r : p.getRoots()) { VariableUtilities.getLiveVariables(r.getValue(), schemaVariables); @@ -294,7 +293,8 @@ } @Override - public Void visitIndexInsertDeleteUpsertOperator(IndexInsertDeleteUpsertOperator op, Void arg) throws AlgebricksException { + public Void visitIndexInsertDeleteUpsertOperator(IndexInsertDeleteUpsertOperator op, Void arg) + throws AlgebricksException { standardLayout(op); return null; } diff --git a/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java b/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java index 8e69cec..dfc12bf 100644 --- a/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java +++ b/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java @@ -82,22 +82,11 @@ HashSet<LogicalVariable> used = new HashSet<LogicalVariable>(); VariableUtilities.getUsedVariables(op, used); for (LogicalVariable v : used) { - if (!freeVars.contains(v)) { - freeVars.add(v); - } + freeVars.add(v); } - if (op.hasNestedPlans()) { AbstractOperatorWithNestedPlans s = (AbstractOperatorWithNestedPlans) op; - for (ILogicalPlan p : s.getNestedPlans()) { - for (Mutable<ILogicalOperator> r : p.getRoots()) { - getFreeVariablesInSelfOrDesc((AbstractLogicalOperator) r.getValue(), freeVars); - } - } - s.getUsedVariablesExceptNestedPlans(freeVars); - HashSet<LogicalVariable> produced2 = new HashSet<LogicalVariable>(); - s.getProducedVariablesExceptNestedPlans(produced2); - freeVars.removeAll(produced); + getFreeVariablesInSubplans(s, freeVars); } for (Mutable<ILogicalOperator> i : op.getInputs()) { getFreeVariablesInSelfOrDesc((AbstractLogicalOperator) i.getValue(), freeVars); @@ -140,53 +129,23 @@ if (op == dest) { return true; } - boolean onPath = false; if (((AbstractLogicalOperator) op).hasNestedPlans()) { AbstractOperatorWithNestedPlans a = (AbstractOperatorWithNestedPlans) op; for (ILogicalPlan p : a.getNestedPlans()) { for (Mutable<ILogicalOperator> r : p.getRoots()) { - if (isDestInNestedPath((AbstractLogicalOperator) r.getValue(), dest)) { - onPath = true; + if (collectUsedAndProducedVariablesInPath(r.getValue(), dest, usedVars, producedVars)) { + VariableUtilities.getUsedVariables(r.getValue(), usedVars); + VariableUtilities.getProducedVariables(r.getValue(), producedVars); + return true; } } } } for (Mutable<ILogicalOperator> childRef : op.getInputs()) { if (collectUsedAndProducedVariablesInPath(childRef.getValue(), dest, usedVars, producedVars)) { - onPath = true; - } - } - if (onPath) { - VariableUtilities.getUsedVariables(op, usedVars); - VariableUtilities.getProducedVariables(op, producedVars); - } - return onPath; - } - - /*** - * Recursively checks if the dest operator is in the path of a nested plan - * - * @param op - * @param dest - * @return - */ - private static boolean isDestInNestedPath(AbstractLogicalOperator op, ILogicalOperator dest) { - if (op == dest) { - return true; - } - for (Mutable<ILogicalOperator> i : op.getInputs()) { - if (isDestInNestedPath((AbstractLogicalOperator) i.getValue(), dest)) { + VariableUtilities.getUsedVariables(op, usedVars); + VariableUtilities.getProducedVariables(op, producedVars); return true; - } - } - if (op.hasNestedPlans()) { - AbstractOperatorWithNestedPlans a = (AbstractOperatorWithNestedPlans) op; - for (ILogicalPlan p : a.getNestedPlans()) { - for (Mutable<ILogicalOperator> r : p.getRoots()) { - if (isDestInNestedPath((AbstractLogicalOperator) r.getValue(), dest)) { - return true; - } - } } } return false; diff --git a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ComplexUnnestToProductRule.java b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ComplexUnnestToProductRule.java index 42ae597..270d8e4 100644 --- a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ComplexUnnestToProductRule.java +++ b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ComplexUnnestToProductRule.java @@ -24,7 +24,6 @@ import org.apache.commons.lang3.mutable.Mutable; import org.apache.commons.lang3.mutable.MutableObject; - import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression; import org.apache.hyracks.algebricks.core.algebra.base.ILogicalOperator; @@ -264,15 +263,17 @@ break; } } - // TODO: For now we bail here, but we could remember such ops and determine their target partition at a later point. - if (targetUsedVars == null) { - return false; - } } else if (innerMatches != 0 && outerMatches != 0) { // The current operator produces variables that are used by both partitions, so the inner and outer are not independent and, therefore, we cannot create a join. // TODO: We may still be able to split the operator to create a viable partitioning. return false; } + + // TODO: For now we bail here, but we could remember such ops and determine their target partition at a later point. + if (targetUsedVars == null) { + return false; + } + // Update used variables of partition that op belongs to. if (op.hasNestedPlans() && op.getOperatorTag() != LogicalOperatorTag.SUBPLAN) { AbstractOperatorWithNestedPlans opWithNestedPlans = (AbstractOperatorWithNestedPlans) op; diff --git a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java index 1cefead..2d57e8d 100644 --- a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java +++ b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushProjectDownRule.java @@ -167,7 +167,7 @@ IOptimizationContext context, ILogicalOperator initialOp) throws AlgebricksException { HashSet<LogicalVariable> allP = new HashSet<LogicalVariable>(); AbstractLogicalOperator op = (AbstractLogicalOperator) opRef.getValue(); - VariableUtilities.getLiveVariables(op, allP); + VariableUtilities.getSubplanLocalLiveVariables(op, allP); HashSet<LogicalVariable> toProject = new HashSet<LogicalVariable>(); for (LogicalVariable v : toPush) { -- To view, visit https://asterix-gerrit.ics.uci.edu/662 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I53ee1e27b41c9c80d51a7e1baf058d97338c18a9 Gerrit-PatchSet: 4 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]>
