jeffw13 opened a new pull request, #25134:
URL: https://github.com/apache/datafusion/pull/25134
## Which issue does this PR close?
Closes #25133.
## Rationale for this change
Sort-merge mark, semi, and anti joins can fail when floating-point key
groups cross input batches. Float32/Float64 keys containing NaN can panic, and
Float16 keys can return an unsupported-type error, even though the main merge
scan supports these keys.
## What changes are included in this PR?
Use the existing `JoinKeyComparator` for batch-boundary comparisons
involving floating-point keys, matching the main merge scan's equality
semantics. Slice the right-hand keys to one row before constructing the
comparator; the left-hand keys are already single-row slices. This bounds
signed-zero normalization to the rows being compared.
Integer-only comparisons keep the existing scalar path. There are no
memory-accounting, buffering, configuration, dependency, or public-API changes.
## What is the testing strategy for this PR?
`join_float_key_batch_boundaries` in
`datafusion/physical-plan/src/joins/sort_merge_join/tests.rs` exercises actual
one- and two-row input batches. Its 144 cases cover Float16/32/64, left/right
mark/semi/anti joins, both null-equality modes, and ascending/descending order.
Inputs include nulls, infinities, signed zero, unmatched keys, and NaN groups
spanning batches. Assertions check exact output row IDs/order, key types, and
mark values.
A physical-plan test is used because the reproducer needs precise input
batch boundaries, independent of SQL optimizer choices.
On unpatched Apache main (`40488988ad596c9b093ad60e1453430d803ce33c`) plus
only the new tests, all 144 cases fail: 96 NaN panics and 48 Float16
unsupported-type errors.
With the fix, `cargo test --locked --profile ci -p datafusion-physical-plan
--lib` passes all 2,096 tests, including the 144 new cases. `cargo fmt --all --
--check`, `git diff --check`, and `cargo clippy --locked --all-targets
--all-features -- -D warnings` also pass.
The initial extended workspace run stopped in `core_integration` with 1,152
passed, 4 failed, and 1 ignored. The failures were the
`sort_no_mem_limit_runner` and `sort_with_mem_limit_{1,2,3}_runner` RSS-limit
tests. They execute a sort-only `generate_series ... ORDER BY` query, outside
the changed join path, and reported a zero baseline RSS. All four runners then
passed an isolated, serial rerun of the same test binary without any code/test
changes. The cause of the initial failures has not been established; a complete
workspace rerun is still pending.
The complete lint suite, remaining broad validation, and relevant existing
benchmark comparison against the same upstream base are pending. This PR is
intentionally a draft; the full workspace is not claimed green. No performance
improvement is claimed.
## Are there any user-facing changes?
Affected sort-merge joins can complete instead of failing at floating-point
batch boundaries. No public API changes.
AI-assisted contribution (OpenAI Codex).
--
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]
