Skip to content

Coalesce batches inside hash join, reuse indices buffer #18972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Coalesce batches inside hash join, reuse indices buffer #18972

wants to merge 15 commits into from
+1,399 −1,493

Conversation

@Dandandan
Copy link
Contributor

@Dandandan Dandandan commented Nov 27, 2025 •
edited
Loading

Which issue does this PR close?

Rationale for this change

Improves the plan / plan readability for queries with joins.

Seems to improve perf as well a bit for more challenging joins: (TPC-H SF=10, in-memory)

│ QQuery 21    │              7948.54 ms │    6340.74 ms │ +1.25x faster │
│ QQuery 22    │                58.30 ms │      49.86 ms │ +1.17x faster │

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Nov 27, 2025
@Dandandan Dandandan changed the title Refactor join2 Coalesce batches inside hash join Nov 27, 2025
@github-actions github-actions bot added the core Core DataFusion crate label Nov 27, 2025
@Dandandan Dandandan changed the title Coalesce batches inside hash join Coalesce batches inside hash join, reuse indices buffer Nov 28, 2025
@Dandandan Dandandan mentioned this pull request Nov 28, 2025
Open
@Dandandan
Copy link
Contributor Author

Dandandan commented Nov 28, 2025

@alamb let's fire some benchmarks?
I mostly see improvement on SF=10 in memory on the standard set of benchmarks

@Dandandan Dandandan mentioned this pull request Nov 28, 2025
Open
martin-g
martin-g reviewed Nov 28, 2025
View reviewed changes
Dandandan
Dandandan commented Nov 28, 2025
View reviewed changes
datafusion/physical-optimizer/src/coalesce_batches.rs
plan.transform_up(|plan| {
let plan_any = plan.as_any();
let wrap_in_coalesce = plan_any.downcast_ref::<HashJoinExec>().is_some()
let wrap_in_coalesce = plan_any
Copy link
Contributor Author

@Dandandan Dandandan Nov 28, 2025

Choose a reason for hiding this comment

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

The list is getting smaller and smaller

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@martin-g martin-g martin-g left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

core Core DataFusion crate optimizer Optimizer rules physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Integrate BatchCoalescer into HashJoinExec and remove from CoalesceBatches optimization rule

2 participants

@Dandandan @martin-g