Skip to content

Remove GroupsAccumulator::supports_convert_to_state and require convert_to_state#23489

Open
lyne7-sc wants to merge 1 commit into
apache:mainfrom
lyne7-sc:remove-groups-accumulator-support-flag
Open

Remove GroupsAccumulator::supports_convert_to_state and require convert_to_state#23489
lyne7-sc wants to merge 1 commit into
apache:mainfrom
lyne7-sc:remove-groups-accumulator-support-flag

Conversation

@lyne7-sc

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Following #23275, all GroupsAccumulator implementations now provide convert_to_state.

The supports_convert_to_state capability flag is therefore no longer needed.

What changes are included in this PR?

  • Make GroupsAccumulator::convert_to_state a required trait method.

  • Remove GroupsAccumulator::supports_convert_to_state and its implementations.

  • Remove the corresponding capability checks from hash aggregation.

  • Simplify skip-partial aggregation to use the required convert_to_state implementation directly.

  • Add a regression test covering the partial hash aggregation skip path.

  • Document the breaking trait change in the 55.0.0 upgrading guide.

  • Remove FFI_GroupsAccumulator::supports_convert_to_state. This changes the FFI ABI layout, so providers and consumers must be rebuilt against DataFusion 55.

Are these changes tested?

Yes. Added a regression test verifying that skip-partial aggregation uses the required convert_to_state implementation without a capability flag.

Existing physical-plan and FFI tests continue to pass.

Are there any user-facing changes?

Yes. This is a breaking Rust API change for external GroupsAccumulator implementations:

  • convert_to_state must now be implemented.
  • supports_convert_to_state should be removed.

The migration is documented in the 55.0.0 upgrading guide.

The FFI_GroupsAccumulator layout has changed. FFI providers and consumers must be rebuilt against DataFusion 55 and must not exchange this struct with older major versions.

@github-actions github-actions Bot added documentation Improvements or additions to documentation logical-expr Logical plan and expressions core Core DataFusion crate functions Changes to functions implementation ffi Changes to the ffi crate physical-plan Changes to the physical-plan crate spark labels Jul 12, 2026
@github-actions

Copy link
Copy Markdown

Thank you for opening this pull request!

Reviewer note: 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
     Cloning apache/main
    Building datafusion v54.0.0 (current)
       Built [ 105.840s] (current)
     Parsing datafusion v54.0.0 (current)
      Parsed [   0.034s] (current)
    Building datafusion v54.0.0 (baseline)
       Built [ 102.844s] (baseline)
     Parsing datafusion v54.0.0 (baseline)
      Parsed [   0.035s] (baseline)
    Checking datafusion v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.628s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [ 211.026s] datafusion
    Building datafusion-expr-common v54.0.0 (current)
       Built [  18.724s] (current)
     Parsing datafusion-expr-common v54.0.0 (current)
      Parsed [   0.018s] (current)
    Building datafusion-expr-common v54.0.0 (baseline)
       Built [  19.079s] (baseline)
     Parsing datafusion-expr-common v54.0.0 (baseline)
      Parsed [   0.018s] (baseline)
    Checking datafusion-expr-common v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.217s] 223 checks: 221 pass, 2 fail, 0 warn, 30 skip

--- failure trait_method_default_impl_removed: pub trait default method impl removed ---

Description:
A method's default impl in an unsealed trait has been removed, breaking trait implementations that relied on that default
        ref: https://doc.rust-lang.org/book/ch10-02-traits.html#default-implementations
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/trait_method_default_impl_removed.ron

Failed in:
  trait method datafusion_expr_common::groups_accumulator::GroupsAccumulator::convert_to_state in file /home/runner/work/datafusion/datafusion/datafusion/expr-common/src/groups_accumulator.rs:232

--- failure trait_method_missing: pub trait method removed or renamed ---

Description:
A trait method is no longer callable, and may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-item-signature
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/trait_method_missing.ron

Failed in:
  method supports_convert_to_state of trait GroupsAccumulator, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/da6043036f7c1e9a11fb1a08008a9adb854dcfe7/datafusion/expr-common/src/groups_accumulator.rs:242

     Summary semver requires new major version: 2 major and 0 minor checks failed
    Finished [  38.715s] datafusion-expr-common
    Building datafusion-ffi v54.0.0 (current)
       Built [  58.386s] (current)
     Parsing datafusion-ffi v54.0.0 (current)
      Parsed [   0.061s] (current)
    Building datafusion-ffi v54.0.0 (baseline)
       Built [  58.041s] (baseline)
     Parsing datafusion-ffi v54.0.0 (baseline)
      Parsed [   0.062s] (baseline)
    Checking datafusion-ffi v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.243s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [ 118.208s] datafusion-ffi
    Building datafusion-functions-aggregate v54.0.0 (current)
       Built [  30.050s] (current)
     Parsing datafusion-functions-aggregate v54.0.0 (current)
      Parsed [   0.045s] (current)
    Building datafusion-functions-aggregate v54.0.0 (baseline)
       Built [  30.048s] (baseline)
     Parsing datafusion-functions-aggregate v54.0.0 (baseline)
      Parsed [   0.046s] (baseline)
    Checking datafusion-functions-aggregate v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.190s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  61.366s] datafusion-functions-aggregate
    Building datafusion-functions-aggregate-common v54.0.0 (current)
       Built [  19.987s] (current)
     Parsing datafusion-functions-aggregate-common v54.0.0 (current)
      Parsed [   0.019s] (current)
    Building datafusion-functions-aggregate-common v54.0.0 (baseline)
       Built [  19.961s] (baseline)
     Parsing datafusion-functions-aggregate-common v54.0.0 (baseline)
      Parsed [   0.020s] (baseline)
    Checking datafusion-functions-aggregate-common v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.135s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  40.768s] datafusion-functions-aggregate-common
    Building datafusion-physical-plan v54.0.0 (current)
       Built [  35.971s] (current)
     Parsing datafusion-physical-plan v54.0.0 (current)
      Parsed [   0.137s] (current)
    Building datafusion-physical-plan v54.0.0 (baseline)
       Built [  35.483s] (baseline)
     Parsing datafusion-physical-plan v54.0.0 (baseline)
      Parsed [   0.144s] (baseline)
    Checking datafusion-physical-plan v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.612s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  75.131s] datafusion-physical-plan
    Building datafusion-spark v54.0.0 (current)
       Built [  57.141s] (current)
     Parsing datafusion-spark v54.0.0 (current)
      Parsed [   0.060s] (current)
    Building datafusion-spark v54.0.0 (baseline)
       Built [  57.275s] (baseline)
     Parsing datafusion-spark v54.0.0 (baseline)
      Parsed [   0.066s] (baseline)
    Checking datafusion-spark v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.345s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [ 116.004s] datafusion-spark

@github-actions github-actions Bot added the auto detected api change Auto detected API change label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto detected api change Auto detected API change core Core DataFusion crate documentation Improvements or additions to documentation ffi Changes to the ffi crate functions Changes to functions implementation logical-expr Logical plan and expressions physical-plan Changes to the physical-plan crate spark

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove GroupsAccumulator::supports_convert_to_state and make convert_to_state mandatory

1 participant