alamb commented on code in PR #10459:
URL: https://github.com/apache/datafusion/pull/10459#discussion_r1598942936
##########
datafusion/optimizer/src/single_distinct_to_groupby.rs:
##########
@@ -228,37 +228,18 @@ impl OptimizerRule for SingleDistinctToGroupBy {
.collect::<Result<Vec<_>>>()?;
// construct the inner AggrPlan
Review Comment:
FYI @appletreeisyellow - this PR contains some changes which may conflict
with https://github.com/apache/datafusion/issues/10295
##########
datafusion/sqllogictest/test_files/select.slt:
##########
@@ -1619,7 +1619,16 @@ select count(1) from v;
query I
select a + b from (select 1 as a, 2 as b, 1 as "a + b");
----
-1
+3
Review Comment:
🎉
##########
datafusion/core/tests/dataframe/mod.rs:
##########
@@ -210,7 +210,6 @@ async fn test_count_wildcard_on_aggregate() -> Result<()> {
let sql_results = ctx
.sql("select count(*) from t1")
.await?
- .select(vec![count(wildcard())])?
Review Comment:
I agree that this tests is not a valid.
Perhaps it meant to show how to select the count(*) again -- if so maybe we
could update the tests to do something like this (rather than removing it
entirely):
```
.select(vec![col("COUNT(*)")])?
```
##########
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 have been trying to remove copies from the optimizer (see
https://github.com/apache/datafusion/issues/9637) and this API forces a copy.
I wonder if we could avoid copies by returning references here instead 🤔 I
looked at the above code and it wasn't immediately clear we could do so
```suggestion
fn output_expressions(&self) -> Result<Vec<&Expr>> {
```
I think then you could have like this perhaps
```rust
pub(crate) fn columnized_output_exprs(&self) -> Result<Vec<(&Expr,
Column)>> {
```
--
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]
