alamb commented on code in PR #9944:
URL: https://github.com/apache/arrow-datafusion/pull/9944#discussion_r1552035289
##########
datafusion/expr/src/utils.rs:
##########
@@ -42,7 +42,7 @@ use sqlparser::ast::{ExceptSelectItem, ExcludeSelectItem,
WildcardAdditionalOpti
/// The value to which `COUNT(*)` is expanded to in
/// `COUNT(<constant>)` expressions
-pub const COUNT_STAR_EXPANSION: ScalarValue = ScalarValue::UInt8(Some(1));
+pub const COUNT_STAR_EXPANSION: ScalarValue = ScalarValue::Int64(Some(1));
Review Comment:
> This does however result in count(1) being auto-named to count(*) now.
FWIW, this is what happens on main with `count(uint8)` (the same thing)
```sql
❯ select count(arrow_cast(1, 'UInt8')) from (values (1));
+----------+
| COUNT(*) |
+----------+
| 1 |
+----------+
1 row(s) fetched.
Elapsed 0.002 seconds.
```
Though admittedly almost no one would actually type `count(arrow_cast(1,
'UInt8'))` so it is likely not a big deal.
> That might be fine as it reduces ambiguity.
I agree having the `COUNT(*)` in the plan actually helps as it makes it
clearer when the fast path is being used.
Let's start with this and if someone else has a different opinion we can
make another PR
--
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]
