trxcllnt commented on code in PR #438:
URL: https://github.com/apache/arrow-js/pull/438#discussion_r3305230381
##########
src/visitor/vectorloader.ts:
##########
@@ -170,6 +170,24 @@ export class VectorLoader extends Visitor {
return nullCount > 0 && this.readData(type, buffer) || new
Uint8Array(0);
}
protected readOffsets<T extends DataType>(type: T, buffer?: BufferRegion)
{ return this.readData(type, buffer); }
+ // Rebases int64 offsets to start at 0 so downstream `bigIntToNumber`
narrowing
+ // always succeeds for spec-conforming wire input. Conforming producers may
+ // pre-shift offsets on sliced views, putting absolute values past 2^53;
after
+ // rebasing, in-memory offsets are bounded by the child buffer's element
count
+ // (which the JS runtime can allocate anyway). Genuinely too-large wire
inputs
+ // fail honestly at child-buffer allocation rather than at offset
narrowing.
Review Comment:
I don't understand this comment and implementation. If the producer already
rebased the offsets buffer values relative to the slice offset, the first
offset value should always be zero. If it isn't, it seems that's the real bug?
--
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]
