adriangb commented on code in PR #20145:
URL: https://github.com/apache/datafusion/pull/20145#discussion_r2764240477
##########
datafusion/physical-plan/src/union.rs:
##########
@@ -370,6 +376,66 @@ impl ExecutionPlan for UnionExec {
) -> Result<FilterDescription> {
FilterDescription::from_children(parent_filters, &self.children())
}
+
+ fn handle_child_pushdown_result(
+ &self,
+ phase: FilterPushdownPhase,
+ child_pushdown_result: ChildPushdownResult,
+ _config: &ConfigOptions,
+ ) -> Result<FilterPushdownPropagation<Arc<dyn ExecutionPlan>>> {
+ // For non-Pre phase, use default behavior
+ if !matches!(phase, FilterPushdownPhase::Pre) {
+ return
Ok(FilterPushdownPropagation::if_all(child_pushdown_result));
+ }
+
+ // Collect unsupported filters for each child
Review Comment:
In principle we could make `UnionExec` transparent similar to
`CoalesceBatchesExec`.
Help me understand why we are adding the FilterExec here.
My guess is that you are trying to address the case of `Child2` supporting
pushdown but `Child1` not supporting it.
Without this specialized implementation we would get:
```
FilterExec
UnionExec
Child1
Child2
```
i.e. no changes to the plan, not incorrect but we are applying filters to
the output of Child2 that are unnecessary (it already applied these filters)
With this logic we get:
```
UnionExec
FilterExec
Child1
Child2
```
Which skips re-applying filters to the output of Child2.
Is this interpretation correct?
##########
datafusion/physical-plan/src/union.rs:
##########
@@ -370,6 +376,66 @@ impl ExecutionPlan for UnionExec {
) -> Result<FilterDescription> {
FilterDescription::from_children(parent_filters, &self.children())
}
+
+ fn handle_child_pushdown_result(
+ &self,
+ phase: FilterPushdownPhase,
+ child_pushdown_result: ChildPushdownResult,
+ _config: &ConfigOptions,
+ ) -> Result<FilterPushdownPropagation<Arc<dyn ExecutionPlan>>> {
+ // For non-Pre phase, use default behavior
+ if !matches!(phase, FilterPushdownPhase::Pre) {
Review Comment:
Why does the Pre / Post phase need different behavior?
--
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]
