jayzhan211 opened a new pull request, #24593:
URL: https://github.com/apache/datafusion/pull/24593
## Rationale for this change
A sort merge join with a non-equi filter panics (`index out of bounds` in
`get_filter_columns`) when the query does not select a column the filter
references:
```sql
SET datafusion.optimizer.prefer_hash_join = false;
SET datafusion.execution.target_partitions = 2;
CREATE TABLE l (k VARCHAR, a INT, b INT, c INT) AS VALUES ('x', 1, 1, 1),
('y', 2, 2, 2), ('x', 3, 3, 3);
CREATE TABLE r (k VARCHAR, a INT, b INT, c INT) AS VALUES ('x', 1, 1, 1),
('y', 2, 2, 2), ('z', 3, 3, 3);
SELECT l.c, l.k, l.b, r.k, r.b, r.a FROM l RIGHT JOIN r ON l.k = r.k AND l.c
>= r.c;
```
```
thread 'tokio-rt-worker' panicked at
datafusion/physical-plan/src/joins/sort_merge_join/filter.rs:169:34:
index out of bounds: the len is 3 but the index is 3
```
The projection pushdown added in #24517 pushes the projection into the
join's children but passes `self.filter.clone()` through unchanged. The right
child no longer produces `c`, but the filter's `ColumnIndex` still points at it.
## What changes are included in this PR?
`SortMergeJoinExec::try_swapping_with_projection` now uses the shared
`try_pushdown_through_join_with_column_indices` helper, as `HashJoinExec` and
`NestedLoopJoinExec` already do. It remaps both the `on` keys and the filter's
column indices to the projected children, and declines the pushdown — embedding
the projection in the join instead — when the filter needs a column the
projection drops.
One behavioral side effect: projections containing expressions (e.g.
`length(w.p)`) are now also embedded into the join, so the join only emits the
columns they reference. This matches the existing behavior of the other join
operators; the affected `EXPLAIN ANALYZE` expectations in
`sort_merge_join_spill.slt` are updated accordingly (`projection=[...]` on the
`SortMergeJoinExec` line).
## Are these changes tested?
Yes. A regression test in `joins.slt` covers the panicking case (explain +
results). Existing SMJ unit tests, projection tests, proto round-trip tests,
and `joins.slt` / `range_partitioning.slt` / `sort_merge_join_spill.slt` pass.
## Are there any user-facing changes?
No API changes. Plans for `SortMergeJoinExec` under an expression projection
now show `projection=[...]` on the join node.
--
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]
