You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by al...@apache.org on 2019/08/21 04:47:05 UTC

[asterixdb] branch master updated: [NO ISSUE][COMP] Fix InlineUnnestFunction to return false if not fired

This is an automated email from the ASF dual-hosted git repository.

alsuliman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git


The following commit(s) were added to refs/heads/master by this push:
     new 0a5528c  [NO ISSUE][COMP] Fix InlineUnnestFunction to return false if not fired
0a5528c is described below

commit 0a5528c38611ae8636ac1cf9a7d102d7b36a3dda
Author: Ali Alsuliman <al...@gmail.com>
AuthorDate: Mon Aug 19 21:55:10 2019 -0700

    [NO ISSUE][COMP] Fix InlineUnnestFunction to return false if not fired
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    Fix InlineUnnestFunctionRule to return false if there are no
    changes in the plan.
    
    - minor clean-ups.
    
    Change-Id: Ib2b69ae3ad9712d1443078e0ba0b254b46376d43
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/3526
    Contrib: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Ali Alsuliman <al...@gmail.com>
    Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
---
 .../optimizer/rules/InlineUnnestFunctionRule.java  | 31 +++++++++++-----------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InlineUnnestFunctionRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InlineUnnestFunctionRule.java
index 9f1b968..15b2d73 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InlineUnnestFunctionRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/InlineUnnestFunctionRule.java
@@ -67,22 +67,22 @@ public class InlineUnnestFunctionRule implements IAlgebraicRewriteRule {
         UnnestOperator unnestOperator = (UnnestOperator) op1;
         AbstractFunctionCallExpression expr =
                 (AbstractFunctionCallExpression) unnestOperator.getExpressionRef().getValue();
-        //we only inline for the scan-collection function
+        // we only inline for the scan-collection function
         if (expr.getFunctionIdentifier() != BuiltinFunctions.SCAN_COLLECTION) {
             return false;
         }
 
         // inline all variables from an unnesting function call
-        AbstractFunctionCallExpression funcExpr = expr;
-        List<Mutable<ILogicalExpression>> args = funcExpr.getArguments();
+        List<Mutable<ILogicalExpression>> args = expr.getArguments();
+        boolean changed = false;
         for (int i = 0; i < args.size(); i++) {
             ILogicalExpression argExpr = args.get(i).getValue();
             if (argExpr.getExpressionTag() == LogicalExpressionTag.VARIABLE) {
                 VariableReferenceExpression varExpr = (VariableReferenceExpression) argExpr;
-                inlineVariable(varExpr.getVariableReference(), unnestOperator);
+                changed |= inlineVariable(varExpr.getVariableReference(), unnestOperator);
             }
         }
-        return true;
+        return changed;
     }
 
     /**
@@ -94,38 +94,38 @@ public class InlineUnnestFunctionRule implements IAlgebraicRewriteRule {
      *            The unnest operator.
      * @throws AlgebricksException
      */
-    private void inlineVariable(LogicalVariable usedVar, UnnestOperator unnestOp) throws AlgebricksException {
+    private boolean inlineVariable(LogicalVariable usedVar, UnnestOperator unnestOp) throws AlgebricksException {
         AbstractFunctionCallExpression expr = (AbstractFunctionCallExpression) unnestOp.getExpressionRef().getValue();
-        List<Pair<AbstractFunctionCallExpression, Integer>> parentAndIndexList =
-                new ArrayList<Pair<AbstractFunctionCallExpression, Integer>>();
+        List<Pair<AbstractFunctionCallExpression, Integer>> parentAndIndexList = new ArrayList<>();
         getParentFunctionExpression(usedVar, expr, parentAndIndexList);
         ILogicalExpression usedVarOrginExpr =
                 findUsedVarOrigin(usedVar, unnestOp, (AbstractLogicalOperator) unnestOp.getInputs().get(0).getValue());
         if (usedVarOrginExpr != null) {
             for (Pair<AbstractFunctionCallExpression, Integer> parentAndIndex : parentAndIndexList) {
-                //we only rewrite the top scan-collection function
+                // we only rewrite the top scan-collection function
                 if (parentAndIndex.first.getFunctionIdentifier() == BuiltinFunctions.SCAN_COLLECTION
                         && parentAndIndex.first == expr) {
                     unnestOp.getExpressionRef().setValue(usedVarOrginExpr);
                 }
             }
+            return true;
         }
+        return false;
     }
 
-    private void getParentFunctionExpression(LogicalVariable usedVar, ILogicalExpression expr,
+    private void getParentFunctionExpression(LogicalVariable usedVar, AbstractFunctionCallExpression funcExpr,
             List<Pair<AbstractFunctionCallExpression, Integer>> parentAndIndexList) {
-        AbstractFunctionCallExpression funcExpr = (AbstractFunctionCallExpression) expr;
         List<Mutable<ILogicalExpression>> args = funcExpr.getArguments();
         for (int i = 0; i < args.size(); i++) {
             ILogicalExpression argExpr = args.get(i).getValue();
             if (argExpr.getExpressionTag() == LogicalExpressionTag.VARIABLE) {
                 VariableReferenceExpression varExpr = (VariableReferenceExpression) argExpr;
                 if (varExpr.getVariableReference().equals(usedVar)) {
-                    parentAndIndexList.add(new Pair<AbstractFunctionCallExpression, Integer>(funcExpr, i));
+                    parentAndIndexList.add(new Pair<>(funcExpr, i));
                 }
             }
             if (argExpr.getExpressionTag() == LogicalExpressionTag.FUNCTION_CALL) {
-                getParentFunctionExpression(usedVar, argExpr, parentAndIndexList);
+                getParentFunctionExpression(usedVar, (AbstractFunctionCallExpression) argExpr, parentAndIndexList);
             }
         }
     }
@@ -134,7 +134,7 @@ public class InlineUnnestFunctionRule implements IAlgebraicRewriteRule {
             AbstractLogicalOperator currentOp) throws AlgebricksException {
         ILogicalExpression ret = null;
         if (currentOp.getOperatorTag() == LogicalOperatorTag.ASSIGN) {
-            List<LogicalVariable> producedVars = new ArrayList<LogicalVariable>();
+            List<LogicalVariable> producedVars = new ArrayList<>();
             VariableUtilities.getProducedVariables(currentOp, producedVars);
             if (producedVars.contains(usedVar)) {
                 AssignOperator assignOp = (AssignOperator) currentOp;
@@ -148,7 +148,7 @@ public class InlineUnnestFunctionRule implements IAlgebraicRewriteRule {
                         ret = returnedExpr;
                     }
                 } else if (returnedExpr.getExpressionTag() == LogicalExpressionTag.VARIABLE) {
-                    //recusively inline
+                    // recursively inline
                     VariableReferenceExpression varExpr = (VariableReferenceExpression) returnedExpr;
                     LogicalVariable var = varExpr.getVariableReference();
                     ILogicalExpression finalExpr = findUsedVarOrigin(var, currentOp,
@@ -173,6 +173,7 @@ public class InlineUnnestFunctionRule implements IAlgebraicRewriteRule {
 
     private void removeUnecessaryAssign(AbstractLogicalOperator parentOp, AbstractLogicalOperator currentOp,
             AssignOperator assignOp, int index) {
+        // TODO: how come the assign variable is removed before checking that other operators might be using it?
         assignOp.getVariables().remove(index);
         assignOp.getExpressions().remove(index);
         if (assignOp.getVariables().size() == 0) {