Core: V4 write direction wrappers by stevenzwu · Pull Request #16936 · apache/iceberg

stevenzwu · 2026-06-23T06:05:07Z

No description provided.

stevenzwu · 2026-06-23T06:14:04Z

+   * <p>REPLACED files are the prior-state entries of v4 REPLACED/MODIFIED pairs and are not live.
+   * Returns null for manifest files written by pre-v4 writers.
+   */
+  default Integer replacedFilesCount() {


Generally, public-interface changes should be minimized and delayed. Justification for keeping these two on the interface in this PR:

In-tree consumers (this PR + close follow-ups):

ContentEntryAdapters.manifestInfo() (this PR) populates the v4 root manifest's manifest_info struct from manifest.replacedFilesCount() / replacedRowsCount().

MergingSnapshotProducer.validateAddedDVs (follow-up phase) filters concurrent data manifests by replacedFilesCount > 0 to detect v4 colocated-DV write conflicts — without it, every concurrent data manifest must be deep-scanned.

Convention. The interface-default pattern matches v3's existing extensions for spec-defined optional fields on ManifestFile: firstRowId (row lineage), keyMetadata (encryption), containsNaN (partition NaN). v4 spec-defined manifest_info counts belong on the same interface in the same shape.

A TrackedFile-based parallel v4 API was considered. Would require rewriting ManifestGroup and every engine that consumes FileScanTask. The cost is disproportionate to the savings.

Projected new default methods across the full v4 stack, all spec-defined additions backing v4 manifest_info / content_entry fields:

In this PR:

default Integer replacedFilesCount()

default Long replacedRowsCount()

default int formatVersion()

when root-level manifest DV bitmaps are implemented:

default ByteBuffer deletedPositions()

default ByteBuffer replacedPositions()

gaborkaszab

Hey @stevenzwu ,
I went through mostly for my own understanding. Left some comments, mostly I'm a bit hesitant to expose setting status and sequence numbers directly through the builders. Let me know what you think!

gaborkaszab · 2026-06-23T14:52:15Z

+
+  private ContentEntryAdapters() {}
+
+  static TrackedFile fromDataFile(


I don't see how the user will use these functions, but would it make sense to merge the fromDataFile and fromDeleteFile into a common fromManifestEntry? Then the user doesn't have to decide if it's data or delete, the adapter can do it instead.

I actually think we should keep them separate. The validation for delete files is different from data files and so is the error handling and messages, I think it's cleaner that way.

The callers also always know which type they have so it should be easier.

I agree with keeping these separate for data and delete file cases.

What I think we need to separate is ManifestEntry and DataFile. We accept DataFile instances through the API and create entires in operations like AppendFiles. The best use case is when accepting data files through the public API. There's a secondary use case for wrapping/adapting ManifestEntry for internal uses like rewriting v3 files to v4, but these are more narrow.

For now, I think we need the DataFile to TrackedFile adapter, not ManifestEntry.

stevenzwu · 2026-06-26T16:37:46Z

+   * whose tracking values come from outside the writer (v4 manifest references, projections from
+   * legacy ManifestEntry).
+   */
+  static TrackedFileBuilder explicitTracking(


@gaborkaszab following up on this comment from your PR, I am adding this new factory method for the new code path in this PR.

Per review (gaborkaszab on PR apache#16936): the first if-block existed only to produce specific error messages distinguishing v3 DVs from v2 position delete files; the actual invariant (only EQUALITY_DELETES allowed in v4 leaf delete manifests) is enforced by the Preconditions check that immediately follows. Keep the explanatory comment on why POSITION_DELETES is rejected (with both context bullets) above the single invariant check. Update the two POSITION_DELETES rejection tests to assert on the unified error message. Generated-by: Claude Code Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Introduces the Phase 1 foundation for v4 metadata tree writes: TrackedFile and supporting types are extended with v4-specific shape, and the new ContentEntryAdapters utility projects legacy ManifestEntry rows and ManifestFile references into TrackedFile instances for the v4 content_entry schema. ManifestFile gains V4_FORMAT_VERSION and LEGACY_FORMAT_VERSION constants and default formatVersion() / recordCount() accessors. GenericManifestFile gets a v4 constructor variant that populates these fields at construction time; pre-v4 callers use the existing 17-arg constructor which delegates with defaults. ContentEntryAdapters.fromManifestFile dispatches the record_count source by the manifest's intrinsic formatVersion: v4+ manifests must carry a non-null recordCount populated by the writer or reader; pre-v4 manifests have it summed from the per-status file counts. TestContentEntryAdapters covers ADDED/EXISTING/DELETED tracking shape, the unpartitioned and partitioned-projection paths, format_version dispatch, record_count requirement for v4+, and the rejected status transitions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

stevenzwu · 2026-06-29T05:29:31Z

+   *     manifest's snapshot_id
   */
-  static TrackedFileBuilder data(long newSnapshotId) {
+  static TrackedFileBuilder data(Long newSnapshotId) {


when writing a data file entry to a staged leaf manifest file, the snapshot id is null and will be inherited from the manifest's snapshot id when written to the root manifest file.

stevenzwu · 2026-07-01T06:38:32Z

+ * Builds {@link TrackedFile} instances for v4+ content_entry rows from legacy {@link ManifestEntry}
+ * and {@link ManifestFile} inputs.
+ */
+class ContentEntryAdapters {


I can also move the utils to the TrackedFileAdapters, which would include two-way adaptations (from and to TrackedFile).

I would prefer to have these in TrackedFileAdapters. We also want to write tests that a double-wrapped adapter is identical, similar to a round-trip serialization test.

rdblue · 2026-07-08T17:46:30Z

+  // Cache of primitive-field types keyed by table schema. The originalTypes map a Metrics
+  // instance carries is read-only inside MetricsUtil.fromMetrics, so a single per-schema map
+  // can be shared across every adapter call for a writer's lifetime instead of being rebuilt
+  // per row. WeakHashMap keys release when callers stop referencing the schema.


This should not be needed. The schema itself indexes fields by ID and can be shared directly.

rdblue

I don't think that translating from one object to another is the right path forward. I was expecting wrappers to wrap existing interfaces rather than using a builder to translate. Ideally, wrappers can be reused so that we don't create lots of needless copies.

github-actions Bot added API core labels Jun 23, 2026

stevenzwu commented Jun 23, 2026

View reviewed changes

stevenzwu added this to V4: metadata tree Jun 23, 2026

gaborkaszab reviewed Jun 23, 2026

View reviewed changes

stevenzwu force-pushed the v4_write_direction_wrappers branch 5 times, most recently from c78d6b1 to 828e0c7 Compare June 25, 2026 21:35

github-actions Bot added docs build and removed API labels Jun 25, 2026

stevenzwu force-pushed the v4_write_direction_wrappers branch from 828e0c7 to fa86dc1 Compare June 25, 2026 22:18

github-actions Bot added the API label Jun 25, 2026

stevenzwu force-pushed the v4_write_direction_wrappers branch 5 times, most recently from fa46d3d to 55f5573 Compare June 26, 2026 05:57

stevenzwu mentioned this pull request Jun 26, 2026

Core: Adjustments for TrackedFileBuilder #16964

Closed

stevenzwu commented Jun 26, 2026

View reviewed changes

stevenzwu force-pushed the v4_write_direction_wrappers branch 8 times, most recently from b5c92a0 to d5c696b Compare June 28, 2026 06:09

stevenzwu force-pushed the v4_write_direction_wrappers branch 2 times, most recently from 0f64967 to bff274c Compare June 28, 2026 22:53

stevenzwu marked this pull request as ready for review June 29, 2026 01:33

stevenzwu force-pushed the v4_write_direction_wrappers branch 2 times, most recently from 30e188a to 6bb3622 Compare June 29, 2026 05:15

stevenzwu force-pushed the v4_write_direction_wrappers branch from 6bb3622 to 0535c88 Compare June 29, 2026 05:24

stevenzwu commented Jun 29, 2026

View reviewed changes

stevenzwu moved this to In review in V4: metadata tree Jun 29, 2026

stevenzwu commented Jul 1, 2026

View reviewed changes

stevenzwu mentioned this pull request Jul 1, 2026

Core: Add ManifestFile adapter for v4 tracked files #16867

Draft

rdblue reviewed Jul 8, 2026

View reviewed changes

rdblue requested changes Jul 8, 2026

View reviewed changes

wombatu-kun mentioned this pull request Jul 10, 2026

Core: Clean up TestTrackedFileAdapters and drop TrackedFileBuilder #17144

Open


		private ContentEntryAdapters() {}

		static TrackedFile fromDataFile(

Uh oh!

Conversation

stevenzwu commented Jun 23, 2026

Uh oh!

stevenzwu Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenzwu Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaborkaszab left a comment

Choose a reason for hiding this comment

Uh oh!

gaborkaszab Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

anuragmantri Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

rdblue Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevenzwu Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

stevenzwu Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

stevenzwu Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdblue Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

rdblue Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

rdblue left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stevenzwu Jun 23, 2026 •

edited

Loading

stevenzwu Jun 23, 2026 •

edited

Loading

stevenzwu Jul 1, 2026 •

edited

Loading