askalt commented on issue #20899:
URL: https://github.com/apache/datafusion/issues/20899#issuecomment-4073514043
> Maybe it makes semse to make recompute_properties part of the
executionPlan trait (probably optional to implement)?
Do not fully understand how this will help `map_expressions` implementation
to recompute them. Each implementation could have its own
`PlanStruct::recompute_properties` and call it (it's not necessary to make it
as the trait method).
There is an alternative, add `ExecutionPlan::recompute_properties` and force
to implement it and `map_expressions_preserve_properties`. Then we can define:
```rust
pub fn map_expressions(...) -> Result<Arc<dyn ExecutionPlan>> {
let mapped = self.map_expressions_preserve_properties(...);
mapped.recompute_properties()
}
```
Please note that `recompute_properties` currently cannot be expressed as
just `plan.with_new_children(plan.children())`, as there is an
[optimization](https://github.com/apache/datafusion/blob/d2278a90b4543939cefb0f3ffbea8b025fe922f0/datafusion/physical-plan/src/execution_plan.rs#L1528-L1542)
that does not recompute properties if children properties stay the same.
--
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]
