-
Notifications
You must be signed in to change notification settings - Fork 3.9k
ARROW-1875: [Java] Write 64-bit ints as strings in integration test JSON files #5002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
It is surprising that this passes, I will need to check it out and run it locally to make sure something funny isn't happening. But then i think it can be merged. |
java/vector/src/main/java/org/apache/arrow/vector/ipc/JsonFileReader.java
Show resolved
Hide resolved
Hi Micah, Anything found? |
…al overflow Related to [ARROW-6218](https://issues.apache.org/jira/browse/ARROW-6218). As per discussion #5002 For UINT type, when write/read json data in integration test, it extend data type(i.e. Long->BigInteger, Int->Long) to avoid potential overflow. Like UINT8 the write side and read side code like this: >case UINT8: generator.writeNumber(UInt8Vector.getNoOverflow(buffer, index)); break; >BigInteger value = parser.getBigIntegerValue(); buf.writeLong(value.longValue()); Should add a test to avoid potential overflow in the data transfer process. Closes #5072 from tianchen92/ARROW-6218 and squashes the following commits: 2bdbe7e <tianchen> use MIN_VALUE and MAX_VALUE 1430979 <tianchen> ARROW-6218: Add UINT type test in integration to avoid potential overflow Authored-by: tianchen <niki.lj@alibaba-inc.com> Signed-off-by: Praveen <praveen@dremio.com>
|
OK, the reason why this doesn't break other languages is the integration tests don't seem to actually use the write path (arrow->json), and the read path (json->arrow) is robust to either strings or numbers. So I think this can be merged. And we can have other languages make similar changes and then change the python script. CC @wesm |
|
+1 thank you. |
…al overflow Related to [ARROW-6218](https://issues.apache.org/jira/browse/ARROW-6218). As per discussion apache#5002 For UINT type, when write/read json data in integration test, it extend data type(i.e. Long->BigInteger, Int->Long) to avoid potential overflow. Like UINT8 the write side and read side code like this: >case UINT8: generator.writeNumber(UInt8Vector.getNoOverflow(buffer, index)); break; >BigInteger value = parser.getBigIntegerValue(); buf.writeLong(value.longValue()); Should add a test to avoid potential overflow in the data transfer process. Closes apache#5072 from tianchen92/ARROW-6218 and squashes the following commits: 2bdbe7e <tianchen> use MIN_VALUE and MAX_VALUE 1430979 <tianchen> ARROW-6218: Add UINT type test in integration to avoid potential overflow Authored-by: tianchen <niki.lj@alibaba-inc.com> Signed-off-by: Praveen <praveen@dremio.com>
…SON files Related to [ARROW-1875](https://issues.apache.org/jira/browse/ARROW-1875). This is Java side implementation. Closes apache#5002 from tianchen92/ARROW-1875 and squashes the following commits: 20cc581 <tianchen> ARROW-1875: Write 64-bit ints as strings in integration test JSON files Authored-by: tianchen <niki.lj@alibaba-inc.com> Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
Related to ARROW-1875.
This is Java side implementation.