fzlzjerry commented on code in PR #24258: URL: https://github.com/apache/datafusion/pull/24258#discussion_r3765878111
########## datafusion/sqllogictest/test_files/predicates.slt: ########## @@ -976,14 +976,22 @@ create table t(x int) as values (1), (2), (3); query TT explain select x from t where x IN (1,2,3) AND x IN (4,5); ---- -logical_plan EmptyRelation: rows=0 -physical_plan EmptyExec +logical_plan +01)Filter: t.x IS NULL AND Boolean(NULL) +02)--TableScan: t projection=[x] +physical_plan +01)FilterExec: x@0 IS NULL AND NULL +02)--DataSourceExec: partitions=1, partition_sizes=[1] Review Comment: Agreed that the filter plan should still optimize away. Rewriting this to `false` is only equivalent under filter semantics; in a projection, a nullable input must still produce NULL. I kept the NULL-preserving expression rewrite and taught `EliminateFilter` to simplify positive `AND` / `OR` trees using filter truth semantics, so this case is `EmptyRelation: rows=0` again. I also added focused optimizer coverage for both operand orders and nested predicates in c2bdac8. ########## datafusion/sqllogictest/test_files/predicates.slt: ########## @@ -976,14 +976,22 @@ create table t(x int) as values (1), (2), (3); query TT explain select x from t where x IN (1,2,3) AND x IN (4,5); ---- -logical_plan EmptyRelation: rows=0 -physical_plan EmptyExec +logical_plan +01)Filter: t.x IS NULL AND Boolean(NULL) +02)--TableScan: t projection=[x] +physical_plan +01)FilterExec: x@0 IS NULL AND NULL +02)--DataSourceExec: partitions=1, partition_sizes=[1] query TT explain select x from t where x NOT IN (1,2,3,4) OR x NOT IN (5,6,7,8); ---- -logical_plan TableScan: t projection=[x] -physical_plan DataSourceExec: partitions=1, partition_sizes=[1] +logical_plan +01)Filter: t.x IS NOT NULL OR Boolean(NULL) +02)--TableScan: t projection=[x] +physical_plan +01)FilterExec: x@0 IS NOT NULL OR NULL +02)--DataSourceExec: partitions=1, partition_sizes=[1] Review Comment: Fixed in c2bdac8. In filter context, `predicate OR NULL` can reduce to `predicate` because only TRUE rows survive, so the compound filter is gone. For this nullable column the resulting plan is `Filter: t1.column1 IS NOT NULL`. A bare `TableScan` would keep NULL rows incorrectly: both `NOT IN` predicates evaluate to NULL for a NULL input, so the row must be rejected. I added runtime NULL coverage as well as filter-tree optimizer tests. -- 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]
