bkietz commented on a change in pull request #11191:
URL: https://github.com/apache/arrow/pull/11191#discussion_r717904967
##########
File path: cpp/src/arrow/compute/kernels/hash_aggregate.cc
##########
@@ -541,9 +616,27 @@ struct GrouperFastImpl : Grouper {
~GrouperFastImpl() { map_.cleanup(); }
Result<Datum> Consume(const ExecBatch& batch) override {
+ // ARROW-14027: expand scalar arguments for now
Review comment:
Nit: elsewhere we call "materialization of a batch.length constant array
from a scalar" broadcasting, I'd prefer to keep that consistent
```suggestion
// ARROW-14027: broadcast scalar arguments for now
```
##########
File path: cpp/src/arrow/compute/kernels/hash_aggregate.cc
##########
@@ -214,8 +248,12 @@ struct DictionaryKeyEncoder : FixedWidthKeyEncoder {
DictionaryKeyEncoder(std::shared_ptr<DataType> type, MemoryPool* pool)
: FixedWidthKeyEncoder(std::move(type)), pool_(pool) {}
- Status Encode(const ArrayData& data, uint8_t** encoded_bytes) override {
- auto dict = MakeArray(data.dictionary);
+ Status Encode(const Datum& data, int64_t batch_length,
+ uint8_t** encoded_bytes) override {
+ auto dict =
+ data.is_array()
+ ? MakeArray(data.array()->dictionary)
+ :
MakeArray(data.scalar_as<DictionaryScalar>().value.dictionary->data());
Review comment:
```suggestion
: data.scalar_as<DictionaryScalar>().value.dictionary;
```
--
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]
