fix: read uint64 columns without boxing through BigInteger - #803
Open
jackylee-ch wants to merge 1 commit into
Open
fix: read uint64 columns without boxing through BigInteger#803jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
getObjectNoOverflow returns the unsigned value as a BigInteger, so longValueExact throws ArithmeticException for every uint64 at or above 2^63 and NPEs on a null slot. Read the raw two's-complement long instead, which is the wrap behaviour the class javadoc and LanceArrowUtils already document and what the _rowaddr path in LanceFragmentColumnarBatchScanner already does, and it drops a BigInteger allocation per row.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The primitive read fixes the demonstrated high-bit and null failures while preserving the connector’s existing UInt64-to-LongType contract. It also removes per-row BigInteger allocation, and the focused regression covers scalar and bulk access across the supported Arrow boundaries.
geruh
reviewed
Sep 4, 2026
|
|
||
| final long getLong(int rowId) { | ||
| return accessor.getObjectNoOverflow(rowId).longValueExact(); | ||
| // Read the raw two's-complement long rather than boxing through getObjectNoOverflow(), which |
Collaborator
There was a problem hiding this comment.
nit: this is a bit verbose we can drop or trim to something like:
// Arrow get() throws on a null slot. Spark getLongs calls getLong without isNullAt.
wdyt?
| try (BufferAllocator allocator = new RootAllocator(); | ||
| UInt8Vector vector = (UInt8Vector) field.createVector(allocator)) { | ||
| vector.allocateNew(3); | ||
| // 2^63 and 2^64-1 are ordinary uint64 values. LongType carries them as their |
Collaborator
There was a problem hiding this comment.
nit: im fine with these comments but can drop wdyt?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UInt8Accessor.getLongboxed throughUInt8Vector.getObjectNoOverflow, which returns the unsignedvalue as a
BigInteger.longValueExact()then throwsArithmeticException: BigInteger out of long rangefor every uint64 at or above 2^63, and NPEs on a null slot, instead of wrapping. The classjavadoc,
LanceArrowUtils.scala:102, and the_rowaddrread atLanceFragmentColumnarBatchScanner.java:197all treat uint64 as wrapping intoLongType.Reachable through any uint64 column, including
_rowidand_rowaddr, whichLanceArrowUtils.scala:538maps toArrowType.Int(64, false).Test plan
LanceArrowColumnVectorTestcase: fails on the old accessor withArithmeticException*Vector*/*Blob*/*Update*suites on Spark 4.1/Scala 2.13 and 3.5/Scala 2.12;make lint🤖 Generated with Claude Code