dmitry-chirkov-dremio commented on code in PR #55253:
URL: https://github.com/apache/arrow/pull/55253#discussion_r4373036066


##########
cpp/src/arrow/compute/kernels/vector_hash.cc:
##########
@@ -513,15 +515,16 @@ class DictionaryHashKernel : public HashKernel {
       return first_dictionary_;
     }
 
-    auto out_dict_type = first_dictionary_->type();
+    // The output keeps the input index type, so the unified dictionary must 
fit in it
     std::shared_ptr<Array> out_dict;
-    RETURN_NOT_OK(dictionary_unifier_->GetResult(&out_dict_type, &out_dict));
+    RETURN_NOT_OK(dictionary_unifier_->GetResultWithIndexType(index_type_, 
&out_dict));

Review Comment:
   128 dictionary entries fit in `int8`: valid indices are `0..127`. With two 
disjoint 64-entry dictionaries, both functions succeed on the parent commit but 
now return `Invalid`.
   
   This helper checks the dictionary length instead of the highest index, right?



-- 
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]

Reply via email to