[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 <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Ali Alsuliman <ali.al.solaiman@gmail.com>
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 @@
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 @@
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 @@
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 @@
context.computeAndSetTypeEnvironmentForOperator(op);
}
joinBranch.setValue(topOp);
+ return true;
}
private static void addCondToJoin(SelectOperator select, AbstractBinaryJoinOperator join,