Skip to content

chore: Upgrade to datafusion 55 - #5634

Merged
mdashti merged 44 commits into
mainfrom
rj.use-datafusion-55
Jul 28, 2026
Merged

chore: Upgrade to datafusion 55#5634
mdashti merged 44 commits into
mainfrom
rj.use-datafusion-55

Conversation

@barbarj

@barbarj barbarj commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Ticket(s) Closed

What

Updates to pre-release DataFusion 55 and a DataFusion Distributed rev based on DF 55.

Why

DataFusion 55 has range partitioning functionality we need for future work.

How

Mostly update cargo.toml and fix compile errors.

There is one additional wrinkle: In DF 55, HashJoinExec changes its behavior to fix a bug (See this). That change assumes that installed dynamic filters remain installed between filter pushdown passes. This PR includes a change to make our handling of filter pushdown compatible by retaining filters between passes and de-duping by expression id. There is a lingering issue (#5635) from that, to be addressed later.

Tests

  • All unit and integration tests pass
  • Regression test output updates fall into 4 categories:
    • Join order or type changes do to planning changes between DF 54 & 55.
    • Cosmetic changes in the printed node info for HashJoinExec
    • +1 increases in dynamic filter counts. This is fine. It's due to the issue mentioned above and will be fixed separately. The orphaned filter is just lit(true), so there are no correctness issues.
    • The addition of a null_aware to some lines.:

barbarj
barbarj commented Jul 20, 2026
View reviewed changes
Comment thread pg_search/Cargo.toml Resolved
@barbarj barbarj added the Do Not Cherry Pick PR should not be cherry-picked to other branches label Jul 20, 2026
@barbarj
barbarj requested review from mdashti and stuhood July 20, 2026 21:48
@barbarj
barbarj marked this pull request as ready for review July 20, 2026 21:48
@barbarj
barbarj requested a review from a team as a code owner July 20, 2026 21:48
@stuhood stuhood added the benchmark-stackoverflow Add this label to a PR to run the StackOverflow benchmark. Automatically removed. label Jul 21, 2026
@stuhood stuhood removed the benchmark-stackoverflow Add this label to a PR to run the StackOverflow benchmark. Automatically removed. label Jul 21, 2026
mdashti
mdashti reviewed Jul 21, 2026
View reviewed changes

@mdashti mdashti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Two things I'd want resolved:

issue_4779 loses MPP. On c36e83a8 regress came back passed=299 failed=1. The expected plan has DistributedExec with Stage 1 ── tasks=3, partitions=9 and a NetworkCoalesceExec. This branch produces a flat single-process plan with no DistributedExec at all.

MPP dispatch may quietly lose its probe-side dynamic filters. DF 55 round-trips hashjoin.dynamic_filter (and the sort / aggregate ones) through proto. So on a worker the decoded HashJoinExec arrives with dynamic_filter already set, and the is_none() gate above makes gather_filters_for_pushdown skip pushing. PgSearchScanPlan::encode_for_dispatch doesn't ship its filters, and PhysicalPlanDecodeContext has no expression_id registry, so each decode creates its own Inner: the join's copy and the scan's copy end up separate instances with the same id. That would leave the relink pass in deserialize_physical_plan_with_runtime a no-op and the worker's scans unfiltered. EXPLAIN prints the leader's plan, so the mpp_* regress files wouldn't show it either way. Did you check that path? The same DF change also makes the "Once DataFusion round-trips the filters natively, this pass can go" note there stale.

Nothing tests the merge directly. Nothing drives handle_child_pushdown_result twice. So the only signal for a regression is the dynamic_filters=N count in the regress plans, and this PR (knowingly) makes that count wrong. A unit test that delivers the same filter on two passes and asserts the set stays at one would be great.

Comment thread pg_search/Cargo.toml Resolved
Comment thread pg_search/Cargo.toml Resolved
Comment thread pg_search/Cargo.toml Resolved
Comment thread pg_search/src/scan/execution_plan.rs Resolved
Comment thread pg_search/src/scan/execution_plan.rs Resolved
Comment thread pg_search/src/scan/physical_codec.rs Resolved
Comment thread pg_search/src/scan/execution_plan.rs Resolved
@barbarj barbarj added benchmark-stackoverflow Add this label to a PR to run the StackOverflow benchmark. Automatically removed. and removed benchmark-stackoverflow Add this label to a PR to run the StackOverflow benchmark. Automatically removed. labels Jul 24, 2026
@barbarj

barbarj commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

issue_4779 loses MPP. On c36e83a regress came back passed=299 failed=1. The expected plan has DistributedExec with Stage 1 ── tasks=3, partitions=9 and a NetworkCoalesceExec. This branch produces a flat single-process plan with no DistributedExec at all.

Stu's work in #5657 caused the same change, and we now know this is desirable.

MPP dispatch may quietly lose its probe-side dynamic filters. DF 55 round-trips hashjoin.dynamic_filter (and the sort / aggregate ones) through proto.....

I've added the correct proto converters, so this should be in a good place now.

Nothing tests the merge directly. Nothing drives handle_child_pushdown_result twice. So the only signal for a regression is the dynamic_filters=N count in the regress plans, and this PR (knowingly) makes that count wrong. A unit test that delivers the same filter on two passes and asserts the set stays at one would be great.

I still need to address this. Ideally, the test would be able to examine the expression ids of the pushed-down filters between runs to assert the set is equivalent.

barbarj
barbarj commented Jul 24, 2026
View reviewed changes
Comment thread pg_search/src/scan/physical_codec.rs Resolved
paradedb-github-bot Bot and others added 8 commits July 24, 2026 22:54
@paradedb-github-bot
fix: update nix cargoHash for changed Cargo.lock
1a78019
@barbarj
bump df-d ref
2ea760e
@paradedb-github-bot
fix: update nix cargoHash for changed Cargo.lock
9c2427a
@mdashti
Fixed the inverted dynamic filter dedup guard.
04ac6ae 
A remapped wrapper shares its expression_id but carries a fresh pointer. The guard replaced only on pointer equality, so it dropped the remap that reflects the current tree shape and the scan kept stale column indices. Replace when the pointer differs.
@mdashti
Merge remote-tracking branch 'origin/main' into rj.use-datafusion-55
f29e292 
# Conflicts:
#	Cargo.lock
#	nix/pg_search.nix
@paradedb-github-bot
fix: update nix cargoHash for changed Cargo.lock
b70d4d0
@mdashti
Pinned datafusion-distributed to the merged main head.
94bd6c2 
The fork upgrade merged to `main`, so pin its head rev instead of the mutable PR branch ref.
@paradedb-github-bot
fix: update nix cargoHash for changed Cargo.lock
b909402
mdashti
mdashti approved these changes Jul 27, 2026
View reviewed changes
Comment thread pg_search/src/scan/execution_plan.rs Resolved
Comment thread pg_search/src/scan/execution_plan.rs Resolved
Comment thread pg_search/src/scan/physical_codec.rs Resolved
mdashti and others added 4 commits July 27, 2026 15:25
@mdashti
Promoted joinscan-subquery-parallel-rti for a DF 55 partition count.
c1e4fc5 
The df-d upgrade lowers the anti-join probe scan to one input partition. The `RoundRobinBatch(3)` above it still fans to 3, so downstream parallelism and results are unchanged.
@mdashti
Merge remote-tracking branch 'origin/main' into rj.use-datafusion-55
fdc4d87 
# Conflicts:
#	nix/pg_search.nix
@paradedb-github-bot
fix: update nix cargoHash for changed Cargo.lock
a2ecf7e
@mdashti
Pinned datafusion-distributed to the snapshot tag.
6938781 
Track the immutable `snapshot-main-*` tag instead of a bare commit rev, matching how the fork is pinned on `main`.
stuhood
stuhood approved these changes Jul 27, 2026
View reviewed changes

@stuhood stuhood left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@mdashti
mdashti merged commit fb90b2b into main Jul 28, 2026
22 checks passed
@mdashti
mdashti deleted the rj.use-datafusion-55 branch July 28, 2026 00:15
@devdattatalele devdattatalele mentioned this pull request Jul 28, 2026
Merged
@barbarj barbarj mentioned this pull request Jul 30, 2026
Merged
stuhood pushed a commit that referenced this pull request Jul 30, 2026
@barbarj
fix: Encode/decode dynamic filters during dispatch encode/decode so t…
924c920 
…hat dynamic filters survive across fragments (#5766)

## What
This PR fixes the performance regression show in the `join_semi_filter`
alternatives 1 and 4 benchmark that was introduced by #5634.

## How
During dispatch encoding and decoding, use the provided proto converter
to encode/decode the dynamic filters.

## Tests
The benchmark run in this PR shows the 50% perf improvement for the 2
`join_semi_filter` benchmarks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stuhood stuhood stuhood approved these changes
@mdashti mdashti mdashti approved these changes

Assignees

No one assigned

Labels

Do Not Cherry Pick PR should not be cherry-picked to other branches

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update to a datafusion-55 prerelease

3 participants

@barbarj @stuhood @mdashti