[ump45nose]

What's Changed

NullableStructWriter / SingleStructWriter (generated from StructWriters.java) walk the struct field's children in their constructor and create a child vector for each one. There was no case NULL, so a struct whose field has a Null-typed child threw UnsupportedOperationException: Unknown type: NULL. Because StructVector builds its writer as soon as it is constructed, this broke new StructVector(field, ...) and anything that goes through it, such as StructVector#getTransferPair(Field, BufferAllocator) from the issue.

There is no writer for the Null type, so the new case only adds the NullVector child to the container (with the child's nullability). The resulting vector's field now matches the input field, and the other children's writers keep working as before.

Tests added to TestStructVector:

• testStructVectorTransferPairWithNullType: the reproducer from the issue, plus a transfer() that checks the value count, struct validity and the NullVector child.
• testStructVectorFromFieldWithNullChild: builds a StructVector straight from a Field with a Null child and an Int child, then writes through the struct writer.

Both fail on main with Unknown type: NULL and pass with this change. I also ran TestStructVector, TestComplexWriter, TestListVector, TestMapVector, TestUnionVector, TestDenseUnionVector, TestComplexCopier, TestPromotableWriter and TestSplitAndTransfer (all pass, both the default and netty runs), and mvn -pl vector spotless:check.

AI assistance disclosure: this change, its tests and this description were prepared with the help of an AI coding assistant. The tests listed above were run locally against this branch.

Closes #830.

[ump45nose]

The "Ensure PR format" check is only waiting for a label. Could a maintainer add bug-fix? I do not have permission to label PRs here. (AI-assisted reply.)

[jbonofre]

@ump45nose I'm reviewing this PR. I set the label.

[jbonofre]

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]

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.

[jbonofre]

Could this child be non nullable and carry metadata? Then assertEquals(structField, vector.getField()) below checks that the Null child field is kept. As written, it passes even though a non nullable Null child comes back nullable.

Suggested change

	Field nullField = new Field("null_child", FieldType.nullable(ArrowType.Null.INSTANCE), null);
	Field nullField =
	new Field(
	"null_child",
	new FieldType(false, ArrowType.Null.INSTANCE, null, Map.of("k", "v")),
	null);

[ump45nose]

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.

[jbonofre]

NullVector.isNull always returns true, so this always passes. Checking the child field against the input would be more helpful:

Suggested change

	assertTrue(toChild.isNull(0));
	assertEquals(nullField, toChild.getField());

Same idea for line 402: s1 is built through the same getNewVector path, so assertEquals(s1.getField(), toVector.getField()) passes even when both are wrong. Comparing with structField would catch it.

[ump45nose]

Switched to assertEquals(nullField, toChild.getField()) and assertEquals(structField, toVector.getField()) as suggested.

[ump45nose]

Addressed @jbonofre's review in edf274a:

• Types.NULL.getNewVector now uses new NullVector(field) so FieldType (nullability + metadata) is kept through Field.createVector / addOrGet.
• StructWriters NULL case uses child.getFieldType().
• Tests now use a non-nullable Null child with metadata and assert field equality against the input structField / nullField.

Local: mvn -pl vector -am test -Dtest=TestStructVector (netty + unsafe) — 15/15 pass.

AI-assisted reply.

[jbonofre]

@ump45nose thanks! I'm doing a new pass 😄

[jbonofre]

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]

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.