[GitHub] [arrow] bkietz commented on a change in pull request #11191: ARROW-14027: [C++] Handle scalars in Grouper

[GitBox <gi...@apache.org>]

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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org