This is an automated email from the ASF dual-hosted git repository. dlych pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 639b836e17746903a48cfa19df74106946a6b7da Author: Dmitry Lychagin <[email protected]> AuthorDate: Wed Sep 30 13:44:02 2020 -0700 [NO ISSUE][COMP] Fix outer join handling in some optimizer rules - user model changes: no - storage format changes: no - interface changes: no Details: - LoadRecordFieldsRule should not search for fields in the right branch of a left outer join - ExtractCommonExpressionsRule should not perform common expression extraction if one expression is in the right branch of a left outer join and another expression is above the outer join Change-Id: Ia76d013d9adb13dce75af06dbe22801afe56424f Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8184 Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Dmitry Lychagin <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> --- .../optimizer/rules/LoadRecordFieldsRule.java | 12 +++++++- .../right_branch_opt_1.1.query.sqlpp | 25 ++++++++++++++++ .../right_branch_opt_1/right_branch_opt_1.1.adm | 4 +++ .../test/resources/runtimets/testsuite_sqlpp.xml | 5 ++++ .../rules/ExtractCommonExpressionsRule.java | 35 ++++++++++++++++++++++ 5 files changed, 80 insertions(+), 1 deletion(-) diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java index ce1312f..b9d512b 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/LoadRecordFieldsRule.java @@ -309,7 +309,17 @@ public class LoadRecordFieldsRule implements IAlgebraicRewriteRule { private static ILogicalExpression findFieldExpression(AbstractLogicalOperator op, LogicalVariable recordVar, Object accessKey, IVariableTypeEnvironment typeEnvironment, FieldResolver resolver) throws AlgebricksException { - for (Mutable<ILogicalOperator> child : op.getInputs()) { + List<Mutable<ILogicalOperator>> inputs = op.getInputs(); + int inputIdxLimit; + if (op.getOperatorTag() == LogicalOperatorTag.LEFTOUTERJOIN) { + // do not search in the right branch of a left outer join + inputIdxLimit = 1; + } else { + inputIdxLimit = inputs.size(); + } + + for (int inputIdx = 0; inputIdx < inputIdxLimit; inputIdx++) { + Mutable<ILogicalOperator> child = inputs.get(inputIdx); AbstractLogicalOperator opChild = (AbstractLogicalOperator) child.getValue(); if (opChild.getOperatorTag() == LogicalOperatorTag.ASSIGN) { AssignOperator op2 = (AssignOperator) opChild; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/right_branch_opt_1/right_branch_opt_1.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/right_branch_opt_1/right_branch_opt_1.1.query.sqlpp new file mode 100644 index 0000000..016b065 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/right_branch_opt_1/right_branch_opt_1.1.query.sqlpp @@ -0,0 +1,25 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +with + t1 as (from range(1, 4) v select v, -v nv, {} as x), + t2 as (from range(1, 2) v select v, -v nv, {} as x) +from t1 left outer join t2 on t1.v = t2.v +select t1.nv t1nv, t2.nv t2nv, t1.x as t1x, t2.x as t2x +order by t1nv desc; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/right_branch_opt_1/right_branch_opt_1.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/right_branch_opt_1/right_branch_opt_1.1.adm new file mode 100644 index 0000000..19eb6b6 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/right_branch_opt_1/right_branch_opt_1.1.adm @@ -0,0 +1,4 @@ +{ "t1nv": -1, "t2nv": -1, "t1x": { }, "t2x": { } } +{ "t1nv": -2, "t2nv": -2, "t1x": { }, "t2x": { } } +{ "t1nv": -3, "t1x": { } } +{ "t1nv": -4, "t1x": { } } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index ca9e269..6d50376 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -11820,6 +11820,11 @@ <output-dir compare="Text">empty-dataset</output-dir> </compilation-unit> </test-case> + <test-case FilePath="leftouterjoin"> + <compilation-unit name="right_branch_opt_1"> + <output-dir compare="Text">right_branch_opt_1</output-dir> + </compilation-unit> + </test-case> </test-group> <test-group name="index-leftouterjoin"> <test-case FilePath="index-leftouterjoin"> diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java index 2cfa241..9420498 100644 --- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java +++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java @@ -19,6 +19,7 @@ package org.apache.hyracks.algebricks.rewriter.rules; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -82,6 +83,9 @@ public class ExtractCommonExpressionsRule implements IAlgebraicRewriteRule { private final Map<ILogicalExpression, ExprEquivalenceClass> exprEqClassMap = new HashMap<ILogicalExpression, ExprEquivalenceClass>(); + private final List<LogicalVariable> tmpLiveVars = new ArrayList<>(); + private final List<LogicalVariable> tmpProducedVars = new ArrayList<>(); + // Set of operators for which common subexpression elimination should not be performed. private static final Set<LogicalOperatorTag> ignoreOps = new HashSet<LogicalOperatorTag>(6); @@ -123,6 +127,31 @@ public class ExtractCommonExpressionsRule implements IAlgebraicRewriteRule { exprEqClass.setVariable(lhs); } + // remove equivalence classes that supply given vars + private void pruneEquivalenceClassMap(Collection<LogicalVariable> pruneVars) throws AlgebricksException { + for (Iterator<Map.Entry<ILogicalExpression, ExprEquivalenceClass>> i = exprEqClassMap.entrySet().iterator(); i + .hasNext();) { + Map.Entry<ILogicalExpression, ExprEquivalenceClass> me = i.next(); + ExprEquivalenceClass eqClass = me.getValue(); + boolean eqClassProvidesPruneVar = false; + if (eqClass.variableIsSet() && pruneVars.contains(eqClass.getVariable())) { + eqClassProvidesPruneVar = true; + } else { + tmpProducedVars.clear(); + VariableUtilities.getProducedVariables(eqClass.getFirstOperator(), tmpProducedVars); + for (LogicalVariable producedVar : tmpProducedVars) { + if (pruneVars.contains(producedVar)) { + eqClassProvidesPruneVar = true; + break; + } + } + } + if (eqClassProvidesPruneVar) { + i.remove(); + } + } + } + private boolean removeCommonExpressions(Mutable<ILogicalOperator> opRef, IOptimizationContext context) throws AlgebricksException { AbstractLogicalOperator op = (AbstractLogicalOperator) opRef.getValue(); @@ -187,6 +216,12 @@ public class ExtractCommonExpressionsRule implements IAlgebraicRewriteRule { // Update equivalence class map with original assign expression. updateEquivalenceClassMap(lhs, exprRef, originalAssignExprs.get(i), op); } + } else if (op.getOperatorTag() == LogicalOperatorTag.LEFTOUTERJOIN) { + // remove equivalence classes that provide vars that are live on the right branch of a left outer join + ILogicalOperator rightBranchOp = op.getInputs().get(1).getValue(); + tmpLiveVars.clear(); + VariableUtilities.getLiveVariables(rightBranchOp, tmpLiveVars); + pruneEquivalenceClassMap(tmpLiveVars); } // TODO: For now do not perform replacement in nested plans
