Repository navigation
refactor(pruning): deprecate PruningPredicate::try_new - #24129
Merged
zhuqi-lucas merged 4 commits intoAug 7, 2026
Merged
Conversation
|
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24129 +/- ##
==========================================
- Coverage 81.05% 81.05% -0.01%
==========================================
Files 1106 1106
Lines 380556 380596 +40
Branches 380556 380596 +40
==========================================
+ Hits 308477 308507 +30
- Misses 53861 53867 +6
- Partials 18218 18222 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Should we add upgrade guide also? |
alamb
approved these changes
Aug 6, 2026
alamb
left a comment
alamb
left a comment
Contributor
There was a problem hiding this comment.
Thank you @goutamadwant -- this looks great
I had one small suggestion
cc @zhuqi-lucas
FYI @kosiew I think the PruningPredicateBuilder may be a natural place to wire in the cache from #21566
| /// Specifically, this example illustrates how to: | ||
| /// 1. Use [`ParquetFileReaderFactory`] to avoid re-reading parquet metadata on each query | ||
| /// 2. Use [`PruningPredicate`] for predicate analysis | ||
| /// 2. Use [`datafusion::physical_optimizer::pruning::PruningPredicate`] for predicate analysis |
Contributor
There was a problem hiding this comment.
can we avoid adding this full path and make it a normal link? (leave it PruningPredicate and then add a definition for pruning predicate below?
Contributor
Author
There was a problem hiding this comment.
I updated this @alamb. I restored the PruningPredicate link and added its reference definition alongside the existing documentation links. Let me know if this is good.
| // `PruningPredicate::prune` would likely be easier to do. | ||
| let pruning_predicate = | ||
| PruningPredicate::try_new(Arc::clone(predicate), self.schema())?; | ||
| let pruning_predicate = PruningPredicateBuilder::new() |
| pub fn try_build(self, predicate: Arc<dyn PhysicalExpr>) -> Result<PruningPredicate> { | ||
| pub fn try_build( | ||
| self, | ||
| mut predicate: Arc<dyn PhysicalExpr>, |
Contributor
There was a problem hiding this comment.
it is strange this got reformatted -- but clearly nothing you did
Contributor
Author
There was a problem hiding this comment.
right. I left this unchanged for now. :)
github-actions
Bot
added
the
documentation
Contributor
Author
@zhuqi-lucas added an entry to the DataFusion 55 upgrade guide documenting the deprecation and the equivalent PruningPredicateBuilder migration. Let me know if any suggestions. thanks! |
zhuqi-lucas
approved these changes
Aug 7, 2026
Contributor
|
LGTM, thank you @goutamadwant and @alamb ! Merged now. |
kosiew
pushed a commit
to kosiew/datafusion
that referenced
this pull request
Aug 12, 2026
## Which issue does this PR close? - Closes apache#24128. ## Rationale for this change `PruningPredicateBuilder` is the extensible entry point for constructing pruning predicates, but the construction logic still lived behind `PruningPredicate::try_new`. Keeping the logic in the builder provides one place to add future construction options and guides callers toward the extensible API. ## What changes are included in this PR? - Move pruning predicate construction into `PruningPredicateBuilder::try_build`. - Deprecate `PruningPredicate::try_new` and retain it as a compatibility wrapper using the builder defaults. - Migrate DataFusion call sites and examples to `PruningPredicateBuilder`. - Add a regression test confirming the deprecated constructor and builder produce equivalent predicates. ## Are these changes tested? Yes. The following checks pass: - `cargo fmt --all -- --check` - `cargo test -p datafusion-pruning` - `cargo test -p datafusion-datasource-parquet` - `cargo check -p datafusion-examples --examples` - `cargo clippy -p datafusion-pruning -p datafusion-datasource-parquet -p datafusion-examples --all-targets --all-features -- -D warnings` - `cargo test --profile=ci --test sqllogictests` - `cargo test -p datafusion` - `cargo test -p datafusion-cli` - `RUSTDOCFLAGS="-D warnings" cargo doc -p datafusion-pruning --no-deps` The workspace-wide clippy command is currently blocked on `main` by an existing `clippy::uninlined_format_args` diagnostic in `datafusion/proto-common/src/generated/pbjson.rs`. The same failure reproduces from a clean checkout of the base commit; all modified packages pass strict clippy checks. ## Are there any user-facing changes? Yes. `PruningPredicate::try_new` is deprecated as of 55.0.0. It remains available as a compatibility wrapper with unchanged behavior. New callers should construct predicates with `PruningPredicateBuilder`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
PruningPredicate::try_newin favor ofPruningPredicateBuilder#24128.Rationale for this change
PruningPredicateBuilderis the extensible entry point for constructing pruning predicates, but the construction logic still lived behindPruningPredicate::try_new. Keeping the logic in the builder provides one place to add future construction options and guides callers toward the extensible API.What changes are included in this PR?
PruningPredicateBuilder::try_build.PruningPredicate::try_newand retain it as a compatibility wrapper using the builder defaults.PruningPredicateBuilder.Are these changes tested?
Yes. The following checks pass:
cargo fmt --all -- --checkcargo test -p datafusion-pruningcargo test -p datafusion-datasource-parquetcargo check -p datafusion-examples --examplescargo clippy -p datafusion-pruning -p datafusion-datasource-parquet -p datafusion-examples --all-targets --all-features -- -D warningscargo test --profile=ci --test sqllogictestscargo test -p datafusioncargo test -p datafusion-cliRUSTDOCFLAGS="-D warnings" cargo doc -p datafusion-pruning --no-depsThe workspace-wide clippy command is currently blocked on
mainby an existingclippy::uninlined_format_argsdiagnostic indatafusion/proto-common/src/generated/pbjson.rs. The same failure reproduces from a clean checkout of the base commit; all modified packages pass strict clippy checks.Are there any user-facing changes?
Yes.
PruningPredicate::try_newis deprecated as of 55.0.0. It remains available as a compatibility wrapper with unchanged behavior. New callers should construct predicates withPruningPredicateBuilder.