You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/02/16 04:20:15 UTC
[GitHub] [calcite] jcamachor opened a new pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
jcamachor opened a new pull request #2349:
URL: https://github.com/apache/calcite/pull/2349
…mijoin input
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zabetak commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r577993154
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2810,19 +2810,14 @@ public static boolean classifyFilters(
List<RexNode> rightFilters) {
RexBuilder rexBuilder = joinRel.getCluster().getRexBuilder();
List<RelDataTypeField> joinFields = joinRel.getRowType().getFieldList();
- final int nTotalFields = joinFields.size();
final int nSysFields = 0; // joinRel.getSystemFieldList().size();
final List<RelDataTypeField> leftFields =
joinRel.getInputs().get(0).getRowType().getFieldList();
final int nFieldsLeft = leftFields.size();
final List<RelDataTypeField> rightFields =
joinRel.getInputs().get(1).getRowType().getFieldList();
final int nFieldsRight = rightFields.size();
-
- // SemiJoin, CorrelateSemiJoin, CorrelateAntiJoin: right fields are not returned
- assert nTotalFields == (!joinType.projectsRight()
- ? nSysFields + nFieldsLeft
- : nSysFields + nFieldsLeft + nFieldsRight);
+ final int nTotalFields = nFieldsLeft + nFieldsRight;
Review comment:
I may be wrong but I have the impression that with this change if somebody calls the method with an `ANTI` join it might push a condition to the right side and possibly affecting correctness. I don't think it happens at the moment at least with the Calcite rules but it may be worth adding some kind of assertion for extra protection.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zabetak commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579834067
##########
File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
##########
@@ -152,4 +152,29 @@ public JoinRelType cancelNullsOnRight() {
public boolean projectsRight() {
return this != SEMI && this != ANTI;
}
+
+ /** Returns whether this join type accepts pushing predicates from above into its predicate. */
+ public boolean canPushIntoFromAbove() {
+ return (this == INNER) || (this == SEMI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from above into its left input. */
+ public boolean canPushLeftFromAbove() {
+ return (this == INNER) || (this == LEFT) || (this == SEMI) || (this == ANTI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from above into its right input. */
+ public boolean canPushRightFromAbove() {
+ return (this == INNER) || (this == RIGHT);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from within into its left input. */
+ public boolean canPushLeftFromWithin() {
+ return (this == INNER) || (this == RIGHT) || (this == SEMI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from within into its right input. */
+ public boolean canPushRightFromWithin() {
+ return (this == INNER) || (this == LEFT) || (this == SEMI);
+ }
Review comment:
These new methods in `JoinRelType` are used only for feeding `RelOptUtil#classifyFilters` method so I am not sure if it is the best place to put them here but I don't have a better idea at the moment. I would suggest to mark them as experimental to have some flexibility if we find a better approach in the near future. WDYT?
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2821,8 +2941,8 @@ public static boolean classifyFilters(
// SemiJoin, CorrelateSemiJoin, CorrelateAntiJoin: right fields are not returned
assert nTotalFields == (!joinType.projectsRight()
- ? nSysFields + nFieldsLeft
- : nSysFields + nFieldsLeft + nFieldsRight);
+ ? nSysFields + nFieldsLeft
+ : nSysFields + nFieldsLeft + nFieldsRight);
Review comment:
nit: formatting?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] jcamachor commented on pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
jcamachor commented on pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#issuecomment-853158956
@rubenada , @zabetak , thanks for the reminder. Let me rebase and push it to the finish line.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] jcamachor commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579373706
##########
File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
##########
@@ -152,4 +152,29 @@ public JoinRelType cancelNullsOnRight() {
public boolean projectsRight() {
return this != SEMI && this != ANTI;
}
+
+ /** Returns whether this join type accepts pushing predicates from above into its predicate. */
+ public boolean canPushIntoFromAbove() {
+ return (this == INNER) || (this == SEMI);
Review comment:
I'm thinking about this in terms of the algebra rather than SQL. The rule can match a filter on top of ANTI, and you are trying to push those expressions from above into the join condition, to the left input as a filter, or the right input as a filter. `canPushLeftFromAbove` returns true for ANTI indeed as you described. However, `canPushIntoFromAbove` returns false for ANTI because it's not semantically equivalent to filter after ANTI and to filter within the ANTI. SEMI semantics are different.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579130907
##########
File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
##########
@@ -152,4 +152,29 @@ public JoinRelType cancelNullsOnRight() {
public boolean projectsRight() {
return this != SEMI && this != ANTI;
}
+
+ /** Returns whether this join type accepts pushing predicates from above into its predicate. */
+ public boolean canPushIntoFromAbove() {
+ return (this == INNER) || (this == SEMI);
Review comment:
Are we sure ANTI should not behave as SEMI in `canPushIntoFromAbove`?
In fact, can it really happen a "pushIntoFromAbove" in the context of SEMI (or ANTI)? If we push from above a SEMI/ANTI, we know the predicate cannot reference the RHS (because SEMI/ANTI don't project it), so it means the predicate should be based on the LHS, which means it would be pushLeft (not into). Maybe there's some scenario I'm forgetting about where a push into can actually happen, so it makes sense to keep the SEMI here, but then I guess ANTI should be added too?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#issuecomment-848650443
@jcamachor do you think you could make a final check on the PR (rebase, resolve conflits, check @zabetak last minor comments, ...) so that we can squeeze this in the next version?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r578224452
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2810,19 +2810,14 @@ public static boolean classifyFilters(
List<RexNode> rightFilters) {
RexBuilder rexBuilder = joinRel.getCluster().getRexBuilder();
List<RelDataTypeField> joinFields = joinRel.getRowType().getFieldList();
- final int nTotalFields = joinFields.size();
final int nSysFields = 0; // joinRel.getSystemFieldList().size();
final List<RelDataTypeField> leftFields =
joinRel.getInputs().get(0).getRowType().getFieldList();
final int nFieldsLeft = leftFields.size();
final List<RelDataTypeField> rightFields =
joinRel.getInputs().get(1).getRowType().getFieldList();
final int nFieldsRight = rightFields.size();
-
- // SemiJoin, CorrelateSemiJoin, CorrelateAntiJoin: right fields are not returned
- assert nTotalFields == (!joinType.projectsRight()
- ? nSysFields + nFieldsLeft
- : nSysFields + nFieldsLeft + nFieldsRight);
+ final int nTotalFields = nFieldsLeft + nFieldsRight;
Review comment:
I agree with @zabetak , I think in case of ANTI the current logic should not be modified.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579375728
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java
##########
@@ -110,10 +110,9 @@ protected void perform(RelOptRuleCall call, @Nullable Filter filter,
if (RelOptUtil.classifyFilters(
join,
aboveFilters,
- joinType,
- true,
- !joinType.generatesNullsOnLeft(),
- !joinType.generatesNullsOnRight(),
+ joinType.canPushIntoFromAbove(),
Review comment:
You're right, good point.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] jcamachor commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579368274
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java
##########
@@ -110,10 +110,9 @@ protected void perform(RelOptRuleCall call, @Nullable Filter filter,
if (RelOptUtil.classifyFilters(
join,
aboveFilters,
- joinType,
- true,
- !joinType.generatesNullsOnLeft(),
- !joinType.generatesNullsOnRight(),
+ joinType.canPushIntoFromAbove(),
Review comment:
Note that the old code was checking internally `if (!joinType.isOuterJoin() && pushInto) {`. Thus, even if `true` was passed, the check on the join type was determining whether we were pushing into or not.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579130065
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2798,6 +2917,7 @@ public static JoinRelType simplifyJoin(RelNode joinRel,
* @param rightFilters list of filters to push to the right child
* @return whether at least one filter was pushed
*/
+ @Deprecated // to be removed before 2.0
Review comment:
If we go into this direction, probably the "deprecated flag" should be added in the javadoc too:
`@deprecated Use newMethod`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] asfgit closed pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2349:
URL: https://github.com/apache/calcite/pull/2349
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] asfgit closed pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2349:
URL: https://github.com/apache/calcite/pull/2349
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579130065
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2798,6 +2917,7 @@ public static JoinRelType simplifyJoin(RelNode joinRel,
* @param rightFilters list of filters to push to the right child
* @return whether at least one filter was pushed
*/
+ @Deprecated // to be removed before 2.0
Review comment:
If we go into this direction, probably the "deprecated flag" it should be added in the javadoc too:
`@deprecated Use newMethod`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#issuecomment-853174427
> @rubenada , @zabetak , thanks for the reminder. Let me rebase and push it to the finish line.
@jcamachor well don't push it immediately since we have a code freeze (ongoing release) :)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] jcamachor commented on pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
jcamachor commented on pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#issuecomment-853180275
Yeah, absolutely, I'm first updating the PR.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579130907
##########
File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
##########
@@ -152,4 +152,29 @@ public JoinRelType cancelNullsOnRight() {
public boolean projectsRight() {
return this != SEMI && this != ANTI;
}
+
+ /** Returns whether this join type accepts pushing predicates from above into its predicate. */
+ public boolean canPushIntoFromAbove() {
+ return (this == INNER) || (this == SEMI);
Review comment:
Are we sure ANTI does not belong to `canPushIntoFromAbove`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r580067228
##########
File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
##########
@@ -152,4 +152,29 @@ public JoinRelType cancelNullsOnRight() {
public boolean projectsRight() {
return this != SEMI && this != ANTI;
}
+
+ /** Returns whether this join type accepts pushing predicates from above into its predicate. */
+ public boolean canPushIntoFromAbove() {
+ return (this == INNER) || (this == SEMI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from above into its left input. */
+ public boolean canPushLeftFromAbove() {
+ return (this == INNER) || (this == LEFT) || (this == SEMI) || (this == ANTI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from above into its right input. */
+ public boolean canPushRightFromAbove() {
+ return (this == INNER) || (this == RIGHT);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from within into its left input. */
+ public boolean canPushLeftFromWithin() {
+ return (this == INNER) || (this == RIGHT) || (this == SEMI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from within into its right input. */
+ public boolean canPushRightFromWithin() {
+ return (this == INNER) || (this == LEFT) || (this == SEMI);
+ }
Review comment:
An alternative could be adding these new methods as static methods in RelOptUtil receiving a Join or JoinRelType as parameter, perhaps?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] jcamachor commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r578952512
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -2810,19 +2810,14 @@ public static boolean classifyFilters(
List<RexNode> rightFilters) {
RexBuilder rexBuilder = joinRel.getCluster().getRexBuilder();
List<RelDataTypeField> joinFields = joinRel.getRowType().getFieldList();
- final int nTotalFields = joinFields.size();
final int nSysFields = 0; // joinRel.getSystemFieldList().size();
final List<RelDataTypeField> leftFields =
joinRel.getInputs().get(0).getRowType().getFieldList();
final int nFieldsLeft = leftFields.size();
final List<RelDataTypeField> rightFields =
joinRel.getInputs().get(1).getRowType().getFieldList();
final int nFieldsRight = rightFields.size();
-
- // SemiJoin, CorrelateSemiJoin, CorrelateAntiJoin: right fields are not returned
- assert nTotalFields == (!joinType.projectsRight()
- ? nSysFields + nFieldsLeft
- : nSysFields + nFieldsLeft + nFieldsRight);
+ final int nTotalFields = nFieldsLeft + nFieldsRight;
Review comment:
@zabetak , @rubenada , thanks for the feedback. You are right: This could potentially cause issues with ANTI join that were not happening before. I was exploring the method and this is all quite messy since join type semantics are mixed with the semantics of the input parameters such as `pushInto`, `pushLeft`, and `pushRight`. Fwiw ANTI join + pushInto == true does not seem to be handled correctly without this patch either.
I have pushed an addendum that takes a slightly different approach. It deprecates the current `classifyFilters` method and creates an alternative that does not rely on join type internally, only on input boolean parameters. Then I changed all calls to `classifyFilters` to use this new method (I also created some utility methods to keep semantics contained within join type). Let me know what you think, I can always take a different approach if you have other ideas.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] jcamachor commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r644129961
##########
File path: core/src/main/java/org/apache/calcite/rel/core/JoinRelType.java
##########
@@ -152,4 +152,29 @@ public JoinRelType cancelNullsOnRight() {
public boolean projectsRight() {
return this != SEMI && this != ANTI;
}
+
+ /** Returns whether this join type accepts pushing predicates from above into its predicate. */
+ public boolean canPushIntoFromAbove() {
+ return (this == INNER) || (this == SEMI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from above into its left input. */
+ public boolean canPushLeftFromAbove() {
+ return (this == INNER) || (this == LEFT) || (this == SEMI) || (this == ANTI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from above into its right input. */
+ public boolean canPushRightFromAbove() {
+ return (this == INNER) || (this == RIGHT);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from within into its left input. */
+ public boolean canPushLeftFromWithin() {
+ return (this == INNER) || (this == RIGHT) || (this == SEMI);
+ }
+
+ /** Returns whether this join type accepts pushing predicates from within into its right input. */
+ public boolean canPushRightFromWithin() {
+ return (this == INNER) || (this == LEFT) || (this == SEMI);
+ }
Review comment:
Added the experimental annotation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2349: [CALCITE-4499] FilterJoinRule misses opportunity to push filter to se…
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2349:
URL: https://github.com/apache/calcite/pull/2349#discussion_r579128240
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java
##########
@@ -110,10 +110,9 @@ protected void perform(RelOptRuleCall call, @Nullable Filter filter,
if (RelOptUtil.classifyFilters(
join,
aboveFilters,
- joinType,
- true,
- !joinType.generatesNullsOnLeft(),
- !joinType.generatesNullsOnRight(),
+ joinType.canPushIntoFromAbove(),
Review comment:
this `true` (old code) vs `joinType.canPushIntoFromAbove()` (new code) seems to be a change in the logic. Not sure which one is correct....
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org