adriangb commented on code in PR #16642:
URL: https://github.com/apache/datafusion/pull/16642#discussion_r2182744547
##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -520,10 +520,19 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
parent_filters: Vec<Arc<dyn PhysicalExpr>>,
_config: &ConfigOptions,
) -> Result<FilterDescription> {
- Ok(
- FilterDescription::new_with_child_count(self.children().len())
- .all_parent_filters_unsupported(parent_filters),
- )
+ // Default implementation: mark all filters as unsupported for all
children
+ let mut desc = FilterDescription::new();
+ for _child in self.children() {
+ let child_filters = parent_filters
+ .iter()
+ .map(|f| PredicateSupport::Unsupported(Arc::clone(f)))
+ .collect();
+ desc = desc.with_child(ChildFilterDescription {
+ parent_filters: child_filters,
+ self_filters: vec![],
+ });
+ }
Review Comment:
This loop is only happening in one place atm. I refactored it a bit to hoist
code out of the loop. I think for things like this that are only called in 1
place we should err on the side of not adding new public APIs for 1 call site
and wait until people actually implementing these methods on their
ExecutionPlan's and request helper methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
