fix: correct count(*) alias #7081
Conversation
github-actions
bot
added
optimizer
| ------Projection: COUNT(*) + Int64(2) AS cnt_plus_2, t2.t2_int | ||
| --------Filter: COUNT(*) = Int64(0) | ||
| ----------Aggregate: groupBy=[[t2.t2_int]], aggr=[[COUNT(UInt8(1)) AS COUNT(*)]] | ||
| ------------TableScan: t2 projection=[t2_int] |
There was a problem hiding this comment.
cc @mingmwang
look like old plan is wrong?
I'm not sure whether new plan is right.
There was a problem hiding this comment.
If you are referring to COUNT(*) I think it should always be 0
jackwener
force-pushed
the
count_name
branch
2 times, most recently
from
162b178 to
4501968
Compare
jackwener
force-pushed
the
count_name
branch
3 times, most recently
from
66a006f to
4994970
Compare
There was a problem hiding this comment.
This looks great @jackwener except for the subquery plan changes.
Perhaps this code needs some adjustment: https://github.com/apache/arrow-datafusion/blob/main/datafusion/optimizer/src/decorrelate.rs#L371-L406
| "+--------------+--------------+-----------------+", | ||
| "| 10 | 110 | 20 |", | ||
| "+--------------+--------------+-----------------+", | ||
| "+--------------+--------------+----------+", |
There was a problem hiding this comment.
that is certainly much nicer ❤️
| ------Projection: COUNT(*) + Int64(2) AS cnt_plus_2, t2.t2_int | ||
| --------Filter: COUNT(*) = Int64(0) | ||
| ----------Aggregate: groupBy=[[t2.t2_int]], aggr=[[COUNT(UInt8(1)) AS COUNT(*)]] | ||
| ------------TableScan: t2 projection=[t2_int] |
There was a problem hiding this comment.
If you are referring to COUNT(*) I think it should always be 0
Agree with it.
Thanks ! |
|
cc @jiangzhx |
There was a problem hiding this comment.
This looks awesome @jackwener -- thank you.
I think it is the mark of an excellent engineer when bugs are fixed by removing code. Very well done 🏆
There was a problem hiding this comment.
The plans in this file look much better
| use std::sync::Arc; | ||
|
|
||
| use crate::analyzer::AnalyzerRule; | ||
|
|
||
| pub const COUNT_STAR: &str = "COUNT(*)"; |
Thanks for your help, @jackwener. Great job! I'm your big fan!! |
Me too! |
Which issue does this PR close?
Closes #6447.
Rationale for this change
What changes are included in this PR?
after replace COUNT(*), we should add a alias to keep name unchanged.
Are these changes tested?
Yes
Are there any user-facing changes?