andygrove opened a new pull request, #2064:
URL: https://github.com/apache/datafusion-ballista/pull/2064
# Which issue does this PR close?
Part of #2046 — this fixes the set-operation half (q38, q87). It does **not**
close the issue: q4 and q78 are a separate root cause and remain skipped.
# Rationale for this change
Under the default (static) planner, Ballista returned wrong results for
TPC-DS
q38 (`INTERSECT`, 0 rows vs 104) and q87 (`EXCEPT`, 47011 vs 46753).
`INTERSECT` / `EXCEPT` lower to semi/anti joins carrying
`NullEquality::NullEqualsNull` so that NULL matches NULL. Ballista sends the
client's *logical* plan to the scheduler as protobuf, and datafusion-proto's
logical `JoinNode` decode rebuilt the join through `LogicalPlanBuilder`,
whose
`join_with_expr_keys` / `join_using` hardcode `NullEqualsNothing`. The
encoded
`null_equality` was therefore silently dropped on decode, and the scheduler
planned a different query than the client asked for: NULL stopped matching
itself, so `INTERSECT` dropped NULL rows and `EXCEPT` retained them — exactly
the reported directions.
Minimal reproduction (SF1 `customer`, which has a NULL `c_last_name`):
```sql
select count(*) from (select distinct c_last_name from customer);
-- 4973
select count(*) from (select distinct c_last_name from customer
intersect
select distinct c_last_name from customer);
-- 4972, expected 4973
select count(*) from (select distinct c_last_name from customer
except
select distinct c_last_name from customer);
-- 1, expected 0
```
`X INTERSECT X` loses exactly the NULL row and `X EXCEPT X` retains exactly
the
NULL row; excluding NULLs makes both correct. This reproduces at
`target_partitions=1` and under both `prefer_hash_join=true` and `false`, so
it
is neither a shuffle nor a join-strategy artifact.
This was fixed upstream in apache/datafusion#22104 and backported to
`branch-54` as apache/datafusion#22785, but it is not in the released
`54.0.0`
tag that this repo pins.
# What changes are included in this PR?
- Pin DataFusion to the `branch-54` tip (`2142d5b2`) via `[patch.crates-io]`
to
pick up the backported fix. This is temporary and should be dropped once a
54.x patch release containing apache/datafusion#22785 is published — happy
to
hold this PR until then if preferred.
- Un-skip TPC-DS q38 and q87 in the correctness gate (86 → 88 queries).
- Add a regression test asserting `null_equality` survives the logical plan
protobuf roundtrip, so a future DataFusion bump cannot silently reintroduce
this.
## Verification
| | Before | After |
|---|---|---|
| Roundtrip test | `NullEqualsNothing` | `NullEqualsNull` |
| `X INTERSECT X` | 4972 | 4973 |
| `X EXCEPT X` | 1 | 0 |
| q38 | 0 vs 104 | verified OK |
| q87 | 47011 vs 46753 | verified OK |
Full SF1 gate: **88/88 verified against single-process DataFusion**, no
mismatches. The 86 queries that already passed still pass, so the DataFusion
pin causes no regression. `cargo test --workspace`, `cargo clippy
--all-targets --workspace -- -D warnings`, and `cargo fmt --all` are clean.
## Still outstanding on #2046
q4 and q78 are unchanged by this fix and stay skipped. Neither uses a set
operation, so the dropped `null_equality` cannot explain them — regular joins
already want `NullEqualsNothing`, so the loss is invisible to them. The
`LIMIT` row-drop (q78: 73 rows vs 100) is a distinct, still-unexplained bug
and
probably deserves its own issue.
# Are there any user-facing changes?
Yes — distributed `INTERSECT` and `EXCEPT` now match single-process
DataFusion
when the compared columns contain NULLs. Previously `INTERSECT` silently
dropped NULL rows and `EXCEPT` silently retained them. No API changes.
--
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]
