asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yingyi Bu (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in hyracks[master]: ASTERIXDB-1322: fix variable visitors for SubplanOperator.
Date Fri, 26 Feb 2016 18:16:16 GMT
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 <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Till Westmann <tillw@apache.org>
---
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 <buyingyi@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>

Mime
View raw message