You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "farkmarnum (via GitHub)" <gi...@apache.org> on 2023/05/20 21:29:10 UTC

[GitHub] [arrow] farkmarnum commented on issue #35700: [JS] pre-epoch Timestamps are off by a fixed amount

farkmarnum commented on issue #35700:
URL: https://github.com/apache/arrow/issues/35700#issuecomment-1556015654

   It looks like this was introduced by https://github.com/apache/arrow/commit/cde18a6ba525d090d11ac6f3563370dce60d03a1 to resolve [Read timestamp low bits as Uint32s](https://github.com/apache/arrow/pull/1678).
   
   The problematic code is [here](https://github.com/apache/arrow/blob/65520b361941e9abad386614999dbc250295959e/js/src/visitor/get.ts#L100):
   ```javascript
   const epochMillisecondsLongToMs = (data: Int32Array, index: number) =>
     4294967296 * data[index + 1] + (data[index] >>> 0);
   ```
   and related code is [here](https://github.com/apache/arrow/blob/65520b361941e9abad386614999dbc250295959e/js/src/visitor/set.ts#LL104C8-L107C3):
   ```javascript
   const setEpochMsToMillisecondsLong = (data: Int32Array, index: number, epochMs: number) => {
       data[index] = Math.trunc(epochMs % 4294967296);
       data[index + 1] = Math.trunc(epochMs / 4294967296);
   };
   ```
   
   The code is transforming the 64bit `epochMs` value into two 32bit values, and then transforming them back. We can reduce these two steps to a simple test like this:
   ```javascript
   const toInt32sAndBack = (epochMs) => {
     data = [];
     data[0] = Math.trunc(epochMs % 4294967296);
     data[1] = Math.trunc(epochMs / 4294967296);
     return 4294967296 * data[1] + (data[0] >>> 0)
   };
   
   const test = (epochMs) => toInt32sAndBack(epochMs) - epochMs;
   
   test(2_000_000_000_000)
   // 0
   
   test(-2_000_000_000_000)
   // 4294967296
   ```
   
   When `epochMs` is negative, `data[0]` is negative but `data[0] >>> 0` is positive.
   
   So, while https://github.com/apache/arrow/commit/cde18a6ba525d090d11ac6f3563370dce60d03a1 solved the precision issue, it introduced a new issue for pre-epoch timestamps.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org