You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/02/26 01:24:24 UTC
[GitHub] [arrow] nbruno opened a new pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
nbruno opened a new pull request #9581:
URL: https://github.com/apache/arrow/pull/9581
This PR fixes a bug where appending an empty list vector fails with a `NullPointerException`. Instead of attempting to process the delta vector, we just return early with the unmodified target vector, since there is nothing to append to the target vector from the delta vector. In addition, it fixes cases where appending an empty delta vector would've caused the same NPE, or where it'd be an optimization to not attempt processing an empty delta vector. Unit tests are added for all supported `VectorAppender` vector types.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9581:
URL: https://github.com/apache/arrow/pull/9581#issuecomment-786344462
https://issues.apache.org/jira/browse/ARROW-11788
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 closed pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
Posted by GitBox <gi...@apache.org>.
liyafan82 closed pull request #9581:
URL: https://github.com/apache/arrow/pull/9581
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #9581:
URL: https://github.com/apache/arrow/pull/9581#discussion_r583528883
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
##########
@@ -95,6 +96,27 @@ public void testAppendFixedWidthVector() {
}
}
+ @Test
+ public void testAppendEmptyFixedWidthVector() {
+ try (IntVector target = new IntVector("", allocator);
+ IntVector delta = new IntVector("", allocator)) {
+
+ target.allocateNew(10);
+ ValueVectorDataPopulator.setVector(target, 0, 1, 2, 3, 4, 5, 6, null, 8, 9);
+
+ VectorAppender appender = new VectorAppender(target);
+ delta.accept(appender, null);
+
+ assertEquals(10, target.getValueCount());
+
+ try (IntVector expected = new IntVector("expected", allocator)) {
+ expected.allocateNew();
Review comment:
This is not needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #9581:
URL: https://github.com/apache/arrow/pull/9581#issuecomment-787626343
@nbruno Thanks for the PR. Merging.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nbruno commented on a change in pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
Posted by GitBox <gi...@apache.org>.
nbruno commented on a change in pull request #9581:
URL: https://github.com/apache/arrow/pull/9581#discussion_r583786671
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
##########
@@ -95,6 +96,27 @@ public void testAppendFixedWidthVector() {
}
}
+ @Test
+ public void testAppendEmptyFixedWidthVector() {
+ try (IntVector target = new IntVector("", allocator);
+ IntVector delta = new IntVector("", allocator)) {
+
+ target.allocateNew(10);
Review comment:
I removed all explicit calls for allocation if the utility method performs the allocation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #9581:
URL: https://github.com/apache/arrow/pull/9581#discussion_r583529700
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
##########
@@ -120,6 +142,26 @@ public void testAppendVariableWidthVector() {
}
}
+ @Test
+ public void testAppendEmptyVariableWidthVector() {
+ try (VarCharVector target = new VarCharVector("", allocator);
+ VarCharVector delta = new VarCharVector("", allocator)) {
+
+ target.allocateNew(5, 10);
Review comment:
This is not needed either, as `ValueVectorDataPopulator.setVector` calls the `getSafe` method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #9581: ARROW-11788: [Java] Fix appending empty delta vectors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #9581:
URL: https://github.com/apache/arrow/pull/9581#discussion_r583528516
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
##########
@@ -95,6 +96,27 @@ public void testAppendFixedWidthVector() {
}
}
+ @Test
+ public void testAppendEmptyFixedWidthVector() {
+ try (IntVector target = new IntVector("", allocator);
+ IntVector delta = new IntVector("", allocator)) {
+
+ target.allocateNew(10);
Review comment:
This is not needed, as `ValueVectorDataPopulator.setVector` will do the allocation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org