Skip to content

Commit 8248a57

Browse files
fix: allow NULL-padded rows in sort-merge join filter evaluation (#26027)
## Which issue does this PR close? - Closes #26001. ## Rationale for this change With sort-merge join (`datafusion.optimizer.prefer_hash_join = false`), a `LEFT`, `RIGHT` or `FULL` join with an extra join filter fails with `Arrow error: Invalid argument error: Column '...' is declared as non-nullable but contains null values` when the NULL-padded side has `NOT NULL` columns referenced by the filter. Hash join returns the correct result for the same queries. ## What changes are included in this PR? **Root cause.** As @jayzhan211 pointed out in the issue, when a streamed row has no match but the buffered side still holds a batch, `null_join_streamed_row` appends it with `Some(scanning_batch_idx)`, so it is materialized in the same chunk as matched rows. Its NULL-padded buffered-side columns then reach join filter evaluation, but the filter's intermediate schema declares them non-nullable, so `RecordBatch::try_new` fails. **Fix.** Following the fix suggested in the issue, `MaterializingSortMergeJoinStream::try_new` builds the filter schema once with buffered-side fields marked nullable (`JoinSide::Right` for `Left`/`Full`, `JoinSide::Left` for `Right`), and filter evaluation uses it. It is only built when `deferred_filtering` is set (outer join with a filter); inner joins keep the original schema. **Why this is safe.** The filter result for a NULL-padded row does not affect the output. Such a row is the only entry for its streamed row, so `get_corrected_filter_mask` keeps it as-is if the filter returns true and null-joins it if false; either way the same NULL-padded row is emitted. The `Full` join filter-status tracking already skips NULL buffered indices, so this path already expects such rows. **Alternative considered.** Keep NULL-padded rows out of the filter by appending them with `buffered_batch_idx = None`. Done naively, this reorders output: `freeze_streamed` pushes `None` chunks immediately but pushes matched chunks together at the end, so sort order would be silently broken. Flushing matched chunks before each `None` chunk would preserve order, but splits the single batched filter evaluation in `freeze_streamed_matched`, which matters most when keys are near-unique. Making the schema nullable avoids both. ## What is the testing strategy for this PR? Added sqllogictest cases to `sort_merge_join.slt` with `NOT NULL` columns and `target_partitions = 2` (needed to reproduce; the queries pass with 1 partition): - `LEFT`, `FULL` and `RIGHT` joins from the issue, with expected results matching hash join. - A `LEFT` join whose filter divides by a buffered-side column (`a.v / b.w < 1`), to check that the zero-valued slots under NULL-padded rows do not cause a divide-by-zero error. All four queries fail on `main` with the Arrow error above and pass with this change. The full extended test suite also passes locally. ## Are there any user-facing changes? No. Bug fix only; no public API change.
1 parent 5741773 commit 8248a57

2 files changed

Lines changed: 92 additions & 4 deletions

File tree

‎datafusion/physical-plan/src/joins/sort_merge_join/materializing_stream.rs‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,11 @@ use arrow::compute::{
4545
self, BatchCoalescer, SortOptions, concat_batches, filter_record_batch, interleave,
4646
take_arrays,
4747
};
48-
use arrow::datatypes::SchemaRef;
48+
use arrow::datatypes::{Schema, SchemaRef};
4949
use datafusion_common::cast::as_uint64_array;
5050
use datafusion_common::instant::Instant;
5151
use datafusion_common::{
52-
DataFusionError, JoinType, NullEquality, Result, exec_err, internal_err,
52+
DataFusionError, JoinSide, JoinType, NullEquality, Result, exec_err, internal_err,
5353
};
5454
use datafusion_execution::memory_pool::MemoryReservation;
5555
use datafusion_execution::runtime_env::RuntimeEnv;
@@ -295,6 +295,10 @@ pub(super) struct MaterializingSortMergeJoinStream {
295295
/// Cached `needs_deferred_filtering(filter, join_type)` — both inputs
296296
/// are fixed at construction time.
297297
pub deferred_filtering: bool,
298+
/// The join filter's schema with buffered-side fields marked nullable,
299+
/// since NULL-padded rows reach filter evaluation. `None` unless
300+
/// `deferred_filtering`.
301+
pub filter_schema: Option<SchemaRef>,
298302
/// Target output batch size
299303
pub batch_size: usize,
300304

@@ -551,6 +555,30 @@ impl MaterializingSortMergeJoinStream {
551555
semi/anti/mark joins use BitwiseSortMergeJoinStream"
552556
);
553557
let join_time = join_metrics.join_time();
558+
let buffered_side = if join_type == JoinType::Right {
559+
JoinSide::Left
560+
} else {
561+
JoinSide::Right
562+
};
563+
let deferred_filtering = needs_deferred_filtering(filter.as_ref(), join_type);
564+
let filter_schema = if deferred_filtering && let Some(f) = &filter {
565+
let mut fields = Vec::with_capacity(f.schema().fields().len());
566+
for (column_index, field) in
567+
f.column_indices().iter().zip(f.schema().fields().iter())
568+
{
569+
if column_index.side == buffered_side {
570+
fields.push(Arc::new(field.as_ref().clone().with_nullable(true)));
571+
} else {
572+
fields.push(Arc::clone(field));
573+
}
574+
}
575+
Some(Arc::new(Schema::new_with_metadata(
576+
fields,
577+
f.schema().metadata().clone(),
578+
)))
579+
} else {
580+
None
581+
};
554582
let mut this = Self {
555583
sort_options,
556584
null_equality,
@@ -566,7 +594,8 @@ impl MaterializingSortMergeJoinStream {
566594
buffered_exhausted: false,
567595
on_streamed,
568596
on_buffered,
569-
deferred_filtering: needs_deferred_filtering(filter.as_ref(), join_type),
597+
deferred_filtering,
598+
filter_schema,
570599
filter,
571600
joined_record_batches: JoinedRecordBatches {
572601
joined_batches: new_output_coalescer(Arc::clone(&schema), batch_size),
@@ -1601,8 +1630,9 @@ impl MaterializingSortMergeJoinStream {
16011630

16021631
if !filter_columns.is_empty() {
16031632
if let Some(f) = &self.filter {
1633+
let schema = self.filter_schema.as_ref().unwrap_or_else(|| f.schema());
16041634
let filter_batch =
1605-
RecordBatch::try_new(Arc::clone(f.schema()), filter_columns)?;
1635+
RecordBatch::try_new(Arc::clone(schema), filter_columns)?;
16061636
let filter_result = f
16071637
.expression()
16081638
.evaluate(&filter_batch)?

‎datafusion/sqllogictest/test_files/sort_merge_join.slt‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -971,6 +971,64 @@ WHERE t1_sorted.data < 0
971971
----
972972
100
973973

974+
# Regression test for #26001: outer SMJ with a join filter must not fail
975+
# when the NULL-padded side has NOT NULL columns. target_partitions = 2
976+
# is needed to reproduce; with 1 partition the bug does not trigger.
977+
978+
statement ok
979+
set datafusion.execution.target_partitions = 2;
980+
981+
statement ok
982+
CREATE TABLE a (k INT NOT NULL, v INT NOT NULL) AS VALUES (1, 10), (2, 20), (3, 30);
983+
984+
statement ok
985+
CREATE TABLE b (k INT NOT NULL, w INT NOT NULL) AS VALUES (1, 100), (3, 300);
986+
987+
statement ok
988+
CREATE TABLE c (k INT NOT NULL, w INT NOT NULL) AS VALUES (1, 100), (2, 200), (3, 300);
989+
990+
query IIII
991+
SELECT * FROM a LEFT JOIN b ON a.k = b.k AND a.v < b.w ORDER BY a.k
992+
----
993+
1 10 1 100
994+
2 20 NULL NULL
995+
3 30 3 300
996+
997+
query IIII
998+
SELECT * FROM a FULL JOIN b ON a.k = b.k AND a.v < b.w ORDER BY a.k
999+
----
1000+
1 10 1 100
1001+
2 20 NULL NULL
1002+
3 30 3 300
1003+
1004+
query IIII
1005+
SELECT * FROM b RIGHT JOIN c ON b.k = c.k AND b.w <= c.w ORDER BY c.k
1006+
----
1007+
1 100 1 100
1008+
NULL NULL 2 200
1009+
3 300 3 300
1010+
1011+
# NULL-padded rows reach the filter; their zero-valued slots must not
1012+
# cause divide-by-zero errors
1013+
query IIII
1014+
SELECT * FROM a LEFT JOIN b ON a.k = b.k AND a.v / b.w < 1 ORDER BY a.k
1015+
----
1016+
1 10 1 100
1017+
2 20 NULL NULL
1018+
3 30 3 300
1019+
1020+
statement ok
1021+
DROP TABLE a;
1022+
1023+
statement ok
1024+
DROP TABLE b;
1025+
1026+
statement ok
1027+
DROP TABLE c;
1028+
1029+
statement ok
1030+
set datafusion.execution.target_partitions = 4;
1031+
9741032
# Config reset
9751033
statement ok
9761034
RESET datafusion.optimizer.prefer_hash_join;

0 commit comments

Comments
 (0)