ARROW-5884: [Java] Fix the get method of StructVector #4831
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4831 +/- ##
=========================================
+ Coverage 87.44% 89.6% +2.16%
=========================================
Files 997 662 -335
Lines 139804 96427 -43377
Branches 1418 0 -1418
=========================================
- Hits 122246 86403 -35843
+ Misses 17196 10024 -7172
+ Partials 362 0 -362Continue to review full report at Codecov.
|
| @@ -482,6 +482,9 @@ public Object getObject(int index) { | |||
| @Override | |||
| public void get(int index, ComplexHolder holder) { | |||
| holder.isSet = isSet(index); | |||
| if (holder.isSet == 0) { | |||
| return; | |||
There was a problem hiding this comment.
should we null out reader here? Is this actually a bug? it seems different then #4543 since i doesn't look like an exception will be thrown by calling super.get below?
There was a problem hiding this comment.
Good suggestion. It should be nulled out.
As you have observed, it is different from #4543 as no exception is thrown. However, it is just useless to retrieve the reader by calling super.get.
There was a problem hiding this comment.
Agreed, I just don't know if there are some (bad) assumptions being made downstream by code someplace. CC @praveenbingo @pravindra if you don't have any objections please go ahead and merge (or I will merge tomorrow if I don't here any)
|
+1, LGTM |
When the data at the specified location is null, there is no need to call the method from super to set the reader holder.isSet = isSet(index); super.get(index, holder); Author: liyafan82 <fan_li_ya@foxmail.com> Closes #4831 from liyafan82/fly_0709_strnull and squashes the following commits: 0b8f798 <liyafan82> Force the reader to be null 49148aa <liyafan82> Fix the get method of StructVector
When the data at the specified location is null, there is no need to call the method from super to set the reader holder.isSet = isSet(index); super.get(index, holder); Author: liyafan82 <fan_li_ya@foxmail.com> Closes apache#4831 from liyafan82/fly_0709_strnull and squashes the following commits: 0b8f798 <liyafan82> Force the reader to be null 49148aa <liyafan82> Fix the get method of StructVector
When the data at the specified location is null, there is no need to call the method from super to set the reader
holder.isSet = isSet(index);
super.get(index, holder);