askalt commented on code in PR #20009:
URL: https://github.com/apache/datafusion/pull/20009#discussion_r2769493560
##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -722,6 +722,51 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
) -> Option<Arc<dyn ExecutionPlan>> {
None
}
+
+ /// Returns an iterator over a subset of [`PhysicalExpr`]s that are used
by this
+ /// [`ExecutionPlan`].
+ ///
+ /// This method provides a way to inspect the physical expressions within
a node without
+ /// knowing its specific type. These expressions could be predicates in a
filter, projection
+ /// expressions, or join conditions, etc.
+ ///
+ /// The default implementation returns `None`, indicating that the node
either has no physical
+ /// expressions or does not support exposing them.
+ fn physical_expressions<'a>(
+ &'a self,
+ ) -> Option<Box<dyn Iterator<Item = Arc<dyn PhysicalExpr>> + 'a>> {
+ None
+ }
+
+ /// Returns a new `ExecutionPlan` with its physical expressions replaced
by the provided
+ /// `exprs`.
+ ///
+ /// This method is the counterpart to
[`ExecutionPlan::physical_expressions`]. It allows
+ /// transforming the expressions within a node while preserving the node
itself and its
+ /// children.
+ ///
+ /// By default, the behavior of this method is similar to
[`ExecutionPlan::reset_state`] in the
+ /// sense that it also resets the internal state of [`ExecutionPlan`].
+ ///
+ /// # Constraints
+ ///
+ /// * The number of expressions in `params.exprs` must match the number of
expressions returned
+ /// by [`ExecutionPlan::physical_expressions`].
+ /// * The order of expressions in `params.exprs` must match the order they
were yielded by the
+ /// iterator from [`ExecutionPlan::physical_expressions`].
+ ///
+ /// # Returns
+ ///
+ /// * `Ok(Some(new_plan))` if the expressions were successfully replaced.
+ /// * `Ok(None)` if the node does not support replacing its expressions
(the default).
+ /// * `Err` if the number of expressions is incorrect or if another error
occurs during
+ /// replacement.
+ fn with_physical_expressions(
+ &self,
+ _params: ReplacePhysicalExpr,
+ ) -> Result<Option<Arc<dyn ExecutionPlan>>> {
+ Ok(None)
+ }
Review Comment:
> If AggregateExecb::physical_expressions returned something like
vec![expr1, expr2, expr3, expr4], how would you know to which of all those
components above each expression belongs? are the 4 expressions
AggregateFunctionExprs
A layout of the vector returned from `ExecutionPlan::physical_expr(...)` is
plan specific. The API does not provide an ability to modify the layout itself:
it could not be done generically, as each plan has different sorts of
expressions, so if specific one should be modified -- the right way is to
downcast the plan (caller knows which expression exactly is desired to be
modified).
On other hand, this API provides an ability to switch some parts of the
expressions prior the plan will be executed. For example, we are going to
implement placeholders in the plans based on this API, as we can write the code
like:
```rust
fn resolve_placeholders(expr: &Arc<dyn PhysicalExpr) -> Arc<dyn
PhysicalExpr> {
...
}
let resolved_expr = plan.physical_expr().iter().map(|p|
resolve_placeholders(p));
let plan = plan.with_physical_expr(resolved_expr);
plan.execute(...)
```
--
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]
