ump45nose commented on code in PR #1379:
URL: https://github.com/apache/arrow-java/pull/1379#discussion_r4490005183
##########
vector/src/main/codegen/templates/StructWriters.java:
##########
@@ -86,6 +86,13 @@ public class ${mode}StructWriter extends AbstractFieldWriter
{
case EXTENSIONTYPE:
extension(child.getName(), child.getType());
break;
+ case NULL: {
+ // There is no writer for the Null type, but the child vector still
has to exist so that
+ // the container matches its field (e.g. when it is created from a
Field by a TransferPair).
+ FieldType fieldType = new FieldType(addVectorAsNullable,
MinorType.NULL.getType(), null, null);
+ container.addOrGet(child.getName(), fieldType, NullVector.class);
+ break;
+ }
Review Comment:
Done in edf274a: `StructWriters` now passes `child.getFieldType()`, and
`Types.NULL.getNewVector` uses `new NullVector(field)` so nullability and
metadata survive `addOrGet` / `Field.createVector`.
`TestStructVector` (netty + unsafe): 15 tests, 0 failures.
##########
vector/src/test/java/org/apache/arrow/vector/TestStructVector.java:
##########
@@ -379,6 +381,63 @@ public void
testStructVectorTransferPairWithExtensionType() {
toVector.close();
}
+ @Test
+ public void testStructVectorTransferPairWithNullType() {
+ Field nullField = new Field("null_child",
FieldType.nullable(ArrowType.Null.INSTANCE), null);
+ Field structField =
+ new Field("struct", FieldType.nullable(new ArrowType.Struct()),
List.of(nullField));
+
+ try (StructVector s1 = (StructVector) structField.createVector(allocator))
{
+ NullVector nullVector =
+ s1.addOrGet("null_child",
FieldType.nullable(ArrowType.Null.INSTANCE), NullVector.class);
+ s1.allocateNew();
+ s1.setIndexDefined(0);
+ s1.setNull(1);
+ s1.setIndexDefined(2);
+ nullVector.setValueCount(3);
+ s1.setValueCount(3);
+
+ TransferPair tp = s1.getTransferPair(structField, allocator);
+ try (StructVector toVector = (StructVector) tp.getTo()) {
+ assertEquals(s1.getField(), toVector.getField());
+
+ tp.transfer();
+ assertEquals(3, toVector.getValueCount());
+ assertFalse(toVector.isNull(0));
+ assertTrue(toVector.isNull(1));
+ assertFalse(toVector.isNull(2));
+ NullVector toChild = toVector.getChild("null_child", NullVector.class);
+ assertNotNull(toChild);
+ assertEquals(3, toChild.getValueCount());
+ assertTrue(toChild.isNull(0));
+ }
+ }
+ }
+
+ @Test
+ public void testStructVectorFromFieldWithNullChild() {
+ Field nullField = new Field("null_child",
FieldType.nullable(ArrowType.Null.INSTANCE), null);
Review Comment:
Updated both Null-child tests to use a non-nullable Null field with metadata
(`Map.of(\"k\", \"v\")`), so `assertEquals(structField, vector.getField())`
actually exercises FieldType preservation.
##########
vector/src/test/java/org/apache/arrow/vector/TestStructVector.java:
##########
@@ -379,6 +381,63 @@ public void
testStructVectorTransferPairWithExtensionType() {
toVector.close();
}
+ @Test
+ public void testStructVectorTransferPairWithNullType() {
+ Field nullField = new Field("null_child",
FieldType.nullable(ArrowType.Null.INSTANCE), null);
+ Field structField =
+ new Field("struct", FieldType.nullable(new ArrowType.Struct()),
List.of(nullField));
+
+ try (StructVector s1 = (StructVector) structField.createVector(allocator))
{
+ NullVector nullVector =
+ s1.addOrGet("null_child",
FieldType.nullable(ArrowType.Null.INSTANCE), NullVector.class);
+ s1.allocateNew();
+ s1.setIndexDefined(0);
+ s1.setNull(1);
+ s1.setIndexDefined(2);
+ nullVector.setValueCount(3);
+ s1.setValueCount(3);
+
+ TransferPair tp = s1.getTransferPair(structField, allocator);
+ try (StructVector toVector = (StructVector) tp.getTo()) {
+ assertEquals(s1.getField(), toVector.getField());
+
+ tp.transfer();
+ assertEquals(3, toVector.getValueCount());
+ assertFalse(toVector.isNull(0));
+ assertTrue(toVector.isNull(1));
+ assertFalse(toVector.isNull(2));
+ NullVector toChild = toVector.getChild("null_child", NullVector.class);
+ assertNotNull(toChild);
+ assertEquals(3, toChild.getValueCount());
+ assertTrue(toChild.isNull(0));
Review Comment:
Switched to `assertEquals(nullField, toChild.getField())` and
`assertEquals(structField, toVector.getField())` as suggested.
--
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]
