felipecrv commented on code in PR #40608:
URL: https://github.com/apache/arrow/pull/40608#discussion_r1527534913
##########
cpp/src/arrow/compute/kernels/hash_aggregate.cc:
##########
@@ -3454,8 +3454,8 @@ void RegisterHashAggregateBasic(FunctionRegistry*
registry) {
}
{
- auto func = std::make_shared<HashAggregateFunction>("hash_count_all",
Arity::Unary(),
- hash_count_all_doc,
NULLPTR);
+ auto func = std::make_shared<HashAggregateFunction>(
+ "hash_count_all", Arity::Nullary(), hash_count_all_doc, NULLPTR);
DCHECK_OK(func->AddKernel(MakeUnaryKernel(HashAggregateInit<GroupedCountAllImpl>)));
Review Comment:
I'm actually undoing this code change and keeping only the documentation
change. All aggregation functions are documented to have one less arity than
what is in the implementation (eg: `hash_count_distinct` is declared `Binary`
in the code, but users only have to pass one argument, so it's documented as
`Unary`).
`hash_count_all` is correctly configured as `Arity::Unary()`, but users pass
unary-1 arguments, so it should be `Nullary` in the documentation.
(I worked on n-ary (0 and 1+) aggregate functions in #15083 more than a year
ago)
--
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]
