godfreyhe commented on code in PR #22049:
URL: https://github.com/apache/flink/pull/22049#discussion_r1135047849
##########
flink-table/flink-table-planner/src/test/resources/org/apache/flink/table/planner/plan/batch/sql/MultipleInputCreationTest.xml:
##########
@@ -136,11 +136,9 @@ LogicalProject(a=[$0], d=[$1], e=[$2], f=[$3], ny=[$4],
a0=[$5], b=[$6], c=[$7])
<![CDATA[
MultipleInput(readOrder=[0,0,1],
members=[\nNestedLoopJoin(joinType=[LeftOuterJoin], where=[(a = a0)],
select=[a, d, e, f, ny, a0, b, c], build=[right])\n:-
NestedLoopJoin(joinType=[LeftOuterJoin], where=[(a = d)], select=[a, d, e, f,
ny], build=[right])\n: :- Calc(select=[a], where=[(a > 10)])\n: : +- [#3]
BoundedStreamScan(table=[[default_catalog, default_database, chainable]],
fields=[a])\n: +- [#2] Exchange(distribution=[broadcast])\n+- [#1]
Exchange(distribution=[broadcast])\n])
:- Exchange(distribution=[broadcast])
-: +- Calc(select=[a, b, c], where=[(a > 10)])
Review Comment:
why the left side filters can not be pushed down ?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/logical/FlinkFilterJoinRule.java:
##########
@@ -386,6 +387,21 @@ private void pushFiltersToAnotherSide(
}
}
+ private boolean isSuitableFilterToPush(RexNode filter, JoinRelType
joinType) {
+ if (filter.isAlwaysTrue()) {
+ return false;
+ }
+ // For left/right outer join, we cannot push down IS_NULL filter to
other side. Take left
+ // outer join as an example, If the join right side contains an
IS_NULL filter, while we try
+ // to push it to the join left side and the left side have any other
filter on this column,
+ // which will conflict and generate wrong plan.
+ if ((joinType == JoinRelType.LEFT || joinType == JoinRelType.RIGHT)
Review Comment:
please also take care about `a2 = null`
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/rules/logical/FlinkFilterJoinRuleTest.java:
##########
@@ -240,6 +240,21 @@ public void testLeftJoinWithAllFiltersFromWhere() {
"SELECT * FROM MyTable1 LEFT JOIN MyTable2 ON true WHERE b1 =
b2 AND c1 = c2 AND a2 = 2 AND b2 > 10 AND COALESCE(c1, c2) <> '' ");
}
+ @Test
Review Comment:
please add some tests (including IT case) for inner joins
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/logical/FlinkFilterJoinRule.java:
##########
@@ -386,6 +387,21 @@ private void pushFiltersToAnotherSide(
}
}
+ private boolean isSuitableFilterToPush(RexNode filter, JoinRelType
joinType) {
+ if (filter.isAlwaysTrue()) {
+ return false;
+ }
+ // For left/right outer join, we cannot push down IS_NULL filter to
other side. Take left
+ // outer join as an example, If the join right side contains an
IS_NULL filter, while we try
+ // to push it to the join left side and the left side have any other
filter on this column,
+ // which will conflict and generate wrong plan.
+ if ((joinType == JoinRelType.LEFT || joinType == JoinRelType.RIGHT)
Review Comment:
additional, less(or equal) than, greater (or equal) than a literal can also
be supported
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
