Skip to content

Commit a601075

Browse files
Un-deprecate with_default_selectivity per reviewer feedback
The reviewer pointed out that with_default_selectivity() simply updates a field and doesn't need the overhead of creating a new FilterExec via the builder. Restored the original efficient implementation. Only with_projection() and with_batch_size() remain deprecated, as they benefit from the builder pattern's single property computation. - Restored original with_default_selectivity implementation - Updated upgrading.md to reflect only 2 deprecated methods - All 114 filter tests passing - No clippy warnings
1 parent aa4a892 commit a601075

File tree

2 files changed

+12
-14
lines changed

2 files changed

+12
-14
lines changed

datafusion/physical-plan/src/filter.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,20 +195,17 @@ impl FilterExec {
195195
}
196196

197197
/// Set the default selectivity
198-
///
199-
/// # Deprecated
200-
/// Use [`FilterExecBuilder::with_default_selectivity`] instead
201-
#[deprecated(since = "52.0.0", note = "Use FilterExecBuilder::with_default_selectivity instead")]
202198
pub fn with_default_selectivity(
203-
self,
199+
mut self,
204200
default_selectivity: u8,
205201
) -> Result<Self, DataFusionError> {
206-
FilterExecBuilder::new(Arc::clone(&self.predicate), Arc::clone(&self.input))
207-
.with_projection(self.projection.clone())
208-
.with_default_selectivity(default_selectivity)
209-
.with_batch_size(self.batch_size)
210-
.with_fetch(self.fetch)
211-
.build()
202+
if default_selectivity > 100 {
203+
return plan_err!(
204+
"Default filter selectivity value needs to be less than or equal to 100"
205+
);
206+
}
207+
self.default_selectivity = default_selectivity;
208+
Ok(self)
212209
}
213210

214211
/// Return new instance of [FilterExec] with the given projection.

docs/source/library-user-guide/upgrading.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ You can see the current [status of the `52.0.0`release here](https://github.com/
3030
The following methods on `FilterExec` have been deprecated in favor of using `FilterExecBuilder`:
3131

3232
- `with_projection()`
33-
- `with_default_selectivity()`
3433
- `with_batch_size()`
3534

3635
**Who is affected:**
@@ -46,20 +45,22 @@ Use `FilterExecBuilder` instead of chaining method calls on `FilterExec`:
4645
```rust,ignore
4746
let filter = FilterExec::try_new(predicate, input)?
4847
.with_projection(Some(vec![0, 2]))?
49-
.with_default_selectivity(30)?;
48+
.with_batch_size(8192)?;
5049
```
5150

5251
**After:**
5352

5453
```rust,ignore
5554
let filter = FilterExecBuilder::new(predicate, input)
5655
.with_projection(Some(vec![0, 2]))
57-
.with_default_selectivity(30)
56+
.with_batch_size(8192)
5857
.build()?;
5958
```
6059

6160
The builder pattern is more efficient as it computes properties once during `build()` rather than recomputing them for each method call.
6261

62+
Note: `with_default_selectivity()` is not deprecated as it simply updates a field value and does not require the overhead of the builder pattern.
63+
6364
### Changes to DFSchema API
6465

6566
To permit more efficient planning, several methods on `DFSchema` have been

0 commit comments

Comments
 (0)