Skip to content

Commit aa4a892

Browse files
Fix clippy warnings: use Arc::clone and builder pattern
- Replace .clone() with Arc::clone() to follow Rust best practices - Replace deprecated method calls in internal code with direct builder usage - Update with_new_children, try_swapping_with_projection, and EmbeddedProjection impl - All 114 filter tests passing - No clippy warnings or errors
1 parent 0726087 commit aa4a892

File tree

1 file changed

+30
-16
lines changed

1 file changed

+30
-16
lines changed

datafusion/physical-plan/src/filter.rs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ impl FilterExec {
203203
self,
204204
default_selectivity: u8,
205205
) -> Result<Self, DataFusionError> {
206-
FilterExecBuilder::new(self.predicate.clone(), self.input.clone())
206+
FilterExecBuilder::new(Arc::clone(&self.predicate), Arc::clone(&self.input))
207207
.with_projection(self.projection.clone())
208208
.with_default_selectivity(default_selectivity)
209209
.with_batch_size(self.batch_size)
@@ -228,7 +228,7 @@ impl FilterExec {
228228
None => None,
229229
};
230230

231-
FilterExecBuilder::new(self.predicate.clone(), self.input.clone())
231+
FilterExecBuilder::new(Arc::clone(&self.predicate), Arc::clone(&self.input))
232232
.with_projection(projection)
233233
.with_default_selectivity(self.default_selectivity)
234234
.with_batch_size(self.batch_size)
@@ -242,7 +242,7 @@ impl FilterExec {
242242
/// Use [`FilterExecBuilder::with_batch_size`] instead
243243
#[deprecated(since = "52.0.0", note = "Use FilterExecBuilder::with_batch_size instead")]
244244
pub fn with_batch_size(&self, batch_size: usize) -> Result<Self> {
245-
FilterExecBuilder::new(self.predicate.clone(), self.input.clone())
245+
FilterExecBuilder::new(Arc::clone(&self.predicate), Arc::clone(&self.input))
246246
.with_projection(self.projection.clone())
247247
.with_default_selectivity(self.default_selectivity)
248248
.with_batch_size(batch_size)
@@ -467,13 +467,13 @@ impl ExecutionPlan for FilterExec {
467467
self: Arc<Self>,
468468
mut children: Vec<Arc<dyn ExecutionPlan>>,
469469
) -> Result<Arc<dyn ExecutionPlan>> {
470-
FilterExec::try_new(Arc::clone(&self.predicate), children.swap_remove(0))
471-
.and_then(|e| {
472-
let selectivity = e.default_selectivity();
473-
e.with_default_selectivity(selectivity)
474-
})
475-
.and_then(|e| e.with_projection(self.projection().cloned()))
476-
.map(|e| e.with_fetch(self.fetch).unwrap())
470+
FilterExecBuilder::new(Arc::clone(&self.predicate), children.swap_remove(0))
471+
.with_default_selectivity(self.default_selectivity())
472+
.with_projection(self.projection().cloned())
473+
.with_batch_size(self.batch_size)
474+
.with_fetch(self.fetch)
475+
.build()
476+
.map(|e| Arc::new(e) as _)
477477
}
478478

479479
fn execute(
@@ -539,14 +539,12 @@ impl ExecutionPlan for FilterExec {
539539
if let Some(new_predicate) =
540540
update_expr(self.predicate(), projection.expr(), false)?
541541
{
542-
return FilterExec::try_new(
542+
return FilterExecBuilder::new(
543543
new_predicate,
544544
make_with_child(projection, self.input())?,
545545
)
546-
.and_then(|e| {
547-
let selectivity = self.default_selectivity();
548-
e.with_default_selectivity(selectivity)
549-
})
546+
.with_default_selectivity(self.default_selectivity())
547+
.build()
550548
.map(|e| Some(Arc::new(e) as _));
551549
}
552550
}
@@ -685,7 +683,23 @@ impl ExecutionPlan for FilterExec {
685683

686684
impl EmbeddedProjection for FilterExec {
687685
fn with_projection(&self, projection: Option<Vec<usize>>) -> Result<Self> {
688-
self.with_projection(projection)
686+
// Check if the projection is valid
687+
can_project(&self.schema(), projection.as_ref())?;
688+
689+
let projection = match projection {
690+
Some(projection) => match &self.projection {
691+
Some(p) => Some(projection.iter().map(|i| p[*i]).collect()),
692+
None => Some(projection),
693+
},
694+
None => None,
695+
};
696+
697+
FilterExecBuilder::new(Arc::clone(&self.predicate), Arc::clone(&self.input))
698+
.with_projection(projection)
699+
.with_default_selectivity(self.default_selectivity)
700+
.with_batch_size(self.batch_size)
701+
.with_fetch(self.fetch)
702+
.build()
689703
}
690704
}
691705

0 commit comments

Comments
 (0)