lwhite1 commented on code in PR #14244:
URL: https://github.com/apache/arrow/pull/14244#discussion_r983707492
##########
java/vector/src/main/codegen/templates/DenseUnionVector.java:
##########
@@ -940,4 +940,13 @@ public void setInitialCapacity(int valueCount, double
density) {
}
}
}
+
+ /**
+ * Set the element at the given index to null. For DenseUnionVector, it is a
no-op as nulls are not supported at the
+ * top level, and isNull() always returns false. Nullability is only handled
at the level of individual child vectors.
Review Comment:
I think UnsupportedOperation is the best solution (and would have been
correct in FieldWriter). I made that change to the code.
There seem to be at least a couple potential issues with delegation from an
API design standpoint. One is that if you only update one vector, you're
effectively choosing the one at random (if you pick the first, it may already
be null), and the user will get inconsistent results if they check for
nullability in the child vectors. Also, you will have made a change to the
data, but the naive test of the effects of the change (calling isNull()) will
report the same result as before the change.
> For dense union vectors, only one would need a value, and an offset would
be appended.
This suggests a second issue. If there's only one child vector with a value
before you setNull(), then none will have a value after you do. I'm not sure
the result would be but it seems like it breaks the expectation.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
