-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix column name for COUNT(*) set by AggregateStatistics #7757
Conversation
This fixes the issue of column name COUNT(UInt8) appearing instead Resolves apache#104
The failure in in |
The failure in in cargo test doc seems to happen in CI itself
Indeed -- https://github.com/apache/arrow-datafusion/actions/runs/6434063712/job/17472738458?pr=7757 looks like some infrastructure error
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @qrilka 🙏 this looks great
I also tried it locally:
DataFusion CLI v31.0.0
❯ select count(*) from (select 1);
+----------+
| COUNT(*) |
+----------+
| 1 |
+----------+
1 row in set. Query took 0.007 seconds.
@@ -93,7 +93,7 @@ query TT | |||
EXPLAIN select count(*) from (values ('a', 1, 100), ('a', 2, 150)) as t (c1,c2,c3) | |||
---- | |||
physical_plan | |||
ProjectionExec: expr=[2 as COUNT(UInt8(1))] | |||
ProjectionExec: expr=[2 as COUNT(*)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this covers the output as well.
I restarted the doc test |
This might be the lowest ticket we have had closed in a while 🏆 |
This fixes the issue of column name COUNT(UInt8) appearing instead Resolves #104
This fixes the issue of column name
COUNT(UInt8)
appearing instead ofCOUNT(*)
Which issue does this PR close?
Closes #104.
Rationale for this change
This should make produced column name consistent with the input SQL
What changes are included in this PR?
Are these changes tested?
Tested with
cargo test
Are there any user-facing changes?
Changes column name for this particular case