jonahgao commented on code in PR #10459:
URL: https://github.com/apache/datafusion/pull/10459#discussion_r1599322262
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -2528,6 +2568,14 @@ impl Aggregate {
})
}
+ /// Get the output expressions.
+ fn output_expressions(&self) -> Result<Vec<Expr>> {
Review Comment:
We can't do this now because
[grouping_set_to_exprlist](https://github.com/jonahgao/datafusion/blob/131d1a96e64e84a01b954d7f5658976cbb64e83a/datafusion/expr/src/logical_plan/plan.rs#L2573)
returns a temporary `Vec<Expr>`.
We need to first remove the copies in `grouping_set_to_exprlist`, and also
need to modify
[`exprlist_to_fields`](https://github.com/jonahgao/datafusion/blob/131d1a96e64e84a01b954d7f5658976cbb64e83a/datafusion/expr/src/logical_plan/plan.rs#L2515).
`exprlist_to_fields` is referenced in many places. ~~Maybe we can try to do
this in subsequent PRs.~~
**Update:** Finished in
https://github.com/apache/datafusion/pull/10459/commits/9d67c29768276c89dccd6d93bed3501b43168073.
`exprlist_to_fields` was changed to
```rust
pub fn exprlist_to_fields<'a>(
exprs: impl IntoIterator<Item = &'a Expr>`,
```
so that it can take `Vec<&Expr>` as parameter without breaking other
callers.
--
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]
