Yingyi Bu has submitted this change and it was merged. Change subject: ASTERIXDB-865: fix for if-else expression. ......................................................................
ASTERIXDB-865: fix for if-else expression. Change-Id: I17978d2f694e2a5082903002b8388c5bd42811a5 Reviewed-on: https://asterix-gerrit.ics.uci.edu/702 Tested-by: Jenkins <[email protected]> Reviewed-by: Till Westmann <[email protected]> --- 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/ConsolidateSelectsRule.java M algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineVariablesRule.java M algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectDownRule.java M algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SimpleUnnestToProductRule.java 5 files changed, 41 insertions(+), 26 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/util/OperatorPropertiesUtil.java b/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorPropertiesUtil.java index 0a434ad..e78db9b 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 @@ -37,13 +37,13 @@ import org.apache.hyracks.algebricks.core.algebra.functions.AlgebricksBuiltinFunctions; import org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractLogicalOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractOperatorWithNestedPlans; -import org.apache.hyracks.algebricks.core.algebra.operators.logical.AssignOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.SelectOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.CardinalityInferenceVisitor; -import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.IsExpressionStatefulVisitor; import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.VariableUtilities; public class OperatorPropertiesUtil { + + private static final String MOVABLE = "isMovable"; public static <T> boolean disjoint(Collection<T> c1, Collection<T> c2) { for (T m : c1) { @@ -269,25 +269,32 @@ } /** - * Whether the operator is an assign operator that calls a stateful function. + * Whether an operator can be moved around in the query plan. * * @param op * the operator to consider. - * @return true if the operator is an assign operator and it calls a stateful function. + * @return true if the operator can be moved, false if the operator cannot be moved. * @throws AlgebricksException */ - public static boolean isStatefulAssign(ILogicalOperator op) throws AlgebricksException { - if (op.getOperatorTag() != LogicalOperatorTag.ASSIGN) { - return false; + public static boolean isMovable(ILogicalOperator op) { + Object annotation = op.getAnnotations().get(MOVABLE); + if (annotation == null) { + // By default, it is movable. + return true; } - AssignOperator assignOp = (AssignOperator) op; - IsExpressionStatefulVisitor visitor = new IsExpressionStatefulVisitor(); - for (Mutable<ILogicalExpression> exprRef : assignOp.getExpressions()) { - ILogicalExpression expr = exprRef.getValue(); - if (expr.accept(visitor, null)) { - return true; - } - } - return false; + Boolean movable = (Boolean) annotation; + return movable; + } + + /** + * Mark an operator to be either movable or not. + * + * @param op + * the operator to consider. + * @param movable + * true means it is movable, false means it is not movable. + */ + public static void markMovable(ILogicalOperator op, boolean movable) { + op.getAnnotations().put(MOVABLE, movable); } } diff --git a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateSelectsRule.java b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateSelectsRule.java index 2aee1a7..ae52c35 100644 --- a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateSelectsRule.java +++ b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ConsolidateSelectsRule.java @@ -67,8 +67,8 @@ do { selectParent = nextSelect; nextSelect = (AbstractLogicalOperator) selectParent.getInputs().get(0).getValue(); - } while (nextSelect.getOperatorTag() == LogicalOperatorTag.ASSIGN && !OperatorPropertiesUtil - .isStatefulAssign(nextSelect) /* Select cannot be pushed through stateful assigns.*/); + } while (nextSelect.getOperatorTag() == LogicalOperatorTag.ASSIGN && OperatorPropertiesUtil + .isMovable(nextSelect) /* Select cannot be pushed through un-movable operators.*/); // Stop if the child op is not a select. if (nextSelect.getOperatorTag() != LogicalOperatorTag.SELECT) { break; diff --git a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineVariablesRule.java b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineVariablesRule.java index f2645f5..e7cf912 100644 --- a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineVariablesRule.java +++ b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/InlineVariablesRule.java @@ -29,6 +29,7 @@ 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; +import org.apache.hyracks.algebricks.core.algebra.base.ILogicalPlan; import org.apache.hyracks.algebricks.core.algebra.base.IOptimizationContext; import org.apache.hyracks.algebricks.core.algebra.base.LogicalExpressionTag; import org.apache.hyracks.algebricks.core.algebra.base.LogicalOperatorTag; @@ -41,7 +42,6 @@ import org.apache.hyracks.algebricks.core.algebra.operators.logical.AssignOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.SubplanOperator; import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.VariableUtilities; -import org.apache.hyracks.algebricks.core.algebra.plan.ALogicalPlanImpl; import org.apache.hyracks.algebricks.core.algebra.visitors.ILogicalExpressionReferenceTransform; import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule; @@ -168,10 +168,18 @@ // Descend into subplan if (op.getOperatorTag() == LogicalOperatorTag.SUBPLAN) { - ALogicalPlanImpl subPlan = (ALogicalPlanImpl) ((SubplanOperator) op).getNestedPlans().get(0); - Mutable<ILogicalOperator> subPlanRootOpRef = subPlan.getRoots().get(0); - if (inlineVariables(subPlanRootOpRef, context)) { - modified = true; + SubplanOperator subplanOp = (SubplanOperator) op; + for (ILogicalPlan nestedPlan : subplanOp.getNestedPlans()) { + for (Mutable<ILogicalOperator> root : nestedPlan.getRoots()) { + if (inlineVariables(root, context)) { + modified = true; + } + // Variables produced by a nested subplan cannot be inlined + // in operators above the subplan. + Set<LogicalVariable> producedVars = new HashSet<LogicalVariable>(); + VariableUtilities.getProducedVariables(root.getValue(), producedVars); + varAssignRhs.keySet().removeAll(producedVars); + } } } diff --git a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectDownRule.java b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectDownRule.java index 3d3331c..aab6d12 100644 --- a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectDownRule.java +++ b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectDownRule.java @@ -73,7 +73,7 @@ throws AlgebricksException { AbstractLogicalOperator op2 = (AbstractLogicalOperator) opRef2.getValue(); if (op2.getInputs().size() != 1 || op2.getOperatorTag() == LogicalOperatorTag.DATASOURCESCAN - || OperatorPropertiesUtil.isStatefulAssign(op2)) { + || !OperatorPropertiesUtil.isMovable(op2)) { return false; } diff --git a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SimpleUnnestToProductRule.java b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SimpleUnnestToProductRule.java index 1ba1ea3..06fb360 100644 --- a/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SimpleUnnestToProductRule.java +++ b/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SimpleUnnestToProductRule.java @@ -110,8 +110,8 @@ && boundaryOpRef.getValue().getOperatorTag() != LogicalOperatorTag.DATASOURCESCAN) { List<LogicalVariable> opUsedVars = new ArrayList<LogicalVariable>(); VariableUtilities.getUsedVariables(boundaryOpRef.getValue(), opUsedVars); - if (opUsedVars.size() == 0 && !OperatorPropertiesUtil.isStatefulAssign(boundaryOpRef.getValue()) - /* We cannot freely move the location of stateful assigns. */) { + if (opUsedVars.size() == 0 && OperatorPropertiesUtil.isMovable(boundaryOpRef.getValue()) + /* We cannot freely move the location of operators tagged as un-movable. */) { // move down the boundary if the operator is a const assigns. boundaryOpRef = boundaryOpRef.getValue().getInputs().get(0); } else { -- To view, visit https://asterix-gerrit.ics.uci.edu/702 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I17978d2f694e2a5082903002b8388c5bd42811a5 Gerrit-PatchSet: 9 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]>
