Skip to content

Conversation

@tianchen92
Copy link
Contributor

Related to 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.

@tianchen92
Copy link
Contributor Author

cc @praveenbingo

final UInt4Vector uInt4Vector = new UInt4Vector("uint4", allocator);
final UInt1Vector uInt1Vector = new UInt1Vector("uint1", allocator)) {

long[] longValues = new long[]{0x8000000000000000L, 0x7fffffffffffffffL, 0xffffffffffffffffL};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long.MIN_VALUE, Long.MAX_VALUE, -1L?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, fixed :)

@codecov-io
Copy link

codecov-io commented Aug 13, 2019

Codecov Report

Merging #5072 into master will increase coverage by 2.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5072      +/-   ##
==========================================
+ Coverage   87.61%   89.71%    +2.1%     
==========================================
  Files        1009      670     -339     
  Lines      144082    99567   -44515     
  Branches     1418        0    -1418     
==========================================
- Hits       126231    89327   -36904     
+ Misses      17489    10240    -7249     
+ Partials      362        0     -362
Impacted Files Coverage Δ
cpp/src/arrow/json/converter.cc 90.05% <0%> (-1.76%) ⬇️
cpp/src/arrow/json/chunked-builder.cc 79.91% <0%> (-1.68%) ⬇️
r/src/recordbatch.cpp
r/R/Table.R
js/src/util/fn.ts
go/arrow/array/bufferbuilder.go
r/src/symbols.cpp
rust/datafusion/src/execution/projection.rs
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
... and 331 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a3eab5...2bdbe7e. Read the comment docs.

Copy link
Contributor

@praveenbingo praveenbingo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM. thanks @tianchen92

pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants