ARROW-14306: [C++][Compute] Add binary reverse kernel#11883
ARROW-14306: [C++][Compute] Add binary reverse kernel#11883johanpel wants to merge 7 commits intoapache:masterfrom
Conversation
|
|
lidavidm
left a comment
There was a problem hiding this comment.
Thanks!
Can we also add this function to https://github.com/apache/arrow/blob/master/docs/source/cpp/compute.rst ? (It should be in the same table as 'ascii_reverse' and 'utf8_reverse'.) We should document that this one does not make any assumptions about the encoding.
|
Oh, and it needs to be listed in https://github.com/apache/arrow/blob/master/docs/source/python/api/compute.rst as well (again alongside its ascii/utf8 counterparts) |
|
Thanks for the comments. I added the docs. I also added a test because the function can be applied to String/LargeString as well. I'm in doubt whether we would want to prevent applying this function to String/LargeString. Use cases would be quite exotic. If we'd like to prevent it I could introduce |
|
Oh, whoops. No, good point: we should not support string types here, we should register only Binary and LargeBinary. For String/LargeString the kernel would need to validate that its output is still valid UTF-8 which would be redundant with string_reverse. |
|
Benchmark runs are scheduled for baseline = 00d5077 and contender = b1f009c. b1f009c is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
No description provided.