Skip to content

Core: Add builders for v4 structs #16092

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

Merged
amogh-jahagirdar merged 10 commits into from
Apr 30, 2026
Merged

Core: Add builders for v4 structs #16092

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
35926a9
Add constructors for DeletionVectorStruct and TrackingStruct
anoopj Apr 23, 2026
7ee9241
Use builders
anoopj Apr 24, 2026
2da1af6
Remove the validation to be consistent with BaseFile
anoopj Apr 24, 2026
6363aef
Feedback from Eduard
anoopj Apr 27, 2026
0cf56f6
Add validation
anoopj Apr 27, 2026
d453f43
Feedback from Eduard
anoopj Apr 28, 2026
f802f85
Feedback from Steven
anoopj Apr 29, 2026
37f114a
Update core/src/main/java/org/apache/iceberg/ManifestInfoStruct.java
anoopj Apr 29, 2026
4cd8c8a
Fix test
anoopj Apr 29, 2026
42173ee
Feedback from Amogh
anoopj Apr 29, 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
8 changes: 8 additions & 0 deletions core/src/main/java/org/apache/iceberg/DeletionVectorStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ class DeletionVectorStruct extends SupportsIndexProjection implements DeletionVe
super(BASE_TYPE, type);
}

DeletionVectorStruct(String location, long offset, long sizeInBytes, long cardinality) {
super(BASE_TYPE.fields().size());
this.location = location;
this.offset = offset;
this.sizeInBytes = sizeInBytes;
this.cardinality = cardinality;
}
Copy link
Copy Markdown
Member

@ebyhr ebyhr Apr 24, 2026

Choose a reason for hiding this comment

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

Does it make sense to add a defensive check with Preconditions.checkArgument such as non-empty location?

Copy link
Copy Markdown
Contributor Author

@anoopj anoopj Apr 24, 2026

Choose a reason for hiding this comment

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

The existing convention for StructLike implementations is not to add validations in constructors. (e.g. BaseFile). So prefer not adding it for consistency.


private DeletionVectorStruct(DeletionVectorStruct toCopy) {
super(toCopy);
this.location = toCopy.location;
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/org/apache/iceberg/TrackingStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ class TrackingStruct extends SupportsIndexProjection implements Tracking, Serial
super(BASE_TYPE.fields().size());
}

TrackingStruct(EntryStatus status, Long snapshotId) {
super(BASE_TYPE.fields().size());
this.status = status;
this.snapshotId = snapshotId;
}

private TrackingStruct(TrackingStruct toCopy) {
super(toCopy);
this.status = toCopy.status;
Expand Down
30 changes: 8 additions & 22 deletions core/src/test/java/org/apache/iceberg/TestDeletionVectorStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,8 @@ class TestDeletionVectorStruct {

@Test
void testFieldAccess() {
DeletionVectorStruct dv = new DeletionVectorStruct(DeletionVector.schema());

dv.set(0, "s3://bucket/data/dv.puffin");
dv.set(1, 256L);
dv.set(2, 128L);
dv.set(3, 42L);
DeletionVectorStruct dv =
new DeletionVectorStruct("s3://bucket/data/dv.puffin", 256L, 128L, 42L);

assertThat(dv.location()).isEqualTo("s3://bucket/data/dv.puffin");
assertThat(dv.offset()).isEqualTo(256L);
Expand All @@ -43,12 +39,8 @@ void testFieldAccess() {

@Test
void testCopy() {
DeletionVectorStruct dv = new DeletionVectorStruct(DeletionVector.schema());

dv.set(0, "s3://bucket/data/dv.puffin");
dv.set(1, 256L);
dv.set(2, 128L);
dv.set(3, 42L);
DeletionVectorStruct dv =
new DeletionVectorStruct("s3://bucket/data/dv.puffin", 256L, 128L, 42L);

DeletionVectorStruct copy = dv.copy();

Expand Down Expand Up @@ -86,11 +78,8 @@ void testProjectedStructLike() {

@Test
void testJavaSerializationRoundTrip() throws IOException, ClassNotFoundException {
DeletionVectorStruct dv = new DeletionVectorStruct(DeletionVector.schema());
dv.set(0, "s3://bucket/data/dv.puffin");
dv.set(1, 256L);
dv.set(2, 128L);
dv.set(3, 42L);
DeletionVectorStruct dv =
new DeletionVectorStruct("s3://bucket/data/dv.puffin", 256L, 128L, 42L);

DeletionVectorStruct deserialized = TestHelpers.roundTripSerialize(dv);

Expand All @@ -102,11 +91,8 @@ void testJavaSerializationRoundTrip() throws IOException, ClassNotFoundException

@Test
void testKryoSerializationRoundTrip() throws IOException {
DeletionVectorStruct dv = new DeletionVectorStruct(DeletionVector.schema());
dv.set(0, "s3://bucket/data/dv.puffin");
dv.set(1, 256L);
dv.set(2, 128L);
dv.set(3, 42L);
DeletionVectorStruct dv =
new DeletionVectorStruct("s3://bucket/data/dv.puffin", 256L, 128L, 42L);

DeletionVectorStruct deserialized = TestHelpers.KryoHelpers.roundTripSerialize(dv);

Expand Down
25 changes: 5 additions & 20 deletions core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,10 @@ class TestTrackedFileStruct {
@Test
void testFieldAccess() {
TrackedFileStruct file = new TrackedFileStruct();
TrackingStruct tracking = new TrackingStruct();
DeletionVectorStruct dv = new DeletionVectorStruct(DeletionVector.schema());
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, 42L);
DeletionVectorStruct dv = new DeletionVectorStruct("s3://bucket/dv.puffin", 100L, 50L, 5L);
ManifestInfoStruct info = new ManifestInfoStruct(ManifestInfo.schema());

tracking.set(0, EntryStatus.ADDED.id());
tracking.set(1, 42L);

dv.set(0, "s3://bucket/dv.puffin");
dv.set(1, 100L);
dv.set(2, 50L);
dv.set(3, 5L);

info.set(0, 10);
info.set(1, 20);
info.set(2, 3);
Expand Down Expand Up @@ -90,8 +82,7 @@ void testFieldAccess() {
void testReaderSideFields() {
TrackedFileStruct file = new TrackedFileStruct();

TrackingStruct tracking = new TrackingStruct();
tracking.set(0, EntryStatus.ADDED.id());
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, null);
tracking.setManifestLocation("s3://bucket/metadata/manifest.avro");
tracking.set(8, 7L);

Expand Down Expand Up @@ -279,18 +270,12 @@ void testKryoSerializationRoundTrip() throws IOException {
}

static TrackedFileStruct createFullTrackedFile() {
TrackingStruct tracking = new TrackingStruct();
tracking.set(0, EntryStatus.ADDED.id());
tracking.set(1, 42L);
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, 42L);
tracking.set(2, 10L);
tracking.setManifestLocation("s3://bucket/manifest.avro");
tracking.set(8, 3L);

DeletionVectorStruct dv = new DeletionVectorStruct(DeletionVector.schema());
dv.set(0, "s3://bucket/dv.puffin");
dv.set(1, 100L);
dv.set(2, 50L);
dv.set(3, 5L);
DeletionVectorStruct dv = new DeletionVectorStruct("s3://bucket/dv.puffin", 100L, 50L, 5L);

TrackedFileStruct file =
new TrackedFileStruct(
Expand Down
20 changes: 6 additions & 14 deletions core/src/test/java/org/apache/iceberg/TestTrackingStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ void testIsLive() {

@Test
void testInheritSnapshotId() {
TrackingStruct tracking = new TrackingStruct(Tracking.schema());
tracking.set(0, EntryStatus.ADDED.id());
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, null);
tracking.inheritFrom(createManifestTracking(100L, 60L));

// snapshotId is null, should inherit from manifest
Expand All @@ -108,8 +107,7 @@ void testInheritSnapshotId() {

@Test
void testInheritSequenceNumberForAddedEntries() {
TrackingStruct tracking = new TrackingStruct(Tracking.schema());
tracking.set(0, EntryStatus.ADDED.id());
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, null);
tracking.inheritFrom(createManifestTracking(100L, 60L));

// sequence numbers are null and status is ADDED, should inherit
Expand All @@ -119,8 +117,7 @@ void testInheritSequenceNumberForAddedEntries() {

@Test
void testDoNotInheritSequenceNumberForExistingEntries() {
TrackingStruct tracking = new TrackingStruct(Tracking.schema());
tracking.set(0, EntryStatus.EXISTING.id());
TrackingStruct tracking = new TrackingStruct(EntryStatus.EXISTING, null);
tracking.set(2, 5L);
tracking.set(3, 6L);
tracking.inheritFrom(createManifestTracking(100L, 60L));
Expand All @@ -132,9 +129,7 @@ void testDoNotInheritSequenceNumberForExistingEntries() {

@Test
void testExplicitValuesOverrideInheritance() {
TrackingStruct tracking = new TrackingStruct(Tracking.schema());
tracking.set(0, EntryStatus.ADDED.id());
tracking.set(1, 200L);
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, 200L);
tracking.set(2, 75L);
tracking.set(3, 76L);
tracking.inheritFrom(createManifestTracking(100L, 60L));
Expand Down Expand Up @@ -163,8 +158,7 @@ void testInheritFromRejectsUnequalSequenceNumbers() {

@Test
void testNoDefaultingWithoutInheritance() {
TrackingStruct tracking = new TrackingStruct(Tracking.schema());
tracking.set(0, EntryStatus.ADDED.id());
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, null);

// no inheritance, nulls stay null
assertThat(tracking.snapshotId()).isNull();
Expand All @@ -173,9 +167,7 @@ void testNoDefaultingWithoutInheritance() {
}

private static Tracking createManifestTracking(long snapshotId, long sequenceNumber) {
TrackingStruct tracking = new TrackingStruct(Tracking.schema());
tracking.set(0, EntryStatus.ADDED.id());
tracking.set(1, snapshotId);
TrackingStruct tracking = new TrackingStruct(EntryStatus.ADDED, snapshotId);
tracking.set(2, sequenceNumber);
tracking.set(3, sequenceNumber);
return tracking;
Expand Down
Loading