eejbyfeldt commented on code in PR #13184:
URL: https://github.com/apache/datafusion/pull/13184#discussion_r1825847913
##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -429,41 +434,63 @@ fn push_down_all_join(
let mut keep_predicates = vec![];
let mut join_conditions = vec![];
let mut checker = ColumnChecker::new(left_schema, right_schema);
+
for predicate in predicates {
- if left_preserved && checker.is_left_only(&predicate) {
+ let columns = predicate.column_refs();
+ macro_rules! restrict_null {
+ () => {{
+ let predicate_cloned = predicate.clone();
+ let cols = columns.iter().cloned();
+ is_restrict_null_predicate(predicate_cloned,
cols).unwrap_or(false)
+ }};
+ }
+
+ if checker.left_only(&columns) && (left_preserved || restrict_null!())
{
left_push.push(predicate);
- } else if right_preserved && checker.is_right_only(&predicate) {
+ } else if checker.right_only(&columns) && (right_preserved ||
restrict_null!()) {
Review Comment:
This seems incorrect. Why would a predicate being restrict null allow us to
push it? It filtering it out null seems to be a argument against why it can be
pushed down.
--
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]
