alamb commented on code in PR #10543:
URL: https://github.com/apache/datafusion/pull/10543#discussion_r1614057653
##########
datafusion/physical-plan/src/work_table.rs:
##########
@@ -169,7 +169,7 @@ impl ExecutionPlan for WorkTableExec {
&self.cache
}
- fn children(&self) -> Vec<Arc<dyn ExecutionPlan>> {
+ fn children(&self) -> Vec<&Arc<dyn ExecutionPlan>> {
Review Comment:
After reading through this I think it is also an API change (as it changes
the API for `ExecutionPlan::children` -- however I think it is for the best.
Though I wonder if we are going to be changing the signature anyways, I
wonder if we should consider something that doesn't require an allocation like
```rust
fn children(&self) -> [&Arc<dyn ExecutionPlan>] {
```
Instead of
```rust
fn children(&self) -> Vec<&Arc<dyn ExecutionPlan>> {
```
🤔
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
