This is an automated email from the ASF dual-hosted git repository. mblow pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 36ae1017115cc25cb23c7f9e6f94fc59c2516733 Author: Dmitry Lychagin <[email protected]> AuthorDate: Thu Apr 16 13:10:36 2020 -0700 [NO ISSUE][COMP] Fix issues in PushSelectIntoJoinRule - user model changes: no - storage format changes: no - interface changes: no Details: - Properly track whether the rewriting had happened and report this to the rule controller - Do not push condition into the join condition of a left outer join - Do not push condition into a right branch of a left outer join - Add tests for the above cases Change-Id: Ic2f0c8d5ac885f1da3d4e6bafa9fcd14c098a89f Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/5844 Tested-by: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> --- .../loj-02-push-select.01.ddl.sqlpp | 32 ++++++++ .../loj-02-push-select.02.update.sqlpp | 34 ++++++++ .../loj-02-push-select.03.query.sqlpp | 32 ++++++++ .../loj-02-push-select.04.query.sqlpp | 33 ++++++++ .../loj-02-push-select.05.query.sqlpp | 33 ++++++++ .../loj-02-push-select.06.query.sqlpp | 33 ++++++++ .../loj-02-push-select/loj-02-push-select.03.adm | 2 + .../loj-02-push-select/loj-02-push-select.04.adm | 2 + .../loj-02-push-select/loj-02-push-select.05.adm | 2 + .../loj-02-push-select/loj-02-push-select.06.adm | 2 + .../test/resources/runtimets/testsuite_sqlpp.xml | 5 ++ .../rewriter/rules/PushSelectIntoJoinRule.java | 91 +++++++++++++++------- 12 files changed, 271 insertions(+), 30 deletions(-) diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.01.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.01.ddl.sqlpp new file mode 100644 index 0000000..e656d9d --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.01.ddl.sqlpp @@ -0,0 +1,32 @@ +/* + * 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. + */ + +drop dataverse test if exists; +create dataverse test; + +use test; + +create type test.TestType as +{ + id : integer +}; + +create dataset t1(TestType) primary key id; + +create dataset t2(TestType) primary key id; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.02.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.02.update.sqlpp new file mode 100644 index 0000000..950f728 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.02.update.sqlpp @@ -0,0 +1,34 @@ +/* + * 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. + */ + +use test; + +insert into t1 +([ + {"id":1, "x":1, "a": 11 }, + {"id":2, "x":2, "a": 12 }, + {"id":3, "x":3, "a": 13 }, + {"id":4, "x":4, "a": 14 } +]); + +insert into t2 +([ + {"id":1, "y":1, "b": 111 }, + {"id":2, "y":2, "b": 112 } +]); diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.03.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.03.query.sqlpp new file mode 100644 index 0000000..5be33b9 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.03.query.sqlpp @@ -0,0 +1,32 @@ +/* + * 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. + */ + +/* + * Test WHERE .. IS MISSING using right side of an outer join. + * 1:1 left outer join. + * Expect only 2 tuples in the result (x=3 and x=4 that didn't join) + * because t2_right.y evaluates to MISSING in those tuples. + */ + +use test; + +select * +from t1 t1_left left join t2 t2_right on t1_left.x = t2_right.y +where t2_right.y is missing +order by t1_left.a; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.04.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.04.query.sqlpp new file mode 100644 index 0000000..20ebff4 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.04.query.sqlpp @@ -0,0 +1,33 @@ +/* + * 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. + */ + +/* + * Test WHERE .. IS UNKNOWN using right side of an outer join. + * 1:1 left outer join. + * Expect only 2 tuples in the result (x=3 and x=4 that didn't join) + * because t2_right.y evaluates to MISSING in those tuples, + * therefore IS UNKNOWN evaluates to TRUE + */ + +use test; + +select * +from t1 t1_left left join t2 t2_right on t1_left.x = t2_right.y +where t2_right.y is unknown +order by t1_left.a; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.05.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.05.query.sqlpp new file mode 100644 index 0000000..0d9515b --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.05.query.sqlpp @@ -0,0 +1,33 @@ +/* + * 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. + */ + +/* + * Test WHERE IF_MISSING(.., "missing") using right side of an outer join. + * 1:1 left outer join. + * Expect only 2 tuples in the result (x=3 and x=4 that didn't join) + * because t2_right.y evaluates to MISSING in those tuples, + * therefore IF_MISSING(.., "missing") evaluates to "missing" + */ + +use test; + +select * +from t1 t1_left left join t2 t2_right on t1_left.x = t2_right.y +where if_missing(t2_right.y, "missing") = "missing" +order by t1_left.a; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.06.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.06.query.sqlpp new file mode 100644 index 0000000..9315032 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/leftouterjoin/loj-02-push-select/loj-02-push-select.06.query.sqlpp @@ -0,0 +1,33 @@ +/* + * 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. + */ + +/* + * Test WHERE clause using both sides of an outer join. + * 1:1 left outer join. + * Expect only 2 tuples in the result (x=1 and x=2 that did join) + * because t2_right.y and therefore to_string(t2_right.y) + * evaluate to MISSING for the other 2 that didn't join. + */ + +use test; + +select * +from t1 t1_left left join t2 t2_right on t1_left.x = t2_right.y +where to_string(t1_left.x) = to_string(t2_right.y) +order by t1_left.a; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.03.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.03.adm new file mode 100644 index 0000000..bd1fb36 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.03.adm @@ -0,0 +1,2 @@ +{ "t1_left": { "id": 3, "x": 3, "a": 13 } } +{ "t1_left": { "id": 4, "x": 4, "a": 14 } } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.04.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.04.adm new file mode 100644 index 0000000..bd1fb36 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.04.adm @@ -0,0 +1,2 @@ +{ "t1_left": { "id": 3, "x": 3, "a": 13 } } +{ "t1_left": { "id": 4, "x": 4, "a": 14 } } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.05.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.05.adm new file mode 100644 index 0000000..bd1fb36 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.05.adm @@ -0,0 +1,2 @@ +{ "t1_left": { "id": 3, "x": 3, "a": 13 } } +{ "t1_left": { "id": 4, "x": 4, "a": 14 } } \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.06.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.06.adm new file mode 100644 index 0000000..e9d3c37 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/leftouterjoin/loj-02-push-select/loj-02-push-select.06.adm @@ -0,0 +1,2 @@ +{ "t1_left": { "id": 1, "x": 1, "a": 11 }, "t2_right": { "id": 1, "y": 1, "b": 111 } } +{ "t1_left": { "id": 2, "x": 2, "a": 12 }, "t2_right": { "id": 2, "y": 2, "b": 112 } } \ 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 8fbff50..cf4a7a3 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -11721,6 +11721,11 @@ </compilation-unit> </test-case> <test-case FilePath="leftouterjoin"> + <compilation-unit name="loj-02-push-select"> + <output-dir compare="Text">loj-02-push-select</output-dir> + </compilation-unit> + </test-case> + <test-case FilePath="leftouterjoin"> <compilation-unit name="query_issue658"> <output-dir compare="Text">query_issue658</output-dir> </compilation-unit> diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectIntoJoinRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectIntoJoinRule.java index d0cd006..5f66c2f 100644 --- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectIntoJoinRule.java +++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/PushSelectIntoJoinRule.java @@ -150,18 +150,24 @@ public class PushSelectIntoJoinRule implements IAlgebraicRewriteRule { if (!intersectsBranch[0] && !intersectsBranch[1]) { return false; } + boolean planChanged; if (needToPushOps) { //We should push independent ops into the first branch that the selection depends on - if (intersectsBranch[0]) { - pushOps(pushedOnEither, joinBranchLeftRef, context); - } else { - pushOps(pushedOnEither, joinBranchRightRef, context); - } - pushOps(pushedOnLeft, joinBranchLeftRef, context); - pushOps(pushedOnRight, joinBranchRightRef, context); + planChanged = + pushOps(pushedOnEither, intersectsBranch[0] ? joinBranchLeftRef : joinBranchRightRef, context); + planChanged |= pushOps(pushedOnLeft, joinBranchLeftRef, context); + planChanged |= pushOps(pushedOnRight, joinBranchRightRef, context); + } else { + planChanged = false; } if (intersectsAllBranches) { - addCondToJoin(select, join, context); + // add condition to the join condition only if we have IJ + if (isLoj) { + notPushedStack.addFirst(select); + } else { + addCondToJoin(select, join, context); + planChanged = true; + } } else { // push down Iterator<Mutable<ILogicalOperator>> branchIter = join.getInputs().iterator(); ILogicalExpression selectCondition = select.getCondition().getValue(); @@ -172,20 +178,20 @@ public class PushSelectIntoJoinRule implements IAlgebraicRewriteRule { if (!inter) { continue; } - - // if a left outer join, if the select condition is not-missing filtering, - // we rewrite left outer join - // to inner join for this case. - if (j > 0 && isLoj && containsNotMissingFiltering(selectCondition)) { - lojToInner = true; - } - if ((j > 0 && isLoj) && containsMissingFiltering(selectCondition)) { - // Select "is-not-missing($$var)" cannot be pushed in the right branch of a LOJ; + if (j > 0 && isLoj) { + // if a LOJ and the select condition is not-missing filtering, + // we rewrite LOJ to IJ for this case. + if (containsNotMissingFiltering(selectCondition)) { + lojToInner = true; + } + // Do not push conditions into the right branch of a LOJ; notPushedStack.addFirst(select); } else { - // Conditions for the left branch can always be pushed. - // Other conditions can be pushed to the right branch of a LOJ. + // Conditions for the left branch for IJ/LOJ or + // for the right branch of IJ can always be pushed into that branch. + // We don't push conditions into the right branch of LOJ at this point. copySelectToBranch(select, branch, context); + planChanged = true; } } if (lojToInner) { @@ -194,23 +200,47 @@ public class PushSelectIntoJoinRule implements IAlgebraicRewriteRule { innerJoin.getInputs().addAll(join.getInputs()); join = innerJoin; context.computeAndSetTypeEnvironmentForOperator(join); + planChanged = true; } } - ILogicalOperator top = join; - for (ILogicalOperator npOp : notPushedStack) { - List<Mutable<ILogicalOperator>> npInpList = npOp.getInputs(); - npInpList.clear(); - npInpList.add(new MutableObject<ILogicalOperator>(top)); - context.computeAndSetTypeEnvironmentForOperator(npOp); - top = npOp; - } - opRef.setValue(top); - return true; + planChanged |= applyNonPushed(opRef, notPushedStack, join, context); + return planChanged; } - private void pushOps(List<ILogicalOperator> opList, Mutable<ILogicalOperator> joinBranch, + private boolean applyNonPushed(Mutable<ILogicalOperator> opRef, LinkedList<ILogicalOperator> notPushedStack, + ILogicalOperator top, IOptimizationContext context) throws AlgebricksException { + switch (notPushedStack.size()) { + case 0: + if (opRef.getValue() == top) { + return false; + } + opRef.setValue(top); + return true; + case 1: + ILogicalOperator notPushedOp = notPushedStack.peek(); + if (opRef.getValue() == notPushedOp && opRef.getValue().getInputs().get(0).getValue() == top) { + return false; + } + // fall thru to 'default' + default: + for (ILogicalOperator npOp : notPushedStack) { + List<Mutable<ILogicalOperator>> npInpList = npOp.getInputs(); + npInpList.clear(); + npInpList.add(new MutableObject<>(top)); + context.computeAndSetTypeEnvironmentForOperator(npOp); + top = npOp; + } + opRef.setValue(top); + return true; + } + } + + private boolean pushOps(List<ILogicalOperator> opList, Mutable<ILogicalOperator> joinBranch, IOptimizationContext context) throws AlgebricksException { + if (opList.isEmpty()) { + return false; + } ILogicalOperator topOp = joinBranch.getValue(); ListIterator<ILogicalOperator> iter = opList.listIterator(opList.size()); while (iter.hasPrevious()) { @@ -222,6 +252,7 @@ public class PushSelectIntoJoinRule implements IAlgebraicRewriteRule { context.computeAndSetTypeEnvironmentForOperator(op); } joinBranch.setValue(topOp); + return true; } private static void addCondToJoin(SelectOperator select, AbstractBinaryJoinOperator join,
