alamb commented on code in PR #6904:
URL: https://github.com/apache/arrow-datafusion/pull/6904#discussion_r1260962339
##########
datafusion/physical-expr/src/aggregate/count.rs:
##########
@@ -76,6 +85,114 @@ impl Count {
}
}
+/// An accumulator to compute the counts of [`PrimitiveArray<T>`].
+/// Stores values as native types, and does overflow checking
+///
+/// Unlike most other accumulators, COUNT never produces NULLs. If no
+/// non-null values are seen in any group the output is 0. Thus, this
+/// accumulator has no additional null or seen filter tracking.
+#[derive(Debug)]
+struct CountGroupsAccumulator {
+ /// Count per group.
+ ///
+ /// Note this is an i64 and not a u64 (or usize) because the
+ /// output type of count is `DataType::Int64`. Thus by using `i64`
Review Comment:
The reason it is Int64 is to align with the logical expression
https://github.com/apache/arrow-datafusion/blob/d6985b0fc1e4362b80beea059857c6bde65b4d04/datafusion/expr/src/aggregate_function.rs#L201-L203
I can't remember why the logical expression is int64 -- maybe it is for
compatibility with some other system. I agree UInt64 would make more sense
--
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]
