Skip to content

Core: V4 write direction wrappers#16936

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

Core: V4 write direction wrappers #16936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0c5044c
Core: Add v4 write-direction TrackedFile adapters
stevenzwu Jul 18, 2026
8f39ba3
Core: address review feedback on v4 write-direction TrackedFile adapters
stevenzwu Jul 20, 2026
32a41af
Core: extract MapBackedContentStats to a top-level class
stevenzwu Jul 24, 2026
ca141a6
Core: address Anoop's review comments
stevenzwu Jul 24, 2026
b5bf934
Core: implement new FieldStats presence methods; simplify MapBackedFi…
stevenzwu Jul 24, 2026
83862c8
Core: address feedback on v4 MapBackedContentStats + tighten tests
stevenzwu Jul 25, 2026
43d51bd
Core: address v4 write-direction adapter review feedback
stevenzwu Jul 26, 2026
87ad5c0
API: remove unused V4_AND_ABOVE test helper
stevenzwu Jul 26, 2026
2890eb0
Core: drop redundant "Instantiate once per writer" from forDataFile J…
stevenzwu Jul 28, 2026
81719d6
Core: trim redundant Javadoc from MapBackedContentStats
stevenzwu Jul 28, 2026
85ac363
Core: drop impl-detail paragraph from MapBackedContentStats Javadoc
stevenzwu Jul 28, 2026
ec36c95
Core: drop the `test` prefix from MapBackedContentStats tests
stevenzwu Jul 31, 2026
4852d17
Core: adapt v4 write-direction adapters to upstream TrackedFile changes
stevenzwu Jul 31, 2026
b62a355
Core: consolidate the TrackedFile ordinal mapping into TrackedFileStruct
stevenzwu Aug 4, 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
29 changes: 29 additions & 0 deletions api/src/main/java/org/apache/iceberg/ManifestFile.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -29,6 +29,9 @@
public interface ManifestFile {
int PARTITION_SUMMARIES_ELEMENT_ID = 508;

/** Format version for pre-v4 manifest files. */
int LEGACY_FORMAT_VERSION = 0;

@rdbluerdblueSep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this need to be public? Another location would probably be better to avoid polluting the public API.


Types.NestedField PATH =
required(500, "manifest_path", Types.StringType.get(), "Location URI with FS scheme");
Types.NestedField LENGTH =
Expand DownExpand Up@@ -186,6 +189,22 @@ default boolean hasDeletedFiles() {
/** Returns the total number of rows in all files with status DELETED in the manifest file. */
Long deletedRowsCount();

/**
Comment thread
stevenzwu marked this conversation as resolved.
* Returns the number of files with status REPLACED in the manifest file, or null for pre-v4
* manifests.
*/
default Integer replacedFilesCount() {
return null;

@rdbluerdblueSep 3, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't the pre-v4 value 0 because that status did not exist? That seems like the best thing to return to me. Should this also be required since it is always known for v4 manifests?

}

/**
* Returns the total number of rows in all files with status REPLACED in the manifest file, or
* null for pre-v4 manifests.
*/
default Long replacedRowsCount() {
return null;
}

/**
* Returns a list of {@link PartitionFieldSummary partition field summaries}.
*
Expand All@@ -210,6 +229,16 @@ default Long firstRowId() {
return null;
}

/** Returns the number of entries in the manifest file, or null for pre-v4 manifests. */
default Long recordCount() {

@rdbluerdblueSep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this needed? We should have all of the more specific counts.

return null;
}

/** Returns the format version of the manifest file, or 0 for pre-v4 manifests. */
default int formatVersion() {
return LEGACY_FORMAT_VERSION;
}

/**
* Copies this {@link ManifestFile manifest file}. Readers can reuse manifest file instances; use
* this method to make defensive copies.
Expand Down
64 changes: 62 additions & 2 deletions core/src/main/java/org/apache/iceberg/GenericManifestFile.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -60,6 +60,8 @@ public class GenericManifestFile extends SupportsIndexProjection
private PartitionFieldSummary[] partitions = null;
private byte[] keyMetadata = null;
private Long firstRowId = null;
private Long recordCount = null;

@anoopjanoopjJul 24, 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.

Looks like the CopyBuilder inner class may need to be updated to include these new fields?

@stevenzwustevenzwuJul 24, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

fixed

private int formatVersion = LEGACY_FORMAT_VERSION;

@rdbluerdblueSep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why update this class when these values are correctly provided by the interface?


/** Used by Avro reflection to instantiate this class when reading manifest files. */
public GenericManifestFile(Schema avroSchema) {
Expand DownExpand Up@@ -94,7 +96,7 @@ public GenericManifestFile(Schema avroSchema) {
this.firstRowId = null;
}

/** Adjust the arg order to avoid conflict with the public constructor below */
/** Pre-v4 constructor. */
GenericManifestFile(
String path,
long length,
Expand DownExpand Up@@ -130,6 +132,50 @@ public GenericManifestFile(Schema avroSchema) {
this.partitions = partitions == null ? null : partitions.toArray(new PartitionFieldSummary[0]);
this.keyMetadata = ByteBuffers.toByteArray(keyMetadata);
this.firstRowId = firstRowId;
this.recordCount = null;
this.formatVersion = LEGACY_FORMAT_VERSION;
}

/** v4+ constructor variant that accepts recordCount and formatVersion. */
GenericManifestFile(

@rdbluerdblueSep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think that this should be used for v4. Manifests are now stored as TrackedFile. Why would we update the v3 class used to read from manifest lists?

String path,
long length,
int specId,
ManifestContent content,
long sequenceNumber,
long minSequenceNumber,
Long snapshotId,
List<PartitionFieldSummary> partitions,
ByteBuffer keyMetadata,
Integer addedFilesCount,
Long addedRowsCount,
Integer existingFilesCount,
Long existingRowsCount,
Integer deletedFilesCount,
Long deletedRowsCount,
Long firstRowId,
Long recordCount,
int formatVersion) {
super(ManifestFile.schema().columns().size());
this.avroSchema = AVRO_SCHEMA;
this.manifestPath = path;
this.length = length;
this.specId = specId;
this.content = content;
this.sequenceNumber = sequenceNumber;
this.minSequenceNumber = minSequenceNumber;
this.snapshotId = snapshotId;
this.addedFilesCount = addedFilesCount;
this.addedRowsCount = addedRowsCount;
this.existingFilesCount = existingFilesCount;
this.existingRowsCount = existingRowsCount;
this.deletedFilesCount = deletedFilesCount;
this.deletedRowsCount = deletedRowsCount;
this.partitions = partitions == null ? null : partitions.toArray(new PartitionFieldSummary[0]);
this.keyMetadata = ByteBuffers.toByteArray(keyMetadata);
this.firstRowId = firstRowId;
this.recordCount = recordCount;
this.formatVersion = formatVersion;
}

/**
Expand DownExpand Up@@ -172,6 +218,8 @@ private GenericManifestFile(GenericManifestFile toCopy) {
? null
: Arrays.copyOf(toCopy.keyMetadata, toCopy.keyMetadata.length);
this.firstRowId = toCopy.firstRowId;
this.recordCount = toCopy.recordCount;
this.formatVersion = toCopy.formatVersion;
}

/** Constructor for Java serialization. */
Expand DownExpand Up@@ -272,6 +320,16 @@ public Long firstRowId() {
return firstRowId;
}

@Override
public Long recordCount() {
return recordCount;
}

@Override
public int formatVersion() {
return formatVersion;
}

@Override
public int size() {
return ManifestFile.schema().columns().size();
Expand DownExpand Up@@ -468,7 +526,9 @@ private CopyBuilder(ManifestFile toCopy) {
toCopy.existingRowsCount(),
toCopy.deletedFilesCount(),
toCopy.deletedRowsCount(),
toCopy.firstRowId());
toCopy.firstRowId(),
toCopy.recordCount(),
toCopy.formatVersion());
}
}

Expand Down
Loading
Loading