Skip to content

Allow Range partitioned inputs to PartitionedTopK - #23355

Merged
gabotechs merged 4 commits into
apache:mainfrom
paradedb:stuhood.range-partitioned-top-k
Jul 15, 2026
Merged

Allow Range partitioned inputs to PartitionedTopK#23355
gabotechs merged 4 commits into
apache:mainfrom
paradedb:stuhood.range-partitioned-top-k

Conversation

@stuhood

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

What changes are included in this PR?

In two commits:

  • Improve the robustness of WindowTopN to intermediate RepartitionExec nodes, to ensure that PartitionedTopK can still be used when target_partitions mismatches physical partitions.
  • Use required_input_distributions to declare required partitioning for PartitionedTopKExec, BoundedWindowAggExec, and WindowAggExec.

Are these changes tested?

Yes, new slt tests are added for both the WindowTopN robustness fix, and for the use of required_input_distributions in PartitionedTopK.

Are there any user-facing changes?

No: only plan changes.

@github-actionsgithub-actionsBot added physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Jul 6, 2026
@stuhoodstuhood changed the title Stuhood.range partitioned top kAllow Range partitioned inputs to PartitionedTopKJul 6, 2026
@github-actionsgithub-actionsBot added the auto detected api change Auto detected API change label Jul 6, 2026
@stuhood
stuhoodforce-pushed the stuhood.range-partitioned-top-k branch from 1b1c490 to e236f7bCompareJuly 8, 2026 19:30
@github-actionsgithub-actionsBot added the datasource Changes to the datasource crate label Jul 8, 2026
@stuhood
stuhoodforce-pushed the stuhood.range-partitioned-top-k branch 3 times, most recently from eb2c841 to 3ab476dCompareJuly 8, 2026 23:10
@gene-bordegaray

Copy link
Copy Markdown
Contributor

This is in my queue, just waiting for #23184 to land 🙇

@stuhood
stuhoodforce-pushed the stuhood.range-partitioned-top-k branch from 3ab476d to 7d0f380CompareJuly 10, 2026 18:10
@github-actionsgithub-actionsBot removed physical-expr Changes to the physical-expr crates core Core DataFusion crate datasource Changes to the datasource crate auto detected api change Auto detected API change labels Jul 10, 2026
@stuhood
stuhood marked this pull request as ready for review July 10, 2026 20:55

@gene-bordegaraygene-bordegaray 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.

Looks goodl, siilar to #23416 lets keep the testing set in slt leaner and not stress the full preserve_file_partitions and subset_repartition_threshold matrix. I will make a PR to have this matrix tested in the unit 👍

I think for slt tests keeping it to a simple positive / negative, subset positive / negative is good, lmk what you think 🙇

Thank you

Comment threaddatafusion/physical-plan/src/windows/window_agg_exec.rs Outdated
Comment threaddatafusion/sqllogictest/test_files/range_partitioning.slt Outdated
Comment threaddatafusion/sqllogictest/test_files/range_partitioning.slt Outdated
Comment threaddatafusion/sqllogictest/test_files/range_partitioning.slt Outdated
Comment threaddatafusion/sqllogictest/test_files/range_partitioning.slt Outdated
Comment threaddatafusion/sqllogictest/test_files/range_partitioning.slt Outdated
@stuhood
stuhoodforce-pushed the stuhood.range-partitioned-top-k branch from 7d0f380 to ab90d5cCompareJuly 13, 2026 21:04
@github-actionsgithub-actionsBot added the core Core DataFusion crate label Jul 13, 2026
@stuhood

Copy link
Copy Markdown
ContributorAuthor

Thanks @gene-bordegaray : have rebased atop #23416 (only the top 3 commits are relevant) and applied your feedback. I'll put this back in draft until #23416 has landed.

@stuhood
stuhoodforce-pushed the stuhood.range-partitioned-top-k branch from ab90d5c to c92419eCompareJuly 14, 2026 19:25
stuhood pushed a commit to paradedb/datafusion that referenced this pull request Jul 14, 2026
…rtitioned requirements (apache#23416)
## Which issue does this PR close?
- Closesapache#23289.
Rebased on main now that apache#23184 has merged. Overlaps with apache#23355, which
also opts the window execs into range satisfaction as part of its
PartitionedTopK work — happy to rebase whichever lands second.
## What changes are included in this PR?
In one commit:
* Opt `WindowAggExec` and `BoundedWindowAggExec` with partition keys
into range satisfaction via
`InputDistributionRequirements::allow_range_satisfaction_for_key_partitioning`,
mirroring `AggregateExec`. Compatible range-partitioned inputs then
satisfy the window key requirement without a hash repartition; subset
satisfaction and the hash fallback for incompatible keys come from the
existing satisfaction machinery. Windows without `PARTITION BY` keep
requiring a single partition.
## Are these changes tested?
Yes: new `slt` tests in `range_partitioning.slt` (exact and subset
reuse, rehash on incompatible keys, `subset_repartition_threshold` /
`preserve_file_partitions` / `target_partitions` behavior,
`WindowAggExec` via an unbounded frame, and no-`PARTITION BY`), plus
plan-shape tests in `enforce_distribution.rs` and acceptance/rejection
tests in `sanity_checker.rs`.
## Are there any user-facing changes?
No: only plan changes.
@github-actionsgithub-actionsBot removed the core Core DataFusion crate label Jul 14, 2026
@stuhood
stuhood marked this pull request as ready for review July 14, 2026 19:25
@stuhood

Copy link
Copy Markdown
ContributorAuthor

@gene-bordegaray: Ready for another look! Thanks.

@gene-bordegaraygene-bordegaray 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.

Great, thanks. Just some small nits 👍

cc: @gabotechs

Comment threaddatafusion/sqllogictest/test_files/range_partitioning.slt Outdated
Comment threaddatafusion/physical-optimizer/src/window_topn.rs
Comment threaddatafusion/sqllogictest/test_files/range_partitioning.slt Outdated

@gabotechsgabotechs 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.

@gabotechs
gabotechs added this pull request to the merge queueJul 15, 2026
Merged via the queue into apache:main with commit e123e6fJul 15, 2026
40 checks passed
gene-bordegaray pushed a commit to gene-bordegaray/datafusion that referenced this pull request Jul 24, 2026
…rtitioned requirements (apache#23416)
## Which issue does this PR close?
- Closesapache#23289.
Rebased on main now that apache#23184 has merged. Overlaps with apache#23355, which
also opts the window execs into range satisfaction as part of its
PartitionedTopK work — happy to rebase whichever lands second.
## What changes are included in this PR?
In one commit:
* Opt `WindowAggExec` and `BoundedWindowAggExec` with partition keys
into range satisfaction via
`InputDistributionRequirements::allow_range_satisfaction_for_key_partitioning`,
mirroring `AggregateExec`. Compatible range-partitioned inputs then
satisfy the window key requirement without a hash repartition; subset
satisfaction and the hash fallback for incompatible keys come from the
existing satisfaction machinery. Windows without `PARTITION BY` keep
requiring a single partition.
## Are these changes tested?
Yes: new `slt` tests in `range_partitioning.slt` (exact and subset
reuse, rehash on incompatible keys, `subset_repartition_threshold` /
`preserve_file_partitions` / `target_partitions` behavior,
`WindowAggExec` via an unbounded frame, and no-`PARTITION BY`), plus
plan-shape tests in `enforce_distribution.rs` and acceptance/rejection
tests in `sanity_checker.rs`.
## Are there any user-facing changes?
No: only plan changes.
gene-bordegaray pushed a commit to gene-bordegaray/datafusion that referenced this pull request Jul 24, 2026
- Closesapache#23290.
In two commits:
* Improve the robustness of `WindowTopN` to intermediate
`RepartitionExec` nodes, to ensure that `PartitionedTopK` can still be
used when `target_partitions` mismatches physical partitions.
* Use `required_input_distributions` to declare required partitioning
for `PartitionedTopKExec`, `BoundedWindowAggExec`, and `WindowAggExec`.
Yes, new `slt` tests are added for both the `WindowTopN` robustness fix,
and for the use of `required_input_distributions` in `PartitionedTopK`.
No: only plan changes.
gene-bordegaray pushed a commit to DataDog/datafusion that referenced this pull request Aug 7, 2026
…rtitioned requirements (apache#23416)
## Which issue does this PR close?
- Closesapache#23289.
Rebased on main now that apache#23184 has merged. Overlaps with apache#23355, which
also opts the window execs into range satisfaction as part of its
PartitionedTopK work — happy to rebase whichever lands second.
## What changes are included in this PR?
In one commit:
* Opt `WindowAggExec` and `BoundedWindowAggExec` with partition keys
into range satisfaction via
`InputDistributionRequirements::allow_range_satisfaction_for_key_partitioning`,
mirroring `AggregateExec`. Compatible range-partitioned inputs then
satisfy the window key requirement without a hash repartition; subset
satisfaction and the hash fallback for incompatible keys come from the
existing satisfaction machinery. Windows without `PARTITION BY` keep
requiring a single partition.
## Are these changes tested?
Yes: new `slt` tests in `range_partitioning.slt` (exact and subset
reuse, rehash on incompatible keys, `subset_repartition_threshold` /
`preserve_file_partitions` / `target_partitions` behavior,
`WindowAggExec` via an unbounded frame, and no-`PARTITION BY`), plus
plan-shape tests in `enforce_distribution.rs` and acceptance/rejection
tests in `sanity_checker.rs`.
## Are there any user-facing changes?
No: only plan changes.
gene-bordegaray pushed a commit to DataDog/datafusion that referenced this pull request Aug 7, 2026
- Closesapache#23290.
In two commits:
* Improve the robustness of `WindowTopN` to intermediate
`RepartitionExec` nodes, to ensure that `PartitionedTopK` can still be
used when `target_partitions` mismatches physical partitions.
* Use `required_input_distributions` to declare required partitioning
for `PartitionedTopKExec`, `BoundedWindowAggExec`, and `WindowAggExec`.
Yes, new `slt` tests are added for both the `WindowTopN` robustness fix,
and for the use of `required_input_distributions` in `PartitionedTopK`.
No: only plan changes.
kosiew pushed a commit to kosiew/datafusion that referenced this pull request Aug 12, 2026
…rtitioned requirements (apache#23416)
## Which issue does this PR close?
- Closesapache#23289.
Rebased on main now that apache#23184 has merged. Overlaps with apache#23355, which
also opts the window execs into range satisfaction as part of its
PartitionedTopK work — happy to rebase whichever lands second.
## What changes are included in this PR?
In one commit:
* Opt `WindowAggExec` and `BoundedWindowAggExec` with partition keys
into range satisfaction via
`InputDistributionRequirements::allow_range_satisfaction_for_key_partitioning`,
mirroring `AggregateExec`. Compatible range-partitioned inputs then
satisfy the window key requirement without a hash repartition; subset
satisfaction and the hash fallback for incompatible keys come from the
existing satisfaction machinery. Windows without `PARTITION BY` keep
requiring a single partition.
## Are these changes tested?
Yes: new `slt` tests in `range_partitioning.slt` (exact and subset
reuse, rehash on incompatible keys, `subset_repartition_threshold` /
`preserve_file_partitions` / `target_partitions` behavior,
`WindowAggExec` via an unbounded frame, and no-`PARTITION BY`), plus
plan-shape tests in `enforce_distribution.rs` and acceptance/rejection
tests in `sanity_checker.rs`.
## Are there any user-facing changes?
No: only plan changes.
kosiew pushed a commit to kosiew/datafusion that referenced this pull request Aug 12, 2026
## Which issue does this PR close?
- Closesapache#23290.
## What changes are included in this PR?
In two commits:
* Improve the robustness of `WindowTopN` to intermediate
`RepartitionExec` nodes, to ensure that `PartitionedTopK` can still be
used when `target_partitions` mismatches physical partitions.
* Use `required_input_distributions` to declare required partitioning
for `PartitionedTopKExec`, `BoundedWindowAggExec`, and `WindowAggExec`.
## Are these changes tested?
Yes, new `slt` tests are added for both the `WindowTopN` robustness fix,
and for the use of `required_input_distributions` in `PartitionedTopK`.
## Are there any user-facing changes?
No: only plan changes.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizerOptimizer rulesphysical-planChanges to the physical-plan cratesqllogictestSQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Partitioning::Range to satisfy PartitionedTopK Distribution::KeyPartitioned requirements

3 participants

@stuhood@gene-bordegaray@gabotechs