Repository navigation
GH-32946: [Go] Implement REE Array and Compare - #14111
Conversation
|
CC @zagto |
|
|
24f1850 to
dd8a8b2
Compare
zeroshade
changed the title
dd8a8b2 to
726a909
Compare
lidavidm
left a comment
lidavidm
left a comment
There was a problem hiding this comment.
Generally LGTM, though I need to look at that merged runs helper more closely
lidavidm
left a comment
lidavidm
left a comment
There was a problem hiding this comment.
nit: PR title needs updating to the GitHub issue
| case arrow.INT16: | ||
| runEnds := arrow.Int16Traits.CastFromBytes(data.Buffers()[1].Bytes()) | ||
| runEnds = runEnds[data.Offset() : data.Offset()+data.Len()] | ||
| return sort.Search(len(runEnds), func(i int) bool { return runEnds[i] > int16(logicalOffset) }) |
There was a problem hiding this comment.
Hmm. Should we explicitly check for overflow? I suppose that's better left to a validation function (to ensure the array length isn't inconsistent).
I suppose more interestingly, a REE array with 16-bit run ends can only store 32767 elements.
There was a problem hiding this comment.
yea, I probably wouldn't check for overflow here. Currently the Array types don't have an equivalent Validation method like the C++ version does, it's on my list of stuff to do but it's fairly low in priority. Right now it's kinda left to the consumer to ensure they use the correct type to not worry about overflows.
zeroshade
changed the title
|
|
|
If there's no further comments before EOD today i'll merge this. |
|
Benchmark runs are scheduled for baseline = 7579dbd and contender = 9a13734. 9a13734 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Uh oh!
There was an error while loading. Please reload this page.