llama90 commented on code in PR #38147:
URL: https://github.com/apache/arrow/pull/38147#discussion_r1358489977
##########
cpp/src/arrow/compute/light_array.h:
##########
@@ -183,6 +183,36 @@ class ARROW_EXPORT KeyColumnArray {
// Starting bit offset within the first byte (between 0 and 7)
// to be used when accessing buffers that store bit vectors.
int bit_offset_[kMaxBuffers - 1];
+
+ bool is_dictionary_type() const {
Review Comment:
Maybe, you are right.
My understanding is that the code is added, but probably unsupported or not
being tested properly.
So, I took a hint from what you said, I removed the code for the dictionary
type within `KeyColumnArray` and ran the entire test and found that none of the
tests covered that code :O
(If that's okay, can we add a **new issue** for `KeyColumns` to have complex
values like dictionary types?)
Looking at the data I was randomly generating in the
`hash_join_node_test.cc` file, I was only generating data for the following
types.
In the code below, I have added support for `LargeString` type.
https://github.com/apache/arrow/blob/e9aac8a4ce4b8833667efe52957fb0881a488c4b/cpp/src/arrow/acero/hash_join_node_test.cc#L284-L291
--
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]
