mdashti commented on code in PR #23106:
URL: https://github.com/apache/datafusion/pull/23106#discussion_r3714939364
##########
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:
Done in 75125ad96. The build report now carries a per-array `null_count`
check, and a NULL-free build skips the widening entirely; the new `EXPLAIN
ANALYZE` in 95d7aee73 shows that filter with no `IS NULL` and the probe NULL
pruned. Two edges: a null-aware anti join keeps the wrap no matter what the
build holds, since one probe NULL collapses `NOT IN` for every build row, and a
canceled partition counts as holding a NULL because its content is unknown.
##########
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:
Added an `EXPLAIN ANALYZE` in 95d7aee73. The populated filter shows the `IS
NULL` disjuncts sitting ahead of the bounds and membership checks.
##########
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:
Added in 95d7aee73. Two parquet files per side plus a zeroed
`hash_join_single_partition_threshold`, so the plan runs real `Hash([id], 4)`
routing with `RepartitionExec` on both sides. The probe NULL still comes
through the partitioned CASE filter.
--
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]
