[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 <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Ali Alsuliman <ali.al.solaiman@gmail.com>
Reviewed-by: Dmitry Lychagin <dmitry.lychagin@couchbase.com>
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 @@
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 @@
* 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 @@
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 @@
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 @@
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) {