viirya commented on code in PR #11203:
URL: https://github.com/apache/datafusion/pull/11203#discussion_r1662578537


##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -441,11 +442,11 @@ fn push_down_all_join(
 
     // Extract from OR clause, generate new predicates for both side of join 
if possible.
     // We only track the unpushable predicates above.
-    if left_preserved {
+    if on_left_preserved {
         left_push.extend(extract_or_clauses_for_join(&keep_predicates, 
left_schema));
         left_push.extend(extract_or_clauses_for_join(&join_conditions, 
left_schema));
     }
-    if right_preserved {
+    if on_right_preserved {

Review Comment:
   Actually we wrongly mix join filter predicates and filter predicates into 
`join_conditions`. I separate them now so only `on_filter_join_conditions` will 
be considered with `on_preserved`.



##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -435,22 +436,37 @@ fn push_down_all_join(
             {
                 right_push.push(on)
             } else {
-                join_conditions.push(on)
+                on_filter_join_conditions.push(on)
             }
         }
     }
 
     // Extract from OR clause, generate new predicates for both side of join 
if possible.
     // We only track the unpushable predicates above.
-    if on_left_preserved {
+    if left_preserved {
         left_push.extend(extract_or_clauses_for_join(&keep_predicates, 
left_schema));
         left_push.extend(extract_or_clauses_for_join(&join_conditions, 
left_schema));
     }
-    if on_right_preserved {
+    if left_preserved {

Review Comment:
   Oops



##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -441,11 +442,11 @@ fn push_down_all_join(
 
     // Extract from OR clause, generate new predicates for both side of join 
if possible.
     // We only track the unpushable predicates above.
-    if left_preserved {
+    if on_left_preserved {
         left_push.extend(extract_or_clauses_for_join(&keep_predicates, 
left_schema));
         left_push.extend(extract_or_clauses_for_join(&join_conditions, 
left_schema));
     }
-    if right_preserved {
+    if on_right_preserved {

Review Comment:
   I tried to add a test case with it.



##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -441,11 +442,11 @@ fn push_down_all_join(
 
     // Extract from OR clause, generate new predicates for both side of join 
if possible.
     // We only track the unpushable predicates above.
-    if left_preserved {
+    if on_left_preserved {
         left_push.extend(extract_or_clauses_for_join(&keep_predicates, 
left_schema));
         left_push.extend(extract_or_clauses_for_join(&join_conditions, 
left_schema));
     }
-    if right_preserved {
+    if on_right_preserved {

Review Comment:
   I tried to add a test case with it.
   
   EDIT: Seems it doesn't test against the corner case. 🤔 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to