Skip to content

ARROW-6021: [Java] Extract copyFrom and copyFromSafe methods to ValueVector interface#4931

Closed
liyafan82 wants to merge 1 commit intoapache:masterfrom
liyafan82:fly_0724_copy
Closed

ARROW-6021: [Java] Extract copyFrom and copyFromSafe methods to ValueVector interface#4931
liyafan82 wants to merge 1 commit intoapache:masterfrom
liyafan82:fly_0724_copy

Conversation

@liyafan82
Copy link
Copy Markdown
Contributor

Currently we have copyFrom and copyFromSafe methods in fixed-width and variable-width vectors. Extracting them to the common super interface will make it much more convenient to use them, and avoid unnecessary if-else statements.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 24, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4931      +/-   ##
==========================================
+ Coverage    87.5%   89.64%   +2.13%     
==========================================
  Files         998      664     -334     
  Lines      142085    98021   -44064     
  Branches     1418        0    -1418     
==========================================
- Hits       124336    87872   -36464     
+ Misses      17387    10149    -7238     
+ Partials      362        0     -362
Impacted Files Coverage Δ
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
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
... and 327 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 b071f6b...9cd2596. Read the comment docs.

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.

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.

Nice catch. Thank you @pravindra

Comment thread java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java Outdated
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.

things can go bad if the caller passes a fixed-width vector in the 'from'.

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.

Good question.
Things will go wrong is the source and target are of different vector types. Even if both are fixed-width vectors, things can go wrong (e.g. from a IntVector to a Float4Vector).

One solution is to check types each time, however, this may have performance penalty, as copyFrom is a frequently called operation. So current assumption is that the caller should make sure the source and target are of the same type.

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.

can we add something cheap, like checking the minorType ?

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.

Good suggestion. Thank you @pravindra

@pravindra pravindra closed this in 25efd82 Aug 1, 2019
@pravindra
Copy link
Copy Markdown
Contributor

thanks for the change, @liyafan82

pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
…Vector interface

Currently we have copyFrom and copyFromSafe methods in fixed-width and variable-width vectors. Extracting them to the common super interface will make it much more convenient to use them, and avoid unnecessary if-else statements.

Closes apache#4931 from liyafan82/fly_0724_copy and squashes the following commits:

1ce95bb <liyafan82>  Extract copyFrom and copyFromSafe methods to ValueVector interface

Authored-by: liyafan82 <fan_li_ya@foxmail.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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants