Skip to content

Core: Refactor v4 struct builders to improve validation #16408

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
anoopj wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Core: Refactor v4 struct builders to improve validation #16408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6bc278
Core: Refactor v4 struct builders for fail-early validation
anoopj May 19, 2026
4eda929
Incorporate feedback from Ryan
anoopj May 23, 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
24 changes: 14 additions & 10 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 @@ -51,7 +51,7 @@ private DeletionVectorStruct(DeletionVectorStruct toCopy) {
}

private DeletionVectorStruct(String location, long offset, long sizeInBytes, long cardinality) {
super(BASE_TYPE, BASE_TYPE);
super(BASE_TYPE.fields().size());
this.location = location;
this.offset = offset;
this.sizeInBytes = sizeInBytes;
Expand Down Expand Up @@ -140,37 +140,41 @@ public String toString() {

static class Builder {
private String location = null;
private long offset = -1L;
private long sizeInBytes = -1L;
private long cardinality = -1L;
private Long offset = null;
private Long sizeInBytes = null;
private Long cardinality = null;

Builder location(String dvLocation) {
Preconditions.checkArgument(dvLocation != null, "Invalid location: null");
this.location = dvLocation;
return this;
}

Builder offset(long dvOffset) {
Preconditions.checkArgument(dvOffset >= 0, "Invalid offset: %s (must be >= 0)", dvOffset);
this.offset = dvOffset;
return this;
}

Builder sizeInBytes(long dvSizeInBytes) {
Preconditions.checkArgument(
dvSizeInBytes >= 0, "Invalid size in bytes: %s (must be >= 0)", dvSizeInBytes);
this.sizeInBytes = dvSizeInBytes;
return this;
}

Builder cardinality(long dvCardinality) {
Preconditions.checkArgument(
dvCardinality > 0, "Invalid cardinality: %s (must be positive)", dvCardinality);
this.cardinality = dvCardinality;
return this;
}

DeletionVectorStruct build() {
Preconditions.checkArgument(location != null, "Invalid location: null");
Preconditions.checkArgument(offset >= 0, "Invalid offset: %s (must be >= 0)", offset);
Preconditions.checkArgument(
sizeInBytes >= 0, "Invalid size in bytes: %s (must be >= 0)", sizeInBytes);
Preconditions.checkArgument(
cardinality >= 0, "Invalid cardinality: %s (must be >= 0)", cardinality);
Preconditions.checkArgument(location != null, "Missing required value: location");
Preconditions.checkArgument(offset != null, "Missing required value: offset");
Preconditions.checkArgument(sizeInBytes != null, "Missing required value: size in bytes");
Preconditions.checkArgument(cardinality != null, "Missing required value: cardinality");
return new DeletionVectorStruct(location, offset, sizeInBytes, cardinality);
}
}
Expand Down
103 changes: 61 additions & 42 deletions core/src/main/java/org/apache/iceberg/ManifestInfoStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private ManifestInfoStruct(
long minSequenceNumber,
byte[] dv,
Long dvCardinality) {
super(BASE_TYPE, BASE_TYPE);
super(BASE_TYPE.fields().size());
this.addedFilesCount = addedFilesCount;
this.existingFilesCount = existingFilesCount;
this.deletedFilesCount = deletedFilesCount;
Expand Down Expand Up @@ -257,116 +257,135 @@ public String toString() {
}

static class Builder {
private int addedFilesCount = -1;
private int existingFilesCount = -1;
private int deletedFilesCount = -1;
private int replacedFilesCount = -1;
private long addedRowsCount = -1L;
private long existingRowsCount = -1L;
private long deletedRowsCount = -1L;
private long replacedRowsCount = -1L;
private long minSequenceNumber = -1L;
private Integer addedFilesCount = null;
private Integer existingFilesCount = null;
private Integer deletedFilesCount = null;
private Integer replacedFilesCount = null;
private Long addedRowsCount = null;
private Long existingRowsCount = null;
private Long deletedRowsCount = null;
private Long replacedRowsCount = null;
private Long minSequenceNumber = null;
private byte[] dv = null;
private Long dvCardinality = null;

Builder addedFilesCount(int count) {
Preconditions.checkArgument(
count >= 0, "Invalid added files count: %s (must be >= 0)", count);
this.addedFilesCount = count;
return this;
}

Builder existingFilesCount(int count) {
Preconditions.checkArgument(
count >= 0, "Invalid existing files count: %s (must be >= 0)", count);
this.existingFilesCount = count;
return this;
}

Builder deletedFilesCount(int count) {
Preconditions.checkArgument(
count >= 0, "Invalid deleted files count: %s (must be >= 0)", count);
this.deletedFilesCount = count;
return this;
}

Builder replacedFilesCount(int count) {
Preconditions.checkArgument(
count >= 0, "Invalid replaced files count: %s (must be >= 0)", count);
this.replacedFilesCount = count;
return this;
}

Builder addedRowsCount(long count) {
Preconditions.checkArgument(count >= 0, "Invalid added rows count: %s (must be >= 0)", count);
this.addedRowsCount = count;
return this;
}

Builder existingRowsCount(long count) {
Preconditions.checkArgument(
count >= 0, "Invalid existing rows count: %s (must be >= 0)", count);
this.existingRowsCount = count;
return this;
}

Builder deletedRowsCount(long count) {
Preconditions.checkArgument(
count >= 0, "Invalid deleted rows count: %s (must be >= 0)", count);
this.deletedRowsCount = count;
return this;
}

Builder replacedRowsCount(long count) {
Preconditions.checkArgument(
count >= 0, "Invalid replaced rows count: %s (must be >= 0)", count);
this.replacedRowsCount = count;
return this;
}

Builder minSequenceNumber(long sequenceNumber) {
Preconditions.checkArgument(
sequenceNumber >= 0, "Invalid min sequence number: %s (must be >= 0)", sequenceNumber);
this.minSequenceNumber = sequenceNumber;
return this;
}

Builder dv(ByteBuffer buffer) {
this.dv = buffer != null ? ByteBuffers.toByteArray(buffer) : null;
return this;
}

Builder dv(byte[] buffer) {
this.dv = buffer;
Preconditions.checkArgument(buffer != null, "Invalid DV: null");
this.dv = ByteBuffers.toByteArray(buffer);
return this;
}

Builder dvCardinality(Long cardinality) {
Builder dvCardinality(long cardinality) {
Preconditions.checkArgument(
cardinality > 0, "Invalid DV cardinality: %s (must be positive)", cardinality);
this.dvCardinality = cardinality;
return this;
}

ManifestInfoStruct build() {
Preconditions.checkArgument(
addedFilesCount >= 0, "Invalid added files count: %s (must be >= 0)", addedFilesCount);
addedFilesCount != null, "Missing required value: added files count");
Preconditions.checkArgument(
existingFilesCount >= 0,
"Invalid existing files count: %s (must be >= 0)",
existingFilesCount);
existingFilesCount != null, "Missing required value: existing files count");
Preconditions.checkArgument(
deletedFilesCount >= 0,
"Invalid deleted files count: %s (must be >= 0)",
deletedFilesCount);
deletedFilesCount != null, "Missing required value: deleted files count");
Preconditions.checkArgument(
replacedFilesCount >= 0,
"Invalid replaced files count: %s (must be >= 0)",
replacedFilesCount);
replacedFilesCount != null, "Missing required value: replaced files count");
Preconditions.checkArgument(
addedRowsCount != null, "Missing required value: added rows count");
Preconditions.checkArgument(
existingRowsCount != null, "Missing required value: existing rows count");
Preconditions.checkArgument(
deletedRowsCount != null, "Missing required value: deleted rows count");
Preconditions.checkArgument(
addedRowsCount >= 0, "Invalid added rows count: %s (must be >= 0)", addedRowsCount);
replacedRowsCount != null, "Missing required value: replaced rows count");
Preconditions.checkArgument(
existingRowsCount >= 0,
"Invalid existing rows count: %s (must be >= 0)",
existingRowsCount);
minSequenceNumber != null, "Missing required value: min sequence number");
Preconditions.checkArgument(
deletedRowsCount >= 0, "Invalid deleted rows count: %s (must be >= 0)", deletedRowsCount);
addedRowsCount == 0 || addedFilesCount > 0,
"Invalid added counts: %s rows in %s files",
addedRowsCount,
addedFilesCount);
Preconditions.checkArgument(
existingRowsCount == 0 || existingFilesCount > 0,
"Invalid existing counts: %s rows in %s files",
existingRowsCount,
existingFilesCount);
Preconditions.checkArgument(
replacedRowsCount >= 0,
"Invalid replaced rows count: %s (must be >= 0)",
replacedRowsCount);
deletedRowsCount == 0 || deletedFilesCount > 0,
"Invalid deleted counts: %s rows in %s files",
deletedRowsCount,
deletedFilesCount);
Preconditions.checkArgument(
minSequenceNumber >= 0,
"Invalid min sequence number: %s (must be >= 0)",
minSequenceNumber);
replacedRowsCount == 0 || replacedFilesCount > 0,
"Invalid replaced counts: %s rows in %s files",
replacedRowsCount,
replacedFilesCount);
Preconditions.checkArgument(
(dv == null) == (dvCardinality == null),
"Invalid DV and cardinality: must both be null or non-null");
Preconditions.checkArgument(
dvCardinality == null || dvCardinality > 0,
"Invalid DV cardinality: %s (must be positive)",
dvCardinality);
return new ManifestInfoStruct(
addedFilesCount,
existingFilesCount,
Expand Down
Loading