Skip to content

GH-830: Create Null children when building a struct writer from a Field #1379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ump45nose wants to merge 4 commits into
base: main
Choose a base branch
from
Open

GH-830: Create Null children when building a struct writer from a Field #1379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
743b7d7
GH-830: Create Null children when building a struct writer from a Field
ump45nose Oct 21, 2026
edf274a
GH-830: Preserve Null child FieldType in struct writers
ump45nose Nov 6, 2026
6e9d1a3
GH-830: Apply spotless formatting to TestStructVector
ump45nose Nov 6, 2026
8fb9a80
GH-830: Keep the Field in NullVector transfer pairs
ump45nose Nov 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions vector/src/main/codegen/templates/StructWriters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ 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).
container.addOrGet(child.getName(), child.getFieldType(), NullVector.class);
break;
}
Comment on lines +89 to +94

@jbonofre jbonofre Nov 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addVectorAsNullable is lost further down: addOrGet -> FieldType.createNewSingleVector -> MinorType.NULL.getNewVector, which returns new NullVector(field.getName()). That always gives a nullable field with no metadata. So for a child like Field("n", FieldType.notNullable(ArrowType.Null.INSTANCE), null), the struct field doesn't match the input field.

Could we change Types.java (line 167) to return new NullVector(field); and pass the child own FieldType here? That keeps both nullability and metadata. It also fixes Field.createVector, which goes through the same getNewVector.

Suggested change
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;
}
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).
container.addOrGet(child.getName(), child.getFieldType(), NullVector.class);
break;

@ump45nose ump45nose Nov 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

case UNION:
FieldType fieldType = new FieldType(addVectorAsNullable, MinorType.UNION.getType(), null, null);
UnionWriter writer = new UnionWriter(container.addOrGet(child.getName(), fieldType, UnionVector.class), getNullableStructWriterFactory());
Expand Down
8 changes: 2 additions & 6 deletions vector/src/main/java/org/apache/arrow/vector/NullVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,12 @@ public int getValueCapacity() {

@Override
public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
return new TransferImpl(ref);
return new TransferImpl(new NullVector(ref, field.getFieldType()));
}

@Override
public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
return new TransferImpl(field.getName());
return new TransferImpl(new NullVector(field));
}

@Override
Expand Down Expand Up @@ -337,10 +337,6 @@ public String getName() {
private class TransferImpl implements TransferPair {
NullVector to;

public TransferImpl(String ref) {
to = new NullVector(ref);
}

@Deprecated
public TransferImpl() {
to = new NullVector();
Expand Down
2 changes: 1 addition & 1 deletion vector/src/main/java/org/apache/arrow/vector/types/Types.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public enum MinorType {
@Override
public FieldVector getNewVector(
Field field, BufferAllocator allocator, CallBack schemaChangeCallback) {
return new NullVector(field.getName());
return new NullVector(field);

@jbonofre jbonofre Nov 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes a NullVector created from a schema keep its nullability. But NullVector transfer pairs still create a plain nullable vector (TransferImpl(String ref) -> new NullVector(ref), and getTransferPair(Field, ...) only uses field.getName()). So the source and target of a transfer now have different fields.

That breaks code that worked before. VectorSchemaRoot.slice() uses getTransferPair(allocator), and VectorSchemaRootAppender compares nullability, so this now throws IllegalArgumentException: Vector schema roots have different schemas.:

Schema schema = new Schema(List.of(
    new Field("n", FieldType.notNullable(ArrowType.Null.INSTANCE), null)));
try (VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator);
     VectorSchemaRoot target = VectorSchemaRoot.create(schema, allocator)) {
  root.setRowCount(3);
  try (VectorSchemaRoot slice = root.slice(0, 2)) {
    VectorSchemaRootAppender.append(target, slice);
  }
}

Before this PR, both sides were nullable, so this worked.

Could you also update NullVector so its transfer pairs keep the field? Something like:

@Override
public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
  return new TransferImpl(new NullVector(ref, field.getFieldType()));
}
@Override
public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
  return new TransferImpl(new NullVector(field));
}

@ump45nose ump45nose Nov 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. In 8fb9a80 NullVector.getTransferPair(String, ...) now builds the target with the source FieldType, and getTransferPair(Field, ...) uses the given Field, so slice() keeps nullability and metadata. I added your scenario as TestVectorSchemaRootAppender#testAppendSliceWithNonNullableNullVector, plus TestSplitAndTransfer#testNullVectorTransferPairKeepsField for the three transfer-pair entry points. Both fail without the change, and the full vector module tests pass locally.

}

@Override
Expand Down
16 changes: 16 additions & 0 deletions vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,22 @@ public void testWithNullVector() {
// no allocations to clear for NullVector
}

@Test
public void testNullVectorTransferPairKeepsField() {
Field field =
new Field("n", new FieldType(false, ArrowType.Null.INSTANCE, null, Map.of("k", "v")), null);
NullVector fromNullVector = new NullVector(field);

assertEquals(field, fromNullVector.getTransferPair(allocator).getTo().getField());
assertEquals(
new Field("renamed", field.getFieldType(), null),
fromNullVector.getTransferPair("renamed", allocator).getTo().getField());

Field targetField = new Field("target", FieldType.nullable(ArrowType.Null.INSTANCE), null);
assertEquals(
targetField, fromNullVector.getTransferPair(targetField, allocator).getTo().getField());
}

@Test
public void testWithZeroVector() {
ZeroVector fromZeroVector = new ZeroVector("zeroVector");
Expand Down
66 changes: 66 additions & 0 deletions vector/src/test/java/org/apache/arrow/vector/TestStructVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
package org.apache.arrow.vector;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -379,6 +381,70 @@ public void testStructVectorTransferPairWithExtensionType() {
toVector.close();
}

@Test
public void testStructVectorTransferPairWithNullType() {
Field nullField =
new Field(
"null_child",
new FieldType(false, ArrowType.Null.INSTANCE, null, Map.of("k", "v")),
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", nullField.getFieldType(), 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(structField, 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());
assertEquals(nullField, toChild.getField());
}
}
}

@Test
public void testStructVectorFromFieldWithNullChild() {
Field nullField =
new Field(
"null_child",
new FieldType(false, ArrowType.Null.INSTANCE, null, Map.of("k", "v")),
null);
Field intField = new Field("int_child", FieldType.nullable(MinorType.INT.getType()), null);
Field structField =
new Field(
"struct", FieldType.nullable(new ArrowType.Struct()), List.of(nullField, intField));

try (StructVector vector = new StructVector(structField, allocator, null)) {
assertEquals(structField, vector.getField());
assertEquals(2, vector.size());
assertNotNull(vector.getChild("null_child", NullVector.class));

// The writer still works for the other children.
NullableStructWriter writer = vector.getWriter();
writer.setPosition(0);
writer.start();
writer.integer("int_child").writeInt(42);
writer.end();
vector.setValueCount(1);
assertEquals(42, vector.getChild("int_child", IntVector.class).get(0));
}
}

private StructVector simpleStructVector(String name, BufferAllocator allocator) {
final String INT_COL = "struct_int_child";
final String FLT_COL = "struct_flt_child";
Expand Down
21 changes: 21 additions & 0 deletions vector/src/test/java/org/apache/arrow/vector/util/TestVectorSchemaRootAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.List;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.apache.arrow.vector.BigIntVector;
import org.apache.arrow.vector.IntVector;
import org.apache.arrow.vector.VarCharVector;
import org.apache.arrow.vector.VectorSchemaRoot;
import org.apache.arrow.vector.testing.ValueVectorDataPopulator;
import org.apache.arrow.vector.types.pojo.ArrowType;
import org.apache.arrow.vector.types.pojo.Field;
import org.apache.arrow.vector.types.pojo.FieldType;
import org.apache.arrow.vector.types.pojo.Schema;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -155,4 +160,20 @@ public void testRootWithDifferentChildTypes() {
assertEquals("Vector schema roots have different schemas.", exp.getMessage());
}
}

@Test
public void testAppendSliceWithNonNullableNullVector() {
Schema schema =
new Schema(List.of(new Field("n", FieldType.notNullable(ArrowType.Null.INSTANCE), null)));
try (VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator);
VectorSchemaRoot target = VectorSchemaRoot.create(schema, allocator)) {
root.setRowCount(3);
try (VectorSchemaRoot slice = root.slice(0, 2)) {
assertEquals(schema, slice.getSchema());
VectorSchemaRootAppender.append(target, slice);
}
assertEquals(2, target.getRowCount());
assertEquals(2, target.getVector("n").getValueCount());
}
}
}
Loading