Karakatiza666 commented on code in PR #438:
URL: https://github.com/apache/arrow-js/pull/438#discussion_r3357723759
##########
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:
Claude said the same thing; it sounded like it's more of a concern with
manually, in-memory generated data. This fallback sounded to me like a low-cost
defensive measure; LMK if you'd rather have it fail loudly to avoid adding the
fallback logic.
--
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]
