Karakatiza666 commented on code in PR #438:
URL: https://github.com/apache/arrow-js/pull/438#discussion_r3359205367
##########
src/visitor/vectorloader.ts:
##########
@@ -167,6 +170,22 @@ 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); }
+ // Large* types carry int64 offsets. Downstream code narrows each offset
to a JS
+ // number when indexing buffers, which is lossless for any offset that
indexes a
+ // buffer the runtime can actually allocate — so the common case is
returned as a
+ // zero-copy view over the wire bytes, untouched. The exception is a
sliced array
+ // serialized with absolute (non-rebased) offsets, whose values can exceed
+ // Number.MAX_SAFE_INTEGER even when the referenced span is small; only
then do we
+ // rebase to 0 (the one case that requires a copy) so the offsets stay
narrowable.
+ protected readLargeOffsets<T extends DataType>(type: T, buffer?:
BufferRegion) {
+ const offsets = this.readOffsets(type, buffer);
+ const wide: BigInt64Array = toBigInt64Array(offsets);
+ if (wide.length === 0 || wide.at(-1)! <=
BigInt(Number.MAX_SAFE_INTEGER)) {
+ return offsets;
+ }
+ const base = wide[0];
+ return wide.map((value) => value - base);
+ }
Review Comment:
Makes sense. I everted to readOffsets() and removed `readLargeOffsets`. No
widening of readOffsets was needed: it returns the raw offset bytes and
makeData already picks toBigInt64Array for the Large* types, so they still get
true int64 offsets.
--
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]
