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 2020/07/08 18:42:07 UTC

[GitHub] [arrow] lidavidm commented on a change in pull request #7290: ARROW-1692: [Java] UnionArray round trip not working

lidavidm commented on a change in pull request #7290:
URL: https://github.com/apache/arrow/pull/7290#discussion_r451749003



##########
File path: java/vector/src/main/codegen/templates/DenseUnionVector.java
##########
@@ -268,11 +270,11 @@ public long getDataBufferAddress() {
 
   @Override
   public long getValidityBufferAddress() {
-    return validityBuffer.memoryAddress();
+    return typeBuffer.memoryAddress();

Review comment:
       It seems `NullVector` throws `UnsupportedOperationException` in this case - should we be consistent with that?

##########
File path: java/vector/src/main/codegen/templates/DenseUnionVector.java
##########
@@ -268,11 +270,11 @@ public long getDataBufferAddress() {
 
   @Override
   public long getValidityBufferAddress() {
-    return validityBuffer.memoryAddress();
+    return typeBuffer.memoryAddress();
   }
 
   @Override
-  public ArrowBuf getValidityBuffer() { return validityBuffer; }
+  public ArrowBuf getValidityBuffer() { return typeBuffer; }

Review comment:
       Ditto here.

##########
File path: java/vector/src/main/codegen/templates/DenseUnionVector.java
##########
@@ -283,7 +285,7 @@ public long getValidityBufferAddress() {
   public ArrowBuf getDataBuffer() { throw new UnsupportedOperationException(); }
 
   public StructVector getStruct(byte typeId) {
-    StructVector structVector = (StructVector) childVectors[typeId];
+    StructVector structVector = typeId < 0 ? null : (StructVector) childVectors[typeId];

Review comment:
       Should negative type IDs throw an explicit error?

##########
File path: java/vector/src/main/codegen/templates/DenseUnionVector.java
##########
@@ -812,12 +757,17 @@ public int getValueCount() {
   }
 
   public boolean isNull(int index) {

Review comment:
       This is inconsistent with getNullCount; this vector has nullCount == 0 but can have isNull return true regardless. C++ seems to always return false when there is no validity buffer.

##########
File path: java/vector/src/main/codegen/templates/DenseUnionVector.java
##########
@@ -283,7 +285,7 @@ public long getValidityBufferAddress() {
   public ArrowBuf getDataBuffer() { throw new UnsupportedOperationException(); }
 
   public StructVector getStruct(byte typeId) {
-    StructVector structVector = (StructVector) childVectors[typeId];
+    StructVector structVector = typeId < 0 ? null : (StructVector) childVectors[typeId];

Review comment:
       Ah, negative IDs are used as a sentinel for not yet allocated...




----------------------------------------------------------------
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