adriangb commented on code in PR #15770:
URL: https://github.com/apache/datafusion/pull/15770#discussion_r2145900061
##########
datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs:
##########
@@ -114,6 +118,18 @@ fn pushdown_sorts_helper(
sort_push_down.data.fetch = fetch;
sort_push_down.data.ordering_requirement =
Some(OrderingRequirements::from(sort_ordering));
+ let filter = plan
+ .as_any()
+ .downcast_ref::<SortExec>()
+ .and_then(|s| s.filter().clone());
Review Comment:
We kind of already have that for the filter pushdown. The issue is that so
that `SortExec` / `TopK` can call `DynamicFilterPhysicalExpr::update` they have
to know it's a concrete `DynamicFilterPhysicalExpr` and not an arbitrary
`Arc<dyn PhysicalExpr>`, but the existing methods all deal with `Arc<dyn
PhysicalExpr>`.
Fundamentally what makes all of this code weird / scary to me is that we are
taking data from a typed thing (`SortExec`) moving into an untyped world (a
tree of `ExecutionPlan`) and then trying to re-create the typed thing restoring
all data (`filter/limit`).
--
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]
