haohuaijin commented on code in PR #21070:
URL: https://github.com/apache/datafusion/pull/21070#discussion_r2963795013
##########
datafusion/physical-plan/src/filter.rs:
##########
@@ -695,7 +695,21 @@ impl ExecutionPlan for FilterExec {
let filter_input = Arc::clone(self.input());
let new_predicate = conjunction(unhandled_filters);
let updated_node = if new_predicate.eq(&lit(true)) {
- // FilterExec is no longer needed, but we may need to leave a
projection in place
+ // FilterExec is no longer needed, but we may need to leave a
projection in place.
+ // If this FilterExec had a fetch limit, propagate it to the child.
+ // When the child also has a fetch, use the minimum of both to
preserve
+ // the tighter constraint.
+ let filter_input = if let Some(outer_fetch) = self.fetch {
+ let effective_fetch = match filter_input.fetch() {
+ Some(inner_fetch) => outer_fetch.min(inner_fetch),
+ None => outer_fetch,
+ };
+ filter_input
+ .with_fetch(Some(effective_fetch))
+ .unwrap_or(filter_input)
Review Comment:
this still have the change that lost the fetch, if the child node do not
support the `with_fetch` method.
One idea is that when a child node does not support fetch, we can leave a
FilterExec with both fetch and filter set to true, if the filter have
projection, keep projection as well. However, I'm uncertain if this is the best
approach and think it requires further discussion.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
