ARROW-1237: [JAVA] expose the ability to set lastSet#868
ARROW-1237: [JAVA] expose the ability to set lastSet#868siddharthteotia wants to merge 3 commits intoapache:masterfrom
Conversation
|
Is it possible to add a unit test for this? Can you also indicate in the PR title that this is a Java patch, e.g. |
siddharthteotia
changed the title
|
Edited the PR title to include [JAVA]. |
DremioQA
left a comment
DremioQA
left a comment
There was a problem hiding this comment.
Let's add some unit tests for each version that has last set. Basically, set some data via memory access then update last set, then confirm a correct vector output.
| } | ||
|
|
||
| public void setLastSet(int value) { | ||
| <#if type.major = "VarLen">lastSet = value;</#if> |
There was a problem hiding this comment.
Is there a lastset for the list vector as well?
There was a problem hiding this comment.
Writing the unit tests.
Yes, the ListVector also has lastSet. Will add the API for ListVector as well and unit tests for each vector type; NullableVarCharVector, NullableVarBinaryVector, ListVector.
|
@jacques-n There is a slightly orthogonal test case test based on VectorLoader and VectorUnloader. |
| final NullableVarCharVector.Accessor accessor1 = vector1.getAccessor(); | ||
| assertArrayEquals(STR1, accessor1.get(0)); | ||
| assertArrayEquals(STR2, accessor1.get(1)); | ||
| assertArrayEquals(STR3, accessor1.get(2)); |
There was a problem hiding this comment.
Given the main issue of holes/blanks in the items set, I suggest you modify these tests to set the valuecount to something higher (e.g. 15). Then verify that everything looks good.
There was a problem hiding this comment.
Added additional test and slightly modified existing ones.
The changes here expose the ability to set "lastSet" on Nullable<var length>Vector. I believe this is needed only for NullableVarCharVector and NullableVarBinaryVector. Hence the API is exposed through NullableValueVectors.java Author: siddharth <[email protected]> Closes apache#868 from siddharthteotia/ARROW-1237 and squashes the following commits: 786dfea [siddharth] ARROW-1237: addressed review comments and added more tests 73b2fc5 [siddharth] ARROW-1237: added some unit tests f8c7277 [siddharth] ARROW-1237: expose the ability to set lastSet
4 participants
The changes here expose the ability to set "lastSet" on NullableVector.
I believe this is needed only for NullableVarCharVector and NullableVarBinaryVector. Hence the API is exposed through NullableValueVectors.java