Skip to content

fix: Enable more TPC-DS queries - #2064

Merged
andygrove merged 2 commits into
apache:mainfrom
andygrove:investigate/2046-distributed-setop-limit
Jul 22, 2026
Merged

fix: Enable more TPC-DS queries#2064
andygrove merged 2 commits into
apache:mainfrom
andygrove:investigate/2046-distributed-setop-limit

Conversation

@andygrove

@andygrove andygrove commented Jul 16, 2026

Copy link
Copy Markdown
Member

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.

The bug fix is actually in DataFusion 54.1.0. This PR just enables queries that were previously failing, and adds regression tests.

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):

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 DataFusion 54.1.0

What changes are included in this PR?

  • 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.

Relationship to the other half of #2046

q4 and q78 are unaffected by this fix and have a different root cause: broadcast
promotion dropping a sort order required by a parent SortMergeJoinExec
(#2065), fixed separately in #2066. 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.

Together, #2066 and this PR close out #2046 and take the TPC-DS correctness gate
to 90 queries. The two PRs both edit the SKIP list in
benchmarks/src/bin/tpcds.rs, so whichever lands second needs a trivial rebase.

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.

`INTERSECT` and `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 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.

This reproduces with a single partition, so it is not a shuffle or
join-strategy artifact, and it affects both hash and sort-merge joins:

    select count(*) from (select distinct c_last_name from customer
                          intersect
                          select distinct c_last_name from customer);
    -- 4972, expected 4973

Fixed upstream by apache/datafusion#22104 and backported to `branch-54` as
apache/datafusion#22785, which is not in the released 54.0.0 tag. Pin
DataFusion to the `branch-54` tip to pick it up; the pin can be dropped once a
54.x patch release containing it is published.

TPC-DS q38 (INTERSECT) and q87 (EXCEPT) now match single-process DataFusion and
join the correctness gate, taking it from 86 to 88 queries. q4 and q78 remain
skipped: neither uses a set operation and both are unaffected by this fix, so
they have a separate root cause.
@andygrove andygrove changed the title fix: preserve null_equality so distributed INTERSECT/EXCEPT match NULLs fix: TPC-DS correctness issues Jul 16, 2026
@andygrove andygrove changed the title fix: TPC-DS correctness issues fix: TPC-DS EmptyExec failure Jul 16, 2026
@andygrove andygrove changed the title fix: TPC-DS EmptyExec failure fix: TPC-DS failures and upgrade to DataFusion 54.0.1 [WIP] Jul 16, 2026
@andygrove andygrove changed the title fix: TPC-DS failures and upgrade to DataFusion 54.0.1 [WIP] fix: TPC-DS failures and upgrade to DataFusion 54.1.0 [WIP] Jul 16, 2026
@andygrove andygrove changed the title fix: TPC-DS failures and upgrade to DataFusion 54.1.0 [WIP] fix: TPC-DS failures Jul 22, 2026
@andygrove andygrove changed the title fix: TPC-DS failures fix: Enable more TPC-DS queries Jul 22, 2026
Resolves three conflicts:

* `Cargo.toml` / `Cargo.lock` — drop the `[patch.crates-io]` DataFusion git
  pin. It existed only to pick up apache/datafusion#22785 (logical `JoinNode`
  proto round-trip preserving `null_equality`), which was on `branch-54` but
  not in the released 54.0.0 tag. main is now on released DataFusion 54.1.0
  (apache#2126), which contains the fix, so the pin retires as its own comment
  instructed. Verified: `logical_join_roundtrip_preserves_null_equality`
  passes against datafusion-proto 54.1.0 from crates.io with no patch block.

* `ballista/core/src/serde/mod.rs` — both sides appended tests at the same
  point in the `test` module. Kept both: this branch's
  `logical_join_roundtrip_preserves_null_equality` plus main's
  `RuntimeStatsExec` / `BufferExec` codec round-trip tests.

* `benchmarks/src/bin/tpcds.rs` — both sides removed different entries from
  the same `SKIP` group. This branch un-skipped q38/q87 (fixed by the
  `null_equality` change); main un-skipped q4/q78 (fixed by apache#2072, which
  removed the incorrect SortMergeJoinExec broadcast conversion). Took the
  union, so all four are now in the correctness gate and the `apache#2046` group
  and its comment are gone.
@andygrove
andygrove force-pushed the investigate/2046-distributed-setop-limit branch from 3aa6754 to e88aeb8 Compare July 22, 2026 13:23
@andygrove
andygrove marked this pull request as ready for review July 22, 2026 13:25
@andygrove
andygrove requested a review from avantgardnerio July 22, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants