fix: Fix precision loss in IntervalMonthDayNano nanosecond

[GeorgeLeePatterson]

Rationale for this change

Integration tests were failing when JavaScript produced IntervalMonthDayNano data that was consumed by C++, Rust, or nanoarrow implementations. The nanosecond values differed by small amounts (e.g., 216 nanoseconds) due to precision loss during BigInt to Number conversion.

Example failure:

• Expected: 6684525287992311000ns
• JS Output: 6684525287992310784ns
• Difference: 216ns

What changes are included in this PR?

Fixed the toIntervalMonthDayNanoInt32Array function in src/util/interval.ts to properly handle unsigned 32-bit integer conversion without precision loss. The issue was that Number(BigInt) conversion for large values (>2^53-1) loses precision. Applied unsigned right shift (>>> 0) to ensure correct unsigned 32-bit representation.

Changed:

// Before (loses precision):
data[ai++] = Number(BigInt(nanoseconds) & BigInt(0xFFFFFFFF));
data[ai++] = Number(BigInt(nanoseconds) >> BigInt(32));

// After (preserves precision):
const ns = BigInt(nanoseconds);
data[ai++] = Number(ns & BigInt(0xFFFFFFFF)) >>> 0;
data[ai++] = Number(ns >> BigInt(32)) >>> 0;

Fixes apache/arrow#46203

Closes #15.

[kou]

Wow! Is this the same problem as #15 ?
#15 (comment) has a data that reproduces the problem. Could you try it?

[GeorgeLeePatterson]

Sure, I'll give it a shot. It should resolve it but I'll confirm. I was pulling my hair out with this issue, hopefully this solution extends. @kou

[GeorgeLeePatterson]

@kou

Yes! After checking it against your comment, this PR does fix issue #15 it looks like.

Both issues stem from the same root cause in toIntervalMonthDayNanoInt32Array(). The function is called by the integration test JSON loader (src/visitor/vectorloader.ts:215), which is where issue #15's test failures occurred. The >>> 0 fix ensures proper unsigned 32-bit representation and should resolve both the integration test failures and the precision loss I encountered.

[Copilot]

Pull Request Overview

This PR improves the handling of nanosecond values in the toIntervalMonthDayNanoInt32Array function by ensuring proper unsigned 32-bit integer conversion. The changes prevent potential precision loss when converting large BigInt nanosecond values to the Int32Array format.

• Extracts the BigInt conversion to a separate variable for clarity
• Applies unsigned right shift (>>> 0) to ensure the results are treated as unsigned 32-bit integers
• Adds clarifying comment about the purpose of the conversion

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kou]

Thanks!

@trxcllnt @domoritz Can we merge this? I'm not sure whether >>> 0 is a right way for this case in JavaScript.

[pitrou]

This does not seem to fix the issue, see #15 (comment)

Also, please add unit tests to exercise the fix, for example in https://github.com/apache/arrow-js/blob/main/test/unit/vector/interval-month-day-nano-tests.ts

[kou]

@GeorgeLeePatterson Could you take a look at the above comment?

#321 (comment)

[GeorgeLeePatterson]

@GeorgeLeePatterson Could you take a look at the above comment?

#321 (comment)

Sure, I can take a look tomorrow. Perhaps it only fixed the integration tests issues in the case of certain randomly chosen values, but wasn't in fact a blanket fix across the underlying problem. I'll see if I can figure this out more broadly.