Skip to content

feat: impl Any for MemoryPool - #21803

Merged
alamb merged 10 commits into
apache:mainfrom
haohuaijin:memory-any
May 13, 2026
Merged

feat: impl Any for MemoryPool#21803
alamb merged 10 commits into
apache:mainfrom
haohuaijin:memory-any

Conversation

@haohuaijin

@haohuaijin haohuaijin commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

implment Any for MemoryPool

Are these changes tested?

yes, add one test case

Are there any user-facing changes?

@github-actions github-actions Bot added the execution Related to the execution crate label Apr 23, 2026
nathanb9
nathanb9 reviewed Apr 23, 2026
View reviewed changes
Comment thread datafusion/execution/src/memory_pool/mod.rs
/// * [`TrackConsumersPool`]: Wraps another [`MemoryPool`] and tracks consumers,
/// providing better error messages on the largest memory users.
pub trait MemoryPool: Send + Sync + std::fmt::Debug + Display {
pub trait MemoryPool: Any + Send + Sync + std::fmt::Debug + Display {

@nathanb9 nathanb9 Apr 23, 2026

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.

Adding Any as a supertrait adds a 'static requirement to every MemoryPool implementor. This might break downstream if they have implemented custom pools which borrowed state or lifetime parameters

@haohuaijin haohuaijin Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thank you for mention this, i add upgrade guide in 344b52f

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Apr 24, 2026
@github-actions

github-actions Bot commented May 6, 2026 •
edited
Loading

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-execution v53.1.0 (current)
       Built [  30.426s] (current)
     Parsing datafusion-execution v53.1.0 (current)
      Parsed [   0.025s] (current)
    Building datafusion-execution v53.1.0 (baseline)
       Built [  29.931s] (baseline)
     Parsing datafusion-execution v53.1.0 (baseline)
      Parsed [   0.026s] (baseline)
    Checking datafusion-execution v53.1.0 -> v53.1.0 (no change; assume patch)
     Checked [   0.199s] 222 checks: 221 pass, 1 fail, 0 warn, 30 skip

--- failure trait_added_supertrait: non-sealed trait added new supertraits ---

Description:
A non-sealed trait added one or more supertraits, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#generic-bounds-tighten
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/trait_added_supertrait.ron

Failed in:
  trait datafusion_execution::memory_pool::MemoryPool gained Any in file /home/runner/work/datafusion/datafusion/datafusion/execution/src/memory_pool/mod.rs:186

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  61.996s] datafusion-execution

@github-actions github-actions Bot added the auto detected api change Auto detected API change label May 6, 2026
@haohuaijin

haohuaijin commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

thanks for your reviews @xudong963

@alamb
alamb added this pull request to the merge queue May 13, 2026
Merged via the queue into apache:main with commit fdaa326 May 13, 2026
36 checks passed
@haohuaijin
haohuaijin deleted the memory-any branch May 13, 2026 14:52
gstvg pushed a commit to gstvg/arrow-datafusion that referenced this pull request May 14, 2026
@haohuaijin @blaginin @gstvg
feat: impl Any for MemoryPool (apache#21803)
300a05e 
## Which issue does this PR close?


- Closes apache#21802 

## Rationale for this change

- see apache#21802

## What changes are included in this PR?


implment Any for MemoryPool

## Are these changes tested?

yes, add one test case

## Are there any user-facing changes?

---------

Co-authored-by: Dmitrii Blaginin <[email protected]>
@morristai morristai mentioned this pull request Oct 23, 2026
Merged
tsenart pushed a commit to perfloop-oss/datafusion that referenced this pull request Nov 13, 2026
@morristai @claude
feat: allow downcasting ParquetFileReaderFactory (apache#26635)
9f89821 
## Which issue does this PR close?

- Closes apache#26633.

## Rationale for this change

There is no way to tell which reader factory a `ParquetSource` uses.
`parquet_file_reader_factory()` returns `&Arc<dyn
ParquetFileReaderFactory>`, and the trait has no downcast path.

## What changes are included in this PR?

`ParquetFileReaderFactory` gets `Any` as a supertrait, and `dyn
ParquetFileReaderFactory` gets `is` and `downcast_ref`, like `dyn
FileSource` in apache#21576. I also added a 57.0.0 upgrade guide entry, like
the ones for `MemoryPool` (apache#21803) and `QueryPlanner` (apache#22241).

## What is the testing strategy for this PR?

A unit test in `source.rs` sets each built-in factory on a
`ParquetSource` and checks that `parquet_file_reader_factory()`
downcasts to it.

## Are there any user-facing changes?

Two new methods on `dyn ParquetFileReaderFactory`. The semver check will
flag the new supertrait, but the trait already required `'static`, so
existing implementations compile unchanged.

Drafted with Claude. I reviewed the change and this text.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xudong963 xudong963 xudong963 approved these changes
+1 more reviewer
@nathanb9 nathanb9 nathanb9 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

auto detected api change Auto detected API change documentation Improvements or additions to documentation execution Related to the execution crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

impl Any for MemoryPool

5 participants

@haohuaijin @xudong963 @nathanb9 @alamb @blaginin