Skip to content

ARROW-6200: [Java] Method getBufferSizeFor in BaseRepeatedValueVector/ListVector not correct#5060

Closed
tianchen92 wants to merge 2 commits into
apache:masterfrom
tianchen92:ARROW-6200
Closed

ARROW-6200: [Java] Method getBufferSizeFor in BaseRepeatedValueVector/ListVector not correct#5060
tianchen92 wants to merge 2 commits into
apache:masterfrom
tianchen92:ARROW-6200

Conversation

@tianchen92
Copy link
Copy Markdown
Contributor

Related to ARROW-6200.

Currently, getBufferSizeFor in BaseRepeatedValueVector implemented as below:
if (valueCount == 0) {
return 0;
}
return ((valueCount + 1) * OFFSET_WIDTH) + vector.getBufferSizeFor(valueCount);

Here vector.getBufferSizeFor(valueCount) seems not right which should be

int innerVectorValueCount = offsetBuffer.getInt(valueCount * OFFSET_WIDTH);
vector.getBufferSizeFor(innerVectorValueCount)

ListVector has the same problem.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 11, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5060      +/-   ##
==========================================
+ Coverage   88.58%   89.71%   +1.13%     
==========================================
  Files         805      670     -135     
  Lines       94407    99383    +4976     
  Branches     1418        0    -1418     
==========================================
+ Hits        83627    89158    +5531     
+ Misses      10418    10225     -193     
+ Partials      362        0     -362
Impacted Files Coverage Δ
r/src/recordbatch.cpp
r/R/Table.R
go/arrow/math/uint64_amd64.go
r/src/symbols.cpp
r/R/Column.R
js/src/builder/interval.ts
go/arrow/ipc/file_reader.go
js/src/builder/index.ts
r/src/arrow_types.h
js/src/enum.ts
... and 428 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 34dd3ed...f0bdbfb. Read the comment docs.

if (valueCount == 0) {
return 0;
}
final int offsetBufferSize = (valueCount + 1) * OFFSET_WIDTH;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reuse code from parent ?

return super.getBufferSizeFor(valueCount) + validityBufferSize;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, fixed. Thanks

@pravindra pravindra closed this in 6381043 Aug 12, 2019
@pravindra
Copy link
Copy Markdown
Contributor

thanks @liyafan82 for the change.

pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
…/ListVector not correct

Related to ARROW-6200.

>Currently, getBufferSizeFor in BaseRepeatedValueVector implemented as below:
if (valueCount == 0) {
  return 0;
}
return ((valueCount + 1) * OFFSET_WIDTH) + vector.getBufferSizeFor(valueCount);

Here vector.getBufferSizeFor(valueCount) seems not right which should be

>int innerVectorValueCount = offsetBuffer.getInt(valueCount * OFFSET_WIDTH);
vector.getBufferSizeFor(innerVectorValueCount)

 ListVector has the same problem.

Closes apache#5060 from tianchen92/ARROW-6200 and squashes the following commits:

f0bdbfb <tianchen> reuse code
90900e7 <tianchen> ARROW-6200:  Method getBufferSizeFor in BaseRepeatedValueVector/ListVector not correct

Authored-by: tianchen <niki.lj@alibaba-inc.com>
Signed-off-by: Pindikura Ravindra <ravindra@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.

3 participants