Repository navigation
GH-32947: [Go] Implement Concatenate for REE Array - #14126
Conversation
|
CC @zagto |
fb90e87 to
aa29f37
Compare
bccb88f to
230ff03
Compare
zeroshade
changed the title
|
|
lidavidm
left a comment
lidavidm
left a comment
There was a problem hiding this comment.
nit: PR/Issue should be "REE" array right?
|
|
||
| physicalLength, overflow = addOvf(physicalLength, int(plen)) | ||
| if overflow { | ||
| return nil, fmt.Errorf("%w: run length encoded array length must fit into a 32-bit signed integer", |
There was a problem hiding this comment.
Hmm, shouldn't this depend on the offset type?
There was a problem hiding this comment.
I suppose there are separate checks below, and since lengths are limited to 32-bit...
There was a problem hiding this comment.
so, this is more of a short circuit. We have to add up all of the physical lengths so we know the size to allocate for the full concatenated array. If we end up overflowing when adding up the physical length we don't have to bother calculating the new run ends, we know we're going to overflow no matter what the offset type is. So this doesn't need to depend on the offset type.
There was a problem hiding this comment.
Fair enough. I suppose you could short circuit here earlier for int16, but as long as it properly fails either way.
zeroshade
changed the title
|
I'll merge this at EOD if no one else has comments |
|
Benchmark runs are scheduled for baseline = 4a1229c and contender = 3b21e0c. 3b21e0c 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.