You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "avantgardnerio (via GitHub)" <gi...@apache.org> on 2026/06/22 15:48:35 UTC
[PR] feat(physical-expr): add Partitioning::DynamicRange variant [datafusion]
avantgardnerio opened a new pull request, #23094:
URL: https://github.com/apache/datafusion/pull/23094
## Which issue does this PR close?
Implements the proposal in #23093. (Not using `Closes #23093` so the discussion thread can stay open for the broader API conversation.)
## Rationale for this change
See #23093 for full design rationale. Short version: `Partitioning::Range` (landed in #22207) covers the *declarative* case where split points are known at plan time. This adds the symmetric *runtime-discovered* sibling — where the boundary set is only known once an upstream operator has observed its actual data range. The partition count stays fixed at plan time so downstream distribution requirements have a stable answer; only the split point values are runtime-discovered.
Motivating downstream use case: parallelizing single-partition window functions (RANGE frames, no PARTITION BY) — see the spike at #23026.
## What changes are included in this PR?
Variant introduction only — no execution slot in this PR.
```rust
pub enum Partitioning {
...
Range(RangePartitioning),
DynamicRange(DynamicRangePartitioning), // <- new
UnknownPartitioning(usize),
}
pub struct DynamicRangePartitioning {
ordering: LexOrdering,
partition_count: usize,
}
```
Behavior mirrors `Range` at every match site:
- `Partitioning::partition_count`, `compatible_with`, `project`, `PartialEq`, `Display` arms added.
- `DynamicRangePartitioning` has `new`, `ordering`, `partition_count`, `compatible_with`, `project`, `Display` mirroring `RangePartitioning` (minus split-point validation, since there are no split points to validate at plan time).
- `RepartitionExec`'s `repartitioned()`, `try_pushdown_sort()`, and projection-pushdown sites return `not_impl_err!` for the new variant, same as they already do for `Range`.
- `RepartitionExec`'s row-routing path was already a catch-all `not_impl_err!` for non-Hash / non-RoundRobin variants, so no change is needed there.
- FFI bridges to `UnknownPartitioning(n)`, same path `Range` takes per #22394.
- Proto serialization returns `not_impl_err!` — proto plumbing for `DynamicRange` will be added incrementally, mirroring how `Range` landed in steps (#22207 → #22787).
## Are these changes tested?
Three new tests in `datafusion/physical-expr/src/partitioning.rs::tests`:
- `test_dynamic_range_partitioning_metadata` — construction, `Display`, `partition_count`, accessors.
- `test_dynamic_range_partitioning_compatible_with` — same ordering and same partition_count → compatible; different partition_count → not; different sort options → not; single-partition / single-partition → always compatible; through the `Partitioning` enum, including cross-variant (DynamicRange vs declared Range never compatible).
- `test_dynamic_range_partitioning_project_preserves_or_degrades` — projection preserves the ordering when the key survives; degrades to `UnknownPartitioning(n)` (preserving partition count) when the key is dropped.
`cargo clippy --all-features --all-targets -- -D warnings --no-deps` clean. `cargo fmt --all` clean. `cargo test -p datafusion-physical-expr --lib partitioning::`: 20 pass (3 new).
## Are there any user-facing changes?
- New public types in `datafusion::physical_expr`: `DynamicRangePartitioning`, `Partitioning::DynamicRange` enum variant.
- No changes to existing variants. Existing match sites on `Partitioning` may need a new arm — most upstream code already had one for `Range` and can extend it. The crates updated in this PR (`physical-plan`, `proto`, `ffi`) cover the in-tree consumers.
- No SQL surface 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat(physical-expr): add Partitioning::DynamicRange variant [datafusion]
Posted by "saadtajwar (via GitHub)" <gi...@apache.org>.
saadtajwar commented on code in PR #23094:
URL: https://github.com/apache/datafusion/pull/23094#discussion_r3455971811
##########
datafusion/proto/src/physical_plan/to_proto.rs:
##########
@@ -380,6 +380,15 @@ pub fn serialize_partitioning(
serialize_range_partitioning(range, codec, proto_converter)?,
)),
},
+ Partitioning::DynamicRange(_) => {
+ // Proto plumbing for DynamicRange is intentionally not
+ // implemented in the variant-introduction PR and will be
+ // added incrementally. See
+ // <https://github.com/apache/datafusion/issues/22395>.
+ return not_impl_err!(
+ "Serialization of DynamicRange partitioning is not implemented"
+ );
+ }
Review Comment:
Following the discussion in the GH issue & PR comments - I don't have a ton of feedback, as I'm still trying to understand the broader context (and Datafusion in general haha!) but if we get alignment here I'm happy to help out with this piece specifically :)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat(physical-expr): add Partitioning::DynamicRange variant [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #23094: feat(physical-expr): add Partitioning::DynamicRange variant
URL: https://github.com/apache/datafusion/pull/23094
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat(physical-expr): add Partitioning::DynamicRange variant [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23094:
URL: https://github.com/apache/datafusion/pull/23094#issuecomment-4770264800
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
<details>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion-ffi v54.0.0 (current)
Built [ 60.494s] (current)
Parsing datafusion-ffi v54.0.0 (current)
Parsed [ 0.059s] (current)
Building datafusion-ffi v54.0.0 (baseline)
Built [ 59.283s] (baseline)
Parsing datafusion-ffi v54.0.0 (baseline)
Parsed [ 0.061s] (baseline)
Checking datafusion-ffi v54.0.0 -> v54.0.0 (no change; assume patch)
Checked [ 0.261s] 223 checks: 223 pass, 30 skip
Summary no semver update required
Finished [ 122.940s] datafusion-ffi
Building datafusion-physical-expr v54.0.0 (current)
Built [ 29.170s] (current)
Parsing datafusion-physical-expr v54.0.0 (current)
Parsed [ 0.049s] (current)
Building datafusion-physical-expr v54.0.0 (baseline)
Built [ 29.392s] (baseline)
Parsing datafusion-physical-expr v54.0.0 (baseline)
Parsed [ 0.048s] (baseline)
Checking datafusion-physical-expr v54.0.0 -> v54.0.0 (no change; assume patch)
Checked [ 0.330s] 223 checks: 222 pass, 1 fail, 0 warn, 30 skip
--- failure enum_variant_added: enum variant added on exhaustive enum ---
Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/enum_variant_added.ron
Failed in:
variant Partitioning:DynamicRange in /home/runner/work/datafusion/datafusion/datafusion/physical-expr/src/partitioning.rs:128
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 60.494s] datafusion-physical-expr
Building datafusion-physical-plan v54.0.0 (current)
Built [ 36.558s] (current)
Parsing datafusion-physical-plan v54.0.0 (current)
Parsed [ 0.139s] (current)
Building datafusion-physical-plan v54.0.0 (baseline)
Built [ 37.026s] (baseline)
Parsing datafusion-physical-plan v54.0.0 (baseline)
Parsed [ 0.129s] (baseline)
Checking datafusion-physical-plan v54.0.0 -> v54.0.0 (no change; assume patch)
Checked [ 0.597s] 223 checks: 223 pass, 30 skip
Summary no semver update required
Finished [ 75.781s] datafusion-physical-plan
Building datafusion-proto v54.0.0 (current)
Built [ 59.800s] (current)
Parsing datafusion-proto v54.0.0 (current)
Parsed [ 0.017s] (current)
Building datafusion-proto v54.0.0 (baseline)
Built [ 59.642s] (baseline)
Parsing datafusion-proto v54.0.0 (baseline)
Parsed [ 0.019s] (baseline)
Checking datafusion-proto v54.0.0 -> v54.0.0 (no change; assume patch)
Checked [ 0.236s] 223 checks: 223 pass, 30 skip
Summary no semver update required
Finished [ 121.120s] datafusion-proto
```
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat(physical-expr): add Partitioning::DynamicRange variant [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23094:
URL: https://github.com/apache/datafusion/pull/23094#issuecomment-5377232600
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat(physical-expr): add Partitioning::DynamicRange variant [datafusion]
Posted by "avantgardnerio (via GitHub)" <gi...@apache.org>.
avantgardnerio commented on PR #23094:
URL: https://github.com/apache/datafusion/pull/23094#issuecomment-4770368684
Pushed `e6f846ed9b` — adds `HaloSpec` and a `halo: Option<HaloSpec>` field on `DynamicRangePartitioning`.
**Why the scope grew:** an honest answer to "what do we need before a runtime range repartitioner is implementable" surfaced halo as a missing piece. The routing operator and the downstream halo-strip operator need to agree at plan time on how far each bucket extends beyond its primary range; the field on the partitioning type is the natural carrier for that agreement. Without it, the two operators would need a side channel.
**Shape:**
```rust
pub struct HaloSpec {
preceding: ScalarValue,
following: ScalarValue,
}
pub struct DynamicRangePartitioning {
ordering: LexOrdering,
partition_count: usize,
halo: Option<HaloSpec>,
}
```
Distances are in the leading sort key's domain. Builder keeps the common (no-halo) case terse: `DynamicRangePartitioning::new(ordering, k)` vs `…::new(ordering, k).with_halo(halo)`.
This is the API hook the runtime range repartitioner needs to publish `ExtremaKind::Expanded` extrema (proposed in #23089 / implemented in #23090) to a downstream halo-strip filter. With halo unset, the partitioning produces disjoint buckets and downstream sees `Observed` extrema.
ROWS-frame halo (a count of neighbor rows rather than a domain distance) is intentionally not represented; a separate variant can be added later if motivated.
`compatible_with` requires halo equality; `project` passes halo through unchanged; `Display` adds the `halo(preceding=…, following=…)` suffix when set. Three new tests cover metadata, compatibility, and projection preservation.
I'll update the discussion at #23093 to mirror this scope shift.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org