-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-5973: [Java] Variable width vectors' get methods should return null when the underlying data is null #4901
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
acefdb4 to
857d3fb
Compare
Codecov Report
@@ Coverage Diff @@
## master #4901 +/- ##
==========================================
+ Coverage 87.31% 89.58% +2.27%
==========================================
Files 894 661 -233
Lines 134066 96617 -37449
==========================================
- Hits 117055 86553 -30502
+ Misses 16659 10064 -6595
+ Partials 352 0 -352Continue to review full report at Codecov.
|
|
This seems like the right functionality. It looks like it impacts FixedSizeBinary as well (https://github.com/apache/arrow/blob/a222c7d4ddfa43ee35a6ecf1c1563b48ecd6b860/java/vector/src/main/java/org/apache/arrow/vector/FixedSizeBinaryVector.java) @praveenbingo @pravindra any concerns with the change in functionality? |
857d3fb to
6796f94
Compare
@emkornfield Revised accordingly. Thanks a lot for your kind reminder. |
…null when the underlying data is null
6796f94 to
8fe83f7
Compare
pravindra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
+1, we should start a list someplace so we don't forget for the next release behavior changes like this. So far I think we have:
|
Good suggestion. Let's start a list in the mailing list. |
…null when the underlying data is null For variable-width vectors (VarCharVector and VarBinaryVector), when the validity bit is not set, it means the underlying data is null, so the get method should return null. However, the current implementation throws an IllegalStateException when NULL_CHECKING_ENABLED is set, or returns an empty array when the flag is clear. Maybe the purpose of this design is to be consistent with fixed-width vectors. However, the scenario is different: fixed-width vectors (e.g. IntVector) throw an IllegalStateException, simply because the primitive types are non-nullable. Author: liyafan82 <fan_li_ya@foxmail.com> Closes #4901 from liyafan82/fly_0717_varget and squashes the following commits: 8fe83f7 <liyafan82> Variable width vectors' get methods should return null when the underlying data is null
…null when the underlying data is null For variable-width vectors (VarCharVector and VarBinaryVector), when the validity bit is not set, it means the underlying data is null, so the get method should return null. However, the current implementation throws an IllegalStateException when NULL_CHECKING_ENABLED is set, or returns an empty array when the flag is clear. Maybe the purpose of this design is to be consistent with fixed-width vectors. However, the scenario is different: fixed-width vectors (e.g. IntVector) throw an IllegalStateException, simply because the primitive types are non-nullable. Author: liyafan82 <fan_li_ya@foxmail.com> Closes apache#4901 from liyafan82/fly_0717_varget and squashes the following commits: 8fe83f7 <liyafan82> Variable width vectors' get methods should return null when the underlying data is null
For variable-width vectors (VarCharVector and VarBinaryVector), when the validity bit is not set, it means the underlying data is null, so the get method should return null.
However, the current implementation throws an IllegalStateException when NULL_CHECKING_ENABLED is set, or returns an empty array when the flag is clear.
Maybe the purpose of this design is to be consistent with fixed-width vectors. However, the scenario is different: fixed-width vectors (e.g. IntVector) throw an IllegalStateException, simply because the primitive types are non-nullable.