adriangb commented on code in PR #23106:
URL: https://github.com/apache/datafusion/pull/23106#discussion_r3708279325
##########
datafusion/sqllogictest/test_files/push_down_filter_parquet.slt:
##########
@@ -1066,6 +1066,33 @@ statement ok
drop table nej_probe;
+# Multi-key null-equal join: the IS NULL disjunct covers every nullable key,
so a probe row with a
+# NULL in either key still reaches the join and null-matches the build side.
+statement ok
+COPY (SELECT * FROM (VALUES (1, 10), (2, NULL), (NULL, 30)) v(a, b)) TO
'test_files/scratch/push_down_filter_parquet/mnej_probe.parquet' STORED AS
PARQUET;
+
+statement ok
+COPY (SELECT * FROM (VALUES (1, 10), (2, NULL)) v(a, b)) TO
'test_files/scratch/push_down_filter_parquet/mnej_build.parquet' STORED AS
PARQUET;
+
+statement ok
+CREATE EXTERNAL TABLE mnej_probe STORED AS PARQUET LOCATION
'test_files/scratch/push_down_filter_parquet/mnej_probe.parquet';
+
+statement ok
+CREATE EXTERNAL TABLE mnej_build STORED AS PARQUET LOCATION
'test_files/scratch/push_down_filter_parquet/mnej_build.parquet';
+
+query IIII rowsort
+SELECT mnej_build.a, mnej_build.b, mnej_probe.a, mnej_probe.b FROM mnej_build
JOIN mnej_probe ON (mnej_build.a IS NOT DISTINCT FROM mnej_probe.a) AND
(mnej_build.b IS NOT DISTINCT FROM mnej_probe.b)
Review Comment:
Can we add an `EXPLAIN` or `EXPLAIN ANALYZE` for this query to show the
filter is being applied?
##########
datafusion/physical-plan/src/joins/hash_join/shared_bounds.rs:
##########
@@ -692,38 +700,51 @@ impl SharedBuildAccumulator {
};
self.dynamic_filter
- .update(self.null_aware_filter(filter_expr))?;
+ .update(self.preserve_probe_nulls(filter_expr)?)?;
}
}
Ok(())
}
- /// Wraps a pushdown filter so a null-aware anti join keeps its probe-side
NULL rows.
+ /// Keeps probe rows with a NULL key when the join semantics need them.
///
- /// The build-side predicate drops probe rows whose key is NULL, but `NOT
IN` three-valued
- /// logic needs that NULL to reach the join. OR-ing `probe_key IS NULL`
preserves the dynamic
- /// filter's selectivity for non-NULL rows while letting the NULL through.
- fn null_aware_filter(
+ /// The build-side predicate drops probe rows whose key is NULL. A
null-aware anti join
+ /// (`NOT IN`) needs that NULL to reach the join so three-valued logic can
collapse the
+ /// result, and a null-equal join needs it to match a build-side NULL.
OR-ing `key IS NULL`
+ /// keeps those rows while preserving the filter's selectivity for the
rest; the join refines
+ /// whatever the widened filter lets through.
+ fn preserve_probe_nulls(
&self,
filter_expr: Arc<dyn PhysicalExpr>,
- ) -> Arc<dyn PhysicalExpr> {
- if !self.null_aware {
- return filter_expr;
+ ) -> Result<Arc<dyn PhysicalExpr>> {
+ if self.null_equality != NullEquality::NullEqualsNull &&
!self.null_aware {
+ return Ok(filter_expr);
+ }
+ // Only a key that can actually be NULL needs the disjunct; a NOT NULL
key never widens.
+ // Null-aware joins are single-key; null-equal joins can be multi-key,
so OR every nullable
+ // key. If every key is NOT NULL the filter is left untouched, at full
selectivity.
+ let mut any_key_is_null: Option<Arc<dyn PhysicalExpr>> = None;
+ for key in &self.on_right {
Review Comment:
Could we also track dynamically if any build side rows are actually null and
leave the filter unchanged if none of them are null?
##########
datafusion/sqllogictest/test_files/push_down_filter_parquet.slt:
##########
@@ -1050,14 +1050,14 @@ SELECT nej_build.id, nej_probe.id FROM nej_build JOIN
nej_probe ON nej_build.id
11 11
NULL NULL
-# No DynamicFilter predicate may appear on the probe side of a null-equal join
+# The probe side now carries a DynamicFilter for a null-equal join (widened
with IS NULL at runtime)
query TT
EXPLAIN SELECT nej_build.id, nej_probe.id FROM nej_build JOIN nej_probe ON
nej_build.id IS NOT DISTINCT FROM nej_probe.id
Review Comment:
Can we make sure we have some tests that run in partitioned mode? These are
all `CollectLeft`. Making multiple files might work, or you can use `set
target_partitions`
--
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]
