This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24129-491de04ec18e4cbd299e773011050bede47147f6 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit f05869b0e6828e6524b06cde71e697bfb84fd2ee Author: Goutam Adwant <[email protected]> AuthorDate: Thu Aug 6 20:08:54 2026 -0700 refactor(pruning): deprecate PruningPredicate::try_new (#24129) ## Which issue does this PR close? - Closes #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`. --- .../examples/data_io/parquet_advanced_index.rs | 8 +- .../examples/data_io/parquet_index.rs | 10 +- .../examples/query_planning/pruning.rs | 9 +- datafusion/datasource-parquet/src/bloom_filter.rs | 24 ++-- datafusion/datasource-parquet/src/page_filter.rs | 10 +- .../datasource-parquet/src/row_group_filter.rs | 43 +++--- datafusion/pruning/src/pruning_predicate.rs | 157 ++++++++++++--------- docs/source/library-user-guide/upgrading/55.0.0.md | 16 +++ 8 files changed, 174 insertions(+), 103 deletions(-) diff --git a/datafusion-examples/examples/data_io/parquet_advanced_index.rs b/datafusion-examples/examples/data_io/parquet_advanced_index.rs index 9bdcda265e..563b536915 100644 --- a/datafusion-examples/examples/data_io/parquet_advanced_index.rs +++ b/datafusion-examples/examples/data_io/parquet_advanced_index.rs @@ -47,7 +47,7 @@ use datafusion::parquet::file::properties::{EnabledStatistics, WriterProperties} use datafusion::parquet::schema::types::ColumnPath; use datafusion::physical_expr::PhysicalExpr; use datafusion::physical_expr::utils::{Guarantee, LiteralGuarantee}; -use datafusion::physical_optimizer::pruning::PruningPredicate; +use datafusion::physical_optimizer::pruning::PruningPredicateBuilder; use datafusion::physical_plan::ExecutionPlan; use datafusion::physical_plan::metrics::ExecutionPlanMetricsSet; use datafusion::prelude::*; @@ -155,6 +155,7 @@ use url::Url; /// ``` /// /// [`ListingTable`]: datafusion::datasource::listing::ListingTable +/// [`PruningPredicate`]: datafusion::physical_optimizer::pruning::PruningPredicate /// [Page Index](https://github.com/apache/parquet-format/blob/master/PageIndex.md) pub async fn parquet_advanced_index() -> Result<()> { // the object store is used to read the parquet files (in this case, it is @@ -300,8 +301,9 @@ impl IndexTableProvider { // In this example, we use the PruningPredicate's literal guarantees to // analyze the predicate. In a real system, using // `PruningPredicate::prune` would likely be easier to do. - let pruning_predicate = - PruningPredicate::try_new(Arc::clone(predicate), self.schema())?; + let pruning_predicate = PruningPredicateBuilder::new() + .with_file_schema(self.schema()) + .try_build(Arc::clone(predicate))?; // The PruningPredicate's guarantees must all be satisfied in order for // the predicate to possibly evaluate to true. diff --git a/datafusion-examples/examples/data_io/parquet_index.rs b/datafusion-examples/examples/data_io/parquet_index.rs index 9be84d8249..753d1b30fc 100644 --- a/datafusion-examples/examples/data_io/parquet_index.rs +++ b/datafusion-examples/examples/data_io/parquet_index.rs @@ -42,7 +42,7 @@ use datafusion::parquet::arrow::{ ArrowWriter, arrow_reader::ParquetRecordBatchReaderBuilder, }; use datafusion::physical_expr::PhysicalExpr; -use datafusion::physical_optimizer::pruning::PruningPredicate; +use datafusion::physical_optimizer::pruning::PruningPredicateBuilder; use datafusion::physical_plan::ExecutionPlan; use datafusion::prelude::*; use std::collections::HashSet; @@ -274,7 +274,8 @@ impl TableProvider for IndexTableProvider { /// Simple in memory secondary index for a set of parquet files /// /// The index is represented as an arrow [`RecordBatch`] that can be passed -/// directly by the DataFusion [`PruningPredicate`] API +/// directly by the DataFusion +/// [`datafusion::physical_optimizer::pruning::PruningPredicate`] API /// /// The `RecordBatch` looks as follows. /// @@ -362,8 +363,9 @@ impl ParquetMetadataIndex { ) -> Result<Vec<(&str, u64)>> { // Use the PruningPredicate API to determine which files can not // possibly have any relevant data. - let pruning_predicate = - PruningPredicate::try_new(predicate, self.schema().clone())?; + let pruning_predicate = PruningPredicateBuilder::new() + .with_file_schema(self.schema().clone()) + .try_build(predicate)?; // Now evaluate the pruning predicate into a boolean mask, one element per // file in the index. If the mask is true, the file may have rows that diff --git a/datafusion-examples/examples/query_planning/pruning.rs b/datafusion-examples/examples/query_planning/pruning.rs index dad57cd261..023058f825 100644 --- a/datafusion-examples/examples/query_planning/pruning.rs +++ b/datafusion-examples/examples/query_planning/pruning.rs @@ -28,7 +28,9 @@ use datafusion::error::Result; use datafusion::execution::context::ExecutionProps; use datafusion::logical_expr::physical_planning_context::PhysicalPlanningContext; use datafusion::physical_expr::create_physical_expr; -use datafusion::physical_optimizer::pruning::PruningPredicate; +use datafusion::physical_optimizer::pruning::{ + PruningPredicate, PruningPredicateBuilder, +}; use datafusion::prelude::*; /// This example shows how to use DataFusion's `PruningPredicate` to prove @@ -202,7 +204,10 @@ fn create_pruning_predicate(expr: Expr, schema: &SchemaRef) -> PruningPredicate &PhysicalPlanningContext::default(), ) .unwrap(); - PruningPredicate::try_new(physical_expr, Arc::clone(schema)).unwrap() + PruningPredicateBuilder::new() + .with_file_schema(Arc::clone(schema)) + .try_build(physical_expr) + .unwrap() } fn i32_array<'a>(values: impl Iterator<Item = &'a Option<i32>>) -> ArrayRef { diff --git a/datafusion/datasource-parquet/src/bloom_filter.rs b/datafusion/datasource-parquet/src/bloom_filter.rs index a8f01a5547..4cfe8bf1f8 100644 --- a/datafusion/datasource-parquet/src/bloom_filter.rs +++ b/datafusion/datasource-parquet/src/bloom_filter.rs @@ -249,13 +249,23 @@ mod tests { use datafusion_expr::{Expr, col, lit}; use datafusion_physical_expr::planner::logical2physical; use datafusion_physical_plan::metrics::ExecutionPlanMetricsSet; - use datafusion_pruning::PruningPredicate; + use datafusion_pruning::{PruningPredicate, PruningPredicateBuilder}; use object_store::{ObjectStore, ObjectStoreExt}; use parquet::arrow::ArrowWriter; use parquet::arrow::ParquetRecordBatchStreamBuilder; use parquet::arrow::async_reader::ParquetObjectReader; use parquet::file::properties::{EnabledStatistics, WriterProperties}; + fn build_test_pruning_predicate( + expr: Arc<dyn datafusion_physical_plan::PhysicalExpr>, + schema: Schema, + ) -> PruningPredicate { + PruningPredicateBuilder::new() + .with_file_schema(Arc::new(schema)) + .try_build(expr) + .unwrap() + } + #[tokio::test] async fn test_row_group_bloom_filter_pruning_predicate_simple_expr() { BloomFilterTest::new_data_index_bloom_encoding_stats() @@ -321,8 +331,7 @@ mod tests { false, ); let expr = logical2physical(&expr, &schema); - let pruning_predicate = - PruningPredicate::try_new(expr, Arc::new(schema)).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, schema); let pruned_row_groups = test_row_group_bloom_filter_pruning_predicate( file_name, @@ -439,8 +448,7 @@ mod tests { None, )); let expr = logical2physical(&expr, &schema); - let pruning_predicate = - PruningPredicate::try_new(expr, Arc::new(schema)).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, schema); let pruned_row_groups = test_row_group_bloom_filter_pruning_predicate( &format!("decimal128-{precision}.parquet"), @@ -477,8 +485,7 @@ mod tests { None, )); let expr = logical2physical(&expr, &schema); - let pruning_predicate = - PruningPredicate::try_new(expr, Arc::new(schema)).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, schema); let pruned_row_groups = test_row_group_bloom_filter_pruning_predicate( &format!("negative-decimal128-{precision}.parquet"), @@ -573,8 +580,7 @@ mod tests { let data = bytes::Bytes::from(std::fs::read(path).unwrap()); let expr = logical2physical(&expr, &schema); - let pruning_predicate = - PruningPredicate::try_new(expr, Arc::new(schema)).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, schema); let pruned_row_groups = test_row_group_bloom_filter_pruning_predicate( &file_name, diff --git a/datafusion/datasource-parquet/src/page_filter.rs b/datafusion/datasource-parquet/src/page_filter.rs index 791f658bea..557ed9157c 100644 --- a/datafusion/datasource-parquet/src/page_filter.rs +++ b/datafusion/datasource-parquet/src/page_filter.rs @@ -31,7 +31,7 @@ use arrow::{ use datafusion_common::ScalarValue; use datafusion_common::pruning::PruningStatistics; use datafusion_physical_expr::{PhysicalExpr, split_conjunction}; -use datafusion_pruning::PruningPredicate; +use datafusion_pruning::{PruningPredicate, PruningPredicateBuilder}; use log::{debug, trace}; use parquet::arrow::arrow_reader::statistics::StatisticsConverter; @@ -144,10 +144,10 @@ impl PagePruningAccessPlanFilter { let predicates = split_conjunction(expr) .into_iter() .filter_map(|predicate| { - let pp = match PruningPredicate::try_new( - Arc::clone(predicate), - Arc::clone(&schema), - ) { + let pp = match PruningPredicateBuilder::new() + .with_file_schema(Arc::clone(&schema)) + .try_build(Arc::clone(predicate)) + { Ok(pp) => pp, Err(e) => { debug!("Ignoring error creating page pruning predicate: {e}"); diff --git a/datafusion/datasource-parquet/src/row_group_filter.rs b/datafusion/datasource-parquet/src/row_group_filter.rs index 2a2544b99b..ddf71bb7e6 100644 --- a/datafusion/datasource-parquet/src/row_group_filter.rs +++ b/datafusion/datasource-parquet/src/row_group_filter.rs @@ -29,7 +29,7 @@ use datafusion_expr::Operator; use datafusion_physical_expr::expressions::{BinaryExpr, IsNullExpr, NotExpr}; use datafusion_physical_expr::utils::collect_columns; use datafusion_physical_expr::{PhysicalExpr, PhysicalExprSimplifier}; -use datafusion_pruning::PruningPredicate; +use datafusion_pruning::{PruningPredicate, PruningPredicateBuilder}; use parquet::arrow::arrow_reader::statistics::StatisticsConverter; use parquet::file::metadata::RowGroupMetaData; use parquet::schema::types::SchemaDescriptor; @@ -373,8 +373,9 @@ impl RowGroupAccessPlanFilter { return; }; - let Ok(inverted_predicate) = - PruningPredicate::try_new(inverted_expr, Arc::clone(predicate.schema())) + let Ok(inverted_predicate) = PruningPredicateBuilder::new() + .with_file_schema(Arc::clone(predicate.schema())) + .try_build(inverted_expr) else { return; }; @@ -550,6 +551,16 @@ mod tests { schema::types::SchemaDescPtr, }; + fn build_test_pruning_predicate( + expr: Arc<dyn PhysicalExpr>, + schema: Arc<Schema>, + ) -> PruningPredicate { + PruningPredicateBuilder::new() + .with_file_schema(schema) + .try_build(expr) + .unwrap() + } + struct PrimitiveTypeField { name: &'static str, physical_ty: PhysicalType, @@ -612,7 +623,7 @@ mod tests { Arc::new(Schema::new(vec![Field::new("c1", DataType::Int32, false)])); let expr = col("c1").gt(lit(15)); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let field = PrimitiveTypeField::new("c1", PhysicalType::INT32); let schema_descr = get_test_schema_descr(vec![field]); @@ -655,7 +666,7 @@ mod tests { let schema = Arc::new(Schema::new(vec![Field::new("c1", DataType::Int32, true)])); let expr = logical2physical(&col("c1").gt(lit(15)), &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let field = PrimitiveTypeField::new("c1", PhysicalType::INT32); let schema_descr = get_test_schema_descr(vec![field]); @@ -781,7 +792,7 @@ mod tests { Arc::new(Schema::new(vec![Field::new("c1", DataType::Int32, false)])); let expr = col("c1").gt(lit(15)); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let field = PrimitiveTypeField::new("c1", PhysicalType::INT32); let schema_descr = get_test_schema_descr(vec![field]); @@ -824,7 +835,7 @@ mod tests { ])); let expr = col("c1").gt(lit(15)).and(col("c2").rem(lit(2)).eq(lit(0))); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let schema_descr = get_test_schema_descr(vec![ PrimitiveTypeField::new("c1", PhysicalType::INT32), @@ -863,7 +874,7 @@ mod tests { // this bypasses the entire predicate expression and no row groups are filtered out let expr = col("c1").gt(lit(15)).or(col("c2").rem(lit(2)).eq(lit(0))); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); // if conditions in predicate are joined with OR and an unsupported expression is used // this bypasses the entire predicate expression and no row groups are filtered out @@ -890,7 +901,7 @@ mod tests { let expr = col("c1").gt(lit(0)); let expr = logical2physical(&expr, &table_schema); let pruning_predicate = - PruningPredicate::try_new(expr, table_schema.clone()).unwrap(); + build_test_pruning_predicate(expr, Arc::clone(&table_schema)); // Model a file schema's column order c2 then c1, which is the opposite // of the table schema @@ -967,7 +978,7 @@ mod tests { let schema_descr = ArrowSchemaConverter::new().convert(&schema).unwrap(); let expr = col("c1").gt(lit(15)).and(col("c2").is_null()); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let groups = gen_row_group_meta_data_for_pruning_predicate(); let metrics = parquet_file_metrics(); @@ -998,7 +1009,7 @@ mod tests { .gt(lit(15)) .and(col("c2").eq(lit(ScalarValue::Boolean(None)))); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let groups = gen_row_group_meta_data_for_pruning_predicate(); let metrics = parquet_file_metrics(); @@ -1033,7 +1044,7 @@ mod tests { let schema_descr = get_test_schema_descr(vec![field]); let expr = col("c1").gt(lit(ScalarValue::Decimal128(Some(500), 9, 2))); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let rgm1 = get_row_group_meta_data( &schema_descr, // [1.00, 6.00] @@ -1101,7 +1112,7 @@ mod tests { Decimal128(11, 2), )); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let rgm1 = get_row_group_meta_data( &schema_descr, // [100, 600] @@ -1190,7 +1201,7 @@ mod tests { let schema_descr = get_test_schema_descr(vec![field]); let expr = col("c1").lt(lit(ScalarValue::Decimal128(Some(500), 18, 2))); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); let rgm1 = get_row_group_meta_data( &schema_descr, // [6.00, 8.00] @@ -1248,7 +1259,7 @@ mod tests { let left = cast(col("c1"), Decimal128(28, 3)); let expr = left.eq(lit(ScalarValue::Decimal128(Some(100000), 28, 3))); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); // we must use the big-endian when encode the i128 to bytes or vec[u8]. let rgm1 = get_row_group_meta_data( &schema_descr, @@ -1323,7 +1334,7 @@ mod tests { let left = cast(col("c1"), Decimal128(28, 3)); let expr = left.eq(lit(ScalarValue::Decimal128(Some(100000), 28, 3))); let expr = logical2physical(&expr, &schema); - let pruning_predicate = PruningPredicate::try_new(expr, schema.clone()).unwrap(); + let pruning_predicate = build_test_pruning_predicate(expr, Arc::clone(&schema)); // we must use the big-endian when encode the i128 to bytes or vec[u8]. let rgm1 = get_row_group_meta_data( &schema_descr, diff --git a/datafusion/pruning/src/pruning_predicate.rs b/datafusion/pruning/src/pruning_predicate.rs index ccb3e2bef5..3a63451495 100644 --- a/datafusion/pruning/src/pruning_predicate.rs +++ b/datafusion/pruning/src/pruning_predicate.rs @@ -108,7 +108,7 @@ use datafusion_physical_plan::{ColumnarValue, PhysicalExpr}; /// C: true (rows might match x = 5) /// ``` /// -/// See [`PruningPredicate::try_new`] and [`PruningPredicate::prune`] for more information. +/// See [`PruningPredicateBuilder`] and [`PruningPredicate::prune`] for more information. /// /// # Background /// @@ -409,8 +409,6 @@ pub fn build_pruning_predicate( /// - [`Self::try_build`]: returns a raw `Result<PruningPredicate>` for /// callers that want to surface errors themselves. /// -/// Callers that only need the historical `expr` / `schema` API can still -/// use [`PruningPredicate::try_new`] directly. #[derive(Default)] pub struct PruningPredicateBuilder<'a> { file_schema: Option<SchemaRef>, @@ -419,8 +417,7 @@ pub struct PruningPredicateBuilder<'a> { } impl<'a> PruningPredicateBuilder<'a> { - /// Create a new builder with defaults matching the historical - /// [`PruningPredicate::try_new`] behaviour. + /// Create a new builder with the default pruning predicate configuration. pub fn new() -> Self { Self { file_schema: None, @@ -483,13 +480,56 @@ impl<'a> PruningPredicateBuilder<'a> { /// Build a [`PruningPredicate`], returning the construction error /// directly. Callers that want the always-true predicate elided or /// errors folded into a counter should use [`Self::build`] instead. - pub fn try_build(self, predicate: Arc<dyn PhysicalExpr>) -> Result<PruningPredicate> { + pub fn try_build( + self, + mut predicate: Arc<dyn PhysicalExpr>, + ) -> Result<PruningPredicate> { let file_schema = self.file_schema.ok_or_else(|| { _internal_datafusion_err!( "PruningPredicateBuilder requires a file schema (call `with_file_schema`)" ) })?; - PruningPredicate::try_new_inner(predicate, file_schema, self.max_in_list_size) + + // Get a (simpler) snapshot of the physical expr here to use with `PruningPredicate`. + // In particular this unravels any `DynamicFilterPhysicalExpr`s by snapshotting them + // so that PruningPredicate can work with a static expression. + let tf = snapshot_physical_expr_opt(predicate)?; + if tf.transformed { + // If we had an expression such as Dynamic(part_col < 5 and col < 10) + // (this could come from something like `select * from t order by part_col, col, limit 10`) + // after snapshotting and because `DynamicFilterPhysicalExpr` applies child replacements to its + // children after snapshotting and previously `replace_columns_with_literals` may have been called with partition values + // the expression we have now is `8 < 5 and col < 10`. + // Thus we need as simplifier pass to get `false and col < 10` => `false` here. + let simplifier = PhysicalExprSimplifier::new(&file_schema); + predicate = simplifier.simplify(tf.data)?; + } else { + predicate = tf.data; + } + let unhandled_hook = Arc::new(ConstantUnhandledPredicateHook::default()) as _; + + // build predicate expression once + let mut required_columns = RequiredColumns::new(); + let predicate_expr = build_predicate_expression( + &predicate, + &file_schema, + &mut required_columns, + &unhandled_hook, + self.max_in_list_size, + ); + let predicate_schema = required_columns.schema(); + // Simplify the newly created predicate to get rid of redundant casts, comparisons, etc. + let predicate_expr = + PhysicalExprSimplifier::new(&predicate_schema).simplify(predicate_expr)?; + let literal_guarantees = LiteralGuarantee::analyze(&predicate); + + Ok(PruningPredicate { + schema: file_schema, + predicate_expr, + required_columns, + orig_expr: predicate, + literal_guarantees, + }) } } @@ -552,59 +592,13 @@ impl PruningPredicate { /// returns a new expression. /// It is recommended that you pass the expressions through [`PhysicalExprSimplifier`] /// before calling this method to make sure the expressions can be used for pruning. + /// + /// Use [`PruningPredicateBuilder`] to construct new pruning predicates. + #[deprecated(since = "55.0.0", note = "Use PruningPredicateBuilder instead")] pub fn try_new(expr: Arc<dyn PhysicalExpr>, schema: SchemaRef) -> Result<Self> { - Self::try_new_inner(expr, schema, MAX_IN_LIST_SIZE) - } - - /// Internal constructor with an explicit cap on the `IN (...)` rewrite - /// size. External callers should reach this through - /// [`PruningPredicateBuilder::with_max_in_list_size`] instead of - /// depending on this signature directly. - pub(crate) fn try_new_inner( - mut expr: Arc<dyn PhysicalExpr>, - schema: SchemaRef, - max_in_list_size: usize, - ) -> Result<Self> { - // Get a (simpler) snapshot of the physical expr here to use with `PruningPredicate`. - // In particular this unravels any `DynamicFilterPhysicalExpr`s by snapshotting them - // so that PruningPredicate can work with a static expression. - let tf = snapshot_physical_expr_opt(expr)?; - if tf.transformed { - // If we had an expression such as Dynamic(part_col < 5 and col < 10) - // (this could come from something like `select * from t order by part_col, col, limit 10`) - // after snapshotting and because `DynamicFilterPhysicalExpr` applies child replacements to its - // children after snapshotting and previously `replace_columns_with_literals` may have been called with partition values - // the expression we have now is `8 < 5 and col < 10`. - // Thus we need as simplifier pass to get `false and col < 10` => `false` here. - let simplifier = PhysicalExprSimplifier::new(&schema); - expr = simplifier.simplify(tf.data)?; - } else { - expr = tf.data; - } - let unhandled_hook = Arc::new(ConstantUnhandledPredicateHook::default()) as _; - - // build predicate expression once - let mut required_columns = RequiredColumns::new(); - let predicate_expr = build_predicate_expression( - &expr, - &schema, - &mut required_columns, - &unhandled_hook, - max_in_list_size, - ); - let predicate_schema = required_columns.schema(); - // Simplify the newly created predicate to get rid of redundant casts, comparisons, etc. - let predicate_expr = - PhysicalExprSimplifier::new(&predicate_schema).simplify(predicate_expr)?; - let literal_guarantees = LiteralGuarantee::analyze(&expr); - - Ok(Self { - schema, - predicate_expr, - required_columns, - orig_expr: expr, - literal_guarantees, - }) + PruningPredicateBuilder::new() + .with_file_schema(schema) + .try_build(expr) } /// For each set of statistics, evaluates the pruning predicate @@ -2594,7 +2588,10 @@ mod tests { ])); let expr = col("c1").eq(lit(100)).and(col("c2").eq(lit(200))); let expr = logical2physical(&expr, &schema); - let p = PruningPredicate::try_new(expr, Arc::clone(&schema)).unwrap(); + let p = PruningPredicateBuilder::new() + .with_file_schema(Arc::clone(&schema)) + .try_build(expr) + .unwrap(); // note pruning expression refers to row_count twice assert_eq!( "c1_null_count@2 != row_count@3 AND c1_min@0 <= 100 AND 100 <= c1_max@1 AND c2_null_count@6 != row_count@3 AND c2_min@4 <= 200 AND 200 <= c2_max@5", @@ -3234,8 +3231,10 @@ mod tests { dynamic_phys_expr.with_new_children(remapped_expr).unwrap(); // After substitution the expression is c1 > 5 AND part = "B" which should prune the file since the partition value is "A" let expected = &[false]; - let p = - PruningPredicate::try_new(dynamic_filter_expr, Arc::clone(&schema)).unwrap(); + let p = PruningPredicateBuilder::new() + .with_file_schema(Arc::clone(&schema)) + .try_build(dynamic_filter_expr) + .unwrap(); let result = p.prune(&statistics).unwrap(); assert_eq!(result, expected); } @@ -3557,6 +3556,30 @@ mod tests { Ok(()) } + #[test] + #[expect(deprecated)] + fn deprecated_try_new_delegates_to_builder() -> Result<()> { + let schema = + Arc::new(Schema::new(vec![Field::new("c1", DataType::Int32, false)])); + let expr = logical2physical(&col("c1").eq(lit(1)), &schema); + + let deprecated = + PruningPredicate::try_new(Arc::clone(&expr), Arc::clone(&schema))?; + let builder = PruningPredicateBuilder::new() + .with_file_schema(schema) + .try_build(expr)?; + + assert_eq!( + deprecated.predicate_expr().to_string(), + builder.predicate_expr().to_string() + ); + assert_eq!( + deprecated.required_columns().schema(), + builder.required_columns().schema() + ); + Ok(()) + } + #[test] fn row_group_predicate_cast_int_int() -> Result<()> { let schema = Schema::new(vec![Field::new("c1", DataType::Int32, false)]); @@ -5943,7 +5966,10 @@ mod tests { ) { println!("Pruning with expr: {expr}"); let expr = logical2physical(&expr, schema); - let p = PruningPredicate::try_new(expr, Arc::<Schema>::clone(schema)).unwrap(); + let p = PruningPredicateBuilder::new() + .with_file_schema(Arc::<Schema>::clone(schema)) + .try_build(expr) + .unwrap(); let result = p.prune(statistics).unwrap(); assert_eq!(result, expected); } @@ -5958,7 +5984,10 @@ mod tests { let expr = logical2physical(&expr, schema); let simplifier = PhysicalExprSimplifier::new(schema); let expr = simplifier.simplify(expr).unwrap(); - let p = PruningPredicate::try_new(expr, Arc::<Schema>::clone(schema)).unwrap(); + let p = PruningPredicateBuilder::new() + .with_file_schema(Arc::<Schema>::clone(schema)) + .try_build(expr) + .unwrap(); let result = p.prune(statistics).unwrap(); assert_eq!(result, expected); } diff --git a/docs/source/library-user-guide/upgrading/55.0.0.md b/docs/source/library-user-guide/upgrading/55.0.0.md index 53cba29f9a..1997b8e1c1 100644 --- a/docs/source/library-user-guide/upgrading/55.0.0.md +++ b/docs/source/library-user-guide/upgrading/55.0.0.md @@ -355,6 +355,22 @@ if tracking.contains_dynamic_filter() { } ``` +### `PruningPredicate::try_new` is deprecated + +`datafusion_pruning::PruningPredicate::try_new` is deprecated. Use +`PruningPredicateBuilder` instead. The deprecated constructor remains available +in DataFusion 55 and preserves its existing behavior. + +```rust +// Before +let predicate = PruningPredicate::try_new(expr, schema)?; + +// After +let predicate = PruningPredicateBuilder::new() + .with_file_schema(schema) + .try_build(expr)?; +``` + ### `FilePruner::try_new` no longer builds a pruner for static predicates without statistics `datafusion_pruning::FilePruner::try_new` now returns `None` when the predicate --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
