Skip to content

Spec: V4 Adaptive Metadata Tree Spec Changes for Entry Structures - #16025

Open
amogh-jahagirdar wants to merge 14 commits into
apache:mainfrom
amogh-jahagirdar:v4-amt-changes
Open

Spec: V4 Adaptive Metadata Tree Spec Changes for Entry Structures#16025
amogh-jahagirdar wants to merge 14 commits into
apache:mainfrom
amogh-jahagirdar:v4-amt-changes

Conversation

@amogh-jahagirdar

@amogh-jahagirdar amogh-jahagirdar commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

This is one PR for V4 Adaptive Metadata Tree spec changes. The focus of this PR is to update the proposed entry structure in https://docs.google.com/document/d/1k4x8utgh41Sn1tr98eynDKCWq035SV_f75rtNHcerVw/edit?tab=t.wd1z5eeup025#heading=h.80fbnuij9rhg

There will be other PRs for updating the scan planning section, adding implementation notes for CDC etc.

@github-actions github-actions Bot added the Specification Issues that may introduce spec changes. label Apr 18, 2026
@amogh-jahagirdar amogh-jahagirdar changed the title V4 amt spec changes Spec: V4 Adaptive Metadata Tree Spec Changes for Entry Structures Apr 28, 2026
@amogh-jahagirdar

Copy link
Copy Markdown
Contributor Author

Two general working principals:

  1. Trying to make it easy for someone who just wants to implement v4 to click the v4 tabs, and have it be self describing.
  2. Try to do 1 without shifting too much of the existing spec contents, to avoid the risk of changing the spec for v1-v3.

Comment thread format/spec.md
| _optional_ | _optional_ | **`min-snapshots-to-keep`** | `int` | For `branch` type only, a positive number for the minimum number of snapshots to keep in a branch while expiring snapshots. Defaults to table property `history.expire.min-snapshots-to-keep`. |
| _optional_ | _optional_ | **`max-snapshot-age-ms`** | `long` | For `branch` type only, a positive number for the max age of snapshots to keep when expiring, including the latest snapshot. Defaults to table property `history.expire.max-snapshot-age-ms`. |
| _optional_ | _optional_ | **`max-ref-age-ms`** | `long` | For snapshot references except the `main` branch, a positive number for the max age of the snapshot reference to keep while expiring snapshots. Defaults to table property `history.expire.max-ref-age-ms`. The `main` branch never expires. |
=== "v1 - v3"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If people like the two tabbed approach then we should get #14656 in first, that'll make this diff easier to review

Comment thread format/spec.md Outdated
@@ -130,8 +130,10 @@ Tables do not require rename, except for tables that use atomic rename to implem
* **Schema** -- Names and types of fields in a table.
* **Partition spec** -- A definition of how partition values are derived from data fields.
* **Snapshot** -- The state of a table at some point in time, including the set of all data files.
* **Manifest list** -- A file that lists manifest files; one per snapshot.
* **Manifest** -- A file that lists data or delete files; a subset of a snapshot.
* **Manifest list** -- (V1-V3 only) A file that lists manifest files; one per snapshot.

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.

nit: most of the time, lower case v1, v2 are used in the spec

Comment thread format/spec.md
* **Manifest** -- A file that lists data or delete files; a subset of a snapshot.
* **Manifest list** -- (V1-V3 only) A file that lists manifest files; one per snapshot.
* **Root Manifest** -- (V4+) A manifest that can reference data files, delete files, and other data and delete manifests; one per snapshot. Replaces manifest lists in V4.
* **Data manifest** -- A file that lists data files; a subset of a snapshot.

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.

Do we want to want to mention colocated DVs and column files for data manifests in v4+?

Comment thread format/spec.md Outdated
@@ -484,7 +486,7 @@ Note that:

### Partitioning

Data files are stored in manifests with a tuple of partition values that are used in scans to filter out files that cannot contain records that match the scan’s filter predicate. Partition values for a data file must be the same for all records stored in the data file. (Manifests store data files from any partition, as long as the partition spec is the same for the data files.)
Data files are stored in manifests with partition values that are used in scans to filter out files that cannot contain records that match the scan’s filter predicate. Partition values for a data file must be the same for all records stored in the data file. In V1-V3, manifests store data files from any partition, as long as the partition spec is the same for the data files. In V4, manifests can store data files from different partition specs because partition values are stored as column statistics.

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.

because partition values are stored as column statistics.

This sentence probably needs to be updated based on the last community sync.

Comment thread format/spec.md

| Field id | Name | Type | Write | Read | Description |
|----------|------|------|-------|------|-------------|
| 134 | **`content_type`** | `int` (0: DATA, 2: EQUALITY DELETES, 3: DATA_MANIFEST, 4: DELETE_MANIFEST) | *required* | *required* | Type of content stored in the entry. Content types 3 and 4 are only valid in root manifests. |

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.

POSITION_DELETES handling — the table omits value 1 entirely, but the comment two paragraphs below ("Value 1 (POSITION_DELETES) no longer applies in entries") is the only place that says so. Worth making the table itself unambiguous: either list 1: RESERVED (writers must not produce; readers must reject) or call out in the table cell that 1 is intentionally skipped. Otherwise a reader reaches for the int and wonders if the spec just forgot it.

Comment thread format/spec.md Outdated
| Field id | Name | Type | Write | Read | Description |
|----------|------|------|-------|------|-------------|
| 134 | **`content_type`** | `int` (0: DATA, 2: EQUALITY DELETES, 3: DATA_MANIFEST, 4: DELETE_MANIFEST) | *required* | *required* | Type of content stored in the entry. Content types 3 and 4 are only valid in root manifests. |
| 157 | **`writer_format_version`** | `int` (0: PRE-V4, 1: V4) | *required* | *required* | Writer format version. V4 writers must produce `writer_format_version` 1. |

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.

V4 writers must produce writer_format_version 1.

why not 4 for V4 writers? null for entries written before V4.

Also, the 0: PRE-V4 enum is odd: PRE-V4 manifests use the manifest_entry schema, not content_entry, so they can't have this field. When does value 0 ever appear?

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.

this should always be 1, right? Shouldn't we remove this and allocate field ID 157 to column_files? cc @amogh-jahagirdar

Comment thread format/spec.md Outdated
| 158 | **`column_files`** | `list<column_file>` | *optional* | *optional* | Column update files associated with this entry. |
| 101 | **`file_format`** | `string` | *required* | *required* | String file format name: `avro`, `orc`, `parquet`, or `puffin` |
| 147 | **`tracking`** | `tracking` struct | *required* | *required* | Groups status, snapshot, and sequence number. See tracking struct below. |
| 148 | **`deletion_vector`** | `deletion_vector` struct | *optional* | *optional* | Row-level deletion vector for a data file. |

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.

the deletion_vector schema is only for external data DVs with location field. the inline manifest DV is stored as part of the manifest_info, which is a bit inconsistent to me. Ideally, the DV struct can capture both outline (data DV) and inline (manifest DV) at the top level. E.g. the cardinality field can be shared.

Comment thread format/spec.md Outdated

Value 1 (POSITION_DELETES) no longer applies in entries because deletion vector metadata is colocated with data files (`content_type` 0).

Leaf data manifests may only contain entries with `content_type` 0 (DATA); leaf delete manifests may only contain entries with `content_type` 2 (EQUALITY DELETES).

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.

leaf delete manifest files can also contain entries with position deletes written back when the format version is 2. that also means the line 683 above is also inaccurate

Comment thread format/spec.md

Leaf data manifests may only contain entries with `content_type` 0 (DATA); leaf delete manifests may only contain entries with `content_type` 2 (EQUALITY DELETES).

The following constraints apply based on `content_type`:

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.

it is better to capture these in the description column in the table (instead of a separate bullet list here).

Comment thread format/spec.md
Comment thread format/spec.md
| | | _required_ | **`first-row-id`** | The first `_row_id` assigned to the first row in the first data file in the first manifest, see [Row Lineage](#row-lineage) |
| | | _required_ | **`added-rows`** | The upper bound of the number of rows with assigned row IDs, see [Row Lineage](#row-lineage) |
| | | _optional_ | **`key-id`** | ID of the encryption key that encrypts the manifest list key metadata |
=== "v1 - v3"

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.

also need a table for v4

Comment thread format/spec.md
@@ -75,9 +75,9 @@ This table format tracks individual data files in a table instead of directories

Table state is maintained in metadata files. All changes to table state create a new metadata file and replace the old metadata with an atomic swap. The table metadata file tracks the table schema, partitioning config, custom properties, and snapshots of the table contents. A snapshot represents the state of a table at some time and is used to access the complete set of data files in the table.

Data files in snapshots are tracked by one or more manifest files that contain a row for each data file in the table, the file's partition data, and its metrics. The data in a snapshot is the union of all files in its manifests. Manifest files are reused across snapshots to avoid rewriting metadata that is slow-changing. Manifests can track data files with any subset of a table and are not associated with partitions.

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.

Do we want to add anything up front about V4 unifying this structure? I just note that you extended this paragraph to say Data Manifests and Delete Manifests, but now we will only have one type of manifest

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.

That sounds like a good idea to me.

Comment thread format/spec.md Outdated

The manifests that make up a snapshot are stored in a manifest list file. Each manifest list stores metadata about manifests, including partition stats and data file counts. These stats are used to avoid reading manifests that are not required for an operation.
In V1-V3, the manifests that make up a snapshot are stored in a manifest list file. Each manifest list stores metadata about manifests, including partition stats and data file counts. These stats are used to avoid reading manifests that are not required for an operation. In V4, manifest lists are replaced by a single root manifest per snapshot, which can contain references to data files, delete files, and other data and delete manifests in a unified structure.

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.

I thought technically we aren't allowing a reference to a delete file in the Root Manifest or in any V4 Manifest except for V4 Delete manifests for equality deletes. Shouldn't it always be a coupled entry of DV and DataFile or DV and Manifest?

Comment thread format/spec.md

A manifest is a valid Iceberg data file: files must use valid Iceberg formats, schemas, and column projection.

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.

Not sure why we are changing the pluralization here but the change is ok. Just wondering because we immediately switch back to singular in the next paragraph.

Comment thread format/spec.md Outdated
1. Technically, data files can be deleted when the last snapshot that contains the file as “live” data is garbage collected. But this is harder to detect and requires finding the diff of multiple snapshots. It is easier to track what files are deleted in a snapshot and delete them when that snapshot expires. It is not recommended to add a deleted file back to a table. Adding a deleted file can lead to edge cases where incremental deletes can break table snapshots.
2. Manifest list files are required in v2, so that the `sequence_number` and `snapshot_id` to inherit are always available.
- V1-V3: A manifest stores files for a single partition spec. When a table’s partition spec changes, old files remain in the older manifest and newer files are written to a new manifest. This is required because a manifest file’s schema is based on its partition spec. The partition spec of each manifest is used to transform predicates on the table’s data rows into predicates on partition values during job planning.
- V4: Manifests are not bound to a single partition spec. Files with different partition specs can coexist in the same manifest because partition values are stored in column statistics using source column IDs rather than in a partition-spec-specific struct. The `partition-spec-id` in manifest metadata is tracked for informational purposes but does not constrain the contents.

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.

I'd leave the rational out of this paragraph. I think it's find to just say that they are not bound to a partition spec, I think partition-spec-id needs a better description here ... The spec id used by the writer when generating this data file?

Comment thread format/spec.md

#### Manifest File Format

Manifests are Avro files in V1-V3. Starting in V4, writers must produce manifests in Parquet.

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.

While I support this for simplicity, I know @rdblue still wants to have the Avro option. currently the code in my PR lets you write either in the SDK and i'm not sure it is much more expensive to allow both in the spec. Worth having a community discussion though.

Comment thread format/spec.md Outdated
| | _required_ | `content` | Type of content files tracked by the manifest: "data" or "deletes" |

=== "v4"
| Write | Read | Key | Value |

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.

Why is there write and read? What would optional "read" be? I assume this was probably an LLM just trying to make a balanced table.

More importantly, do we want to relax the write requirements? In V2/3 these were all required, but now they are optional

Comment thread format/spec.md Outdated
| Write | Read | Key | Value |
|------------|------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------------------|
| _optional_ | _optional_ | `schema-id` | ID of the schema used to write the manifest as a string |
| _optional_ | _optional_ | `partition-spec-id` | ID of the partition spec used to write the manifest as a string |

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.

Not sure this one makes sense now? Entries should all have a spec, but i'm not sure it makes sense to have a global spec id for the manifest anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Miss on my part, yes manifests are no longer bound to a partition spec!

Comment thread format/spec.md Outdated
| _optional_ | _optional_ | `schema-id` | ID of the schema used to write the manifest as a string |
| _optional_ | _optional_ | `partition-spec-id` | ID of the partition spec used to write the manifest as a string |
| _optional_ | _optional_ | `format-version` | Table format version number of the manifest as a string |
| _optional_ | _optional_ | `content` | Type of content files tracked by the manifest: "data" or "deletes" |

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.

In V4 this is data or "equality deletes" but I think it's fine to just call it deletes

Comment thread format/spec.md
Iceberg v2 adds data and file sequence numbers to the entry and makes the snapshot ID optional. Values for these fields are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the manifest list.
The `sequence_number` field represents the data sequence number and must never change after a file is added to the dataset. The data sequence number represents a relative age of the file content and should be used for planning which delete files apply to a data file.
The `file_sequence_number` field represents the sequence number of the snapshot that added the file and must also remain unchanged upon assigning at commit. The file sequence number can't be used for pruning delete files as the data within the file may have an older data sequence number.
The data and file sequence numbers are inherited only if the entry status is 1 (added). If the entry status is 0 (existing) or 2 (deleted), the entry must include both sequence numbers explicitly.

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.

We should probably add the MODIFIED status also? Also, would you please update the AMT proposal doc to include the status? (Also we need to move the content from the Colocated entry tab into the main tab?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I still need to update these when I get a chance, will do

@stevenzwu stevenzwu mentioned this pull request Jun 6, 2026
3 tasks
stevenzwu added a commit to stevenzwu/iceberg that referenced this pull request Jun 6, 2026
Add the writer_format_version field (id 157, required int) to the v4
content_entry schema per spec PR apache#16025. Values: 0 = pre-v4 (entries
referencing legacy leaf manifests carried over during a v3->v4 upgrade),
1 = v4 (entries produced by a v4 writer).

The field is positioned after content_type and before location in
TrackedFile.schemaWithContentStats(...). TrackedFileStruct gains a
positional slot, default value 1, a setter, and copy-preservation.
Tests cover schema field-id presence, default-1, explicit-0, and
round-trip through copy/copyWithStats/copyWithoutStats.

No validation is added at this layer; the read-side bounds check
(SUPPORTED_WRITER_FORMAT_VERSION = 1) lands with the v4 leaf reader in
a follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevenzwu added a commit to stevenzwu/iceberg that referenced this pull request Jun 7, 2026
…add matching reader

V4Writer and V4DeleteWriter now emit content_entry Parquet rows via
TrackedFileWrapper/ContentEntryAdapter rather than the legacy manifest_entry
Avro shape. ContentEntryReader and ContentEntryManifestReaderAdapter project
content_entry rows back to ManifestEntry<DataFile/DeleteFile> so all downstream
consumers (ManifestGroup, MergingSnapshotProducer rewrite paths) work
unchanged.

Read-path dispatch in ManifestFiles is layered:
1. Avro manifests are always legacy (no file inspection).
2. Snapshot-tree callers thread an Integer writerFormatVersion hint through
   the new package-private read overloads: 1 routes to ContentEntryReader,
   0 routes to legacy.
3. Callers without a hint (tests writing-then-reading, ad-hoc tooling) fall
   back to inspecting the Parquet footer schema for field id 134 (content_type)
   or 147 (tracking). The footer read is delegated to InternalParquet via
   DynMethods so core has no compile-time dependency on iceberg-parquet.

Key design choices:
- TrackedFile.schemaWithContentStats omits partition and content_stats when
  their struct types are empty (Parquet rejects empty groups).
- TrackedFileWrapper uses hasPartition/hasContentStats flags to map positions
  dynamically when either optional group is absent.
- V4Writer.add(DataFile) bypasses Delegates.suppressFirstRowId so per-entry
  firstRowId is stored in the tracking struct rather than at manifest level.
- ContentEntryReader.setEntry uses wrapAppendPreservingFirstRowId for ADDED
  entries so firstRowId read from the tracking struct is not re-suppressed.
- ContentEntryAdapter preserves firstRowId for EXISTING entries so uncommitted
  manifests can round-trip per-entry row IDs.
- ContentEntryManifestReaderAdapter applies the same committed/uncommitted
  firstRowId nullification logic as ManifestReader.idAssigner.
- ContentEntryManifestReaderAdapter.iterator tracks ordinal position and sets
  fileOrdinal and manifestLocation on each BaseFile to match Avro reader behavior.
- Parquet.readSchema(InputFile) is a new public helper that returns just the
  Iceberg-converted file schema; InternalParquet.readSchema delegates to it
  for the DynMethods entry point.
- v4 spec forbids content_type=POSITION_DELETES (PR apache#16025); three
  TestManifestReader tests that write standalone position-delete files / DV
  delete files are guarded with assumeThat isLessThan(4) and will be removed
  once PR apache#16677 (or its successor) gates v4 out of the broad parameterized
  test suite during incubation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevenzwu added a commit to stevenzwu/iceberg that referenced this pull request Jun 11, 2026
Add the writer_format_version field (id 157, required int) to the v4
content_entry schema per spec PR apache#16025. Values: 0 = pre-v4 (entries
referencing legacy leaf manifests carried over during a v3->v4 upgrade),
1 = v4 (entries produced by a v4 writer).

The field is positioned after content_type and before location in
TrackedFile.schemaWithContentStats(...). TrackedFileStruct gains a
positional slot, default value 1, a setter, and copy-preservation.
Tests cover schema field-id presence, default-1, explicit-0, and
round-trip through copy/copyWithStats/copyWithoutStats.

No validation is added at this layer; the read-side bounds check
(SUPPORTED_WRITER_FORMAT_VERSION = 1) lands with the v4 leaf reader in
a follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread format/spec.md
| 100 | **`location`** | `string` | *required* | *required* | Location of the file or manifest. |
| 101 | **`file_format`** | `string` | *required* | *required* | String file format name: `avro`, `orc`, `parquet`, or `puffin` |
| 147 | **`tracking`** | `tracking` struct | *required* | *required* | Groups status, snapshot, and sequence number. See tracking struct below. |
| 141 | **`spec_id`** | `int` | *optional* | *optional* | ID of the partition spec used to write this manifest or data file. |

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.

This field is optional, but in fact it's mandatory for data/delete file entries, right? It can be null for leaf manifests. I think ww should articulate this constraint in the spec.

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.

Agree. Allowed to be null for leaf manifests because we allow mixed specs in a manifest. Relevant implementation thread.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will update this, yes it certainly can be null for leafs manifests since they are not bound to a partition spec,.

We should reason about if it makes sense to make it required for data files rather than assume we just carry forward the requirement for data files that we have in v3 and older format versions. Remember the data file partition tuple currently modeled in v4 is the union schema, so we don't strictly need the specific partition spec for this file even for matching.

I think the main benefit of having it required for data files is that it'd be explicit on read rather than readers having to have a fallback to assume unpartitioned in the null case; but what are the cases where we even need to read the spec, it's for the tuple for eq. delete matching and pruning, which as currently established is the union schema anyways. So do we strictly need to impose the writer side requirement for v4 data files? I would argue no, unless there's another reason the spec used at the time of the write is useful information later on.

@RussellSpitzer RussellSpitzer added the Iceberg V4 Iceberg Table Format Version 4 label Jul 16, 2026
stevenzwu added a commit to stevenzwu/iceberg that referenced this pull request Jul 17, 2026
…add matching reader

V4Writer and V4DeleteWriter now emit content_entry Parquet rows via
TrackedFileWrapper/ContentEntryAdapter rather than the legacy manifest_entry
Avro shape. ContentEntryReader and ContentEntryManifestReaderAdapter project
content_entry rows back to ManifestEntry<DataFile/DeleteFile> so all downstream
consumers (ManifestGroup, MergingSnapshotProducer rewrite paths) work
unchanged.

Read-path dispatch in ManifestFiles is layered:
1. Avro manifests are always legacy (no file inspection).
2. Snapshot-tree callers thread an Integer writerFormatVersion hint through
   the new package-private read overloads: 1 routes to ContentEntryReader,
   0 routes to legacy.
3. Callers without a hint (tests writing-then-reading, ad-hoc tooling) fall
   back to inspecting the Parquet footer schema for field id 134 (content_type)
   or 147 (tracking). The footer read is delegated to InternalParquet via
   DynMethods so core has no compile-time dependency on iceberg-parquet.

Key design choices:
- TrackedFile.schemaWithContentStats omits partition and content_stats when
  their struct types are empty (Parquet rejects empty groups).
- TrackedFileWrapper uses hasPartition/hasContentStats flags to map positions
  dynamically when either optional group is absent.
- V4Writer.add(DataFile) bypasses Delegates.suppressFirstRowId so per-entry
  firstRowId is stored in the tracking struct rather than at manifest level.
- ContentEntryReader.setEntry uses wrapAppendPreservingFirstRowId for ADDED
  entries so firstRowId read from the tracking struct is not re-suppressed.
- ContentEntryAdapter preserves firstRowId for EXISTING entries so uncommitted
  manifests can round-trip per-entry row IDs.
- ContentEntryManifestReaderAdapter applies the same committed/uncommitted
  firstRowId nullification logic as ManifestReader.idAssigner.
- ContentEntryManifestReaderAdapter.iterator tracks ordinal position and sets
  fileOrdinal and manifestLocation on each BaseFile to match Avro reader behavior.
- Parquet.readSchema(InputFile) is a new public helper that returns just the
  Iceberg-converted file schema; InternalParquet.readSchema delegates to it
  for the DynMethods entry point.
- v4 spec forbids content_type=POSITION_DELETES (PR apache#16025); three
  TestManifestReader tests that write standalone position-delete files / DV
  delete files are guarded with assumeThat isLessThan(4) and will be removed
  once PR apache#16677 (or its successor) gates v4 out of the broad parameterized
  test suite during incubation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevenzwu added a commit to stevenzwu/iceberg that referenced this pull request Jul 17, 2026
…add matching reader

V4Writer and V4DeleteWriter now emit content_entry Parquet rows via
TrackedFileWrapper/ContentEntryAdapter rather than the legacy manifest_entry
Avro shape. ContentEntryReader and ContentEntryManifestReaderAdapter project
content_entry rows back to ManifestEntry<DataFile/DeleteFile> so all downstream
consumers (ManifestGroup, MergingSnapshotProducer rewrite paths) work
unchanged.

Read-path dispatch in ManifestFiles is layered:
1. Avro manifests are always legacy (no file inspection).
2. Snapshot-tree callers thread an Integer writerFormatVersion hint through
   the new package-private read overloads: 1 routes to ContentEntryReader,
   0 routes to legacy.
3. Callers without a hint (tests writing-then-reading, ad-hoc tooling) fall
   back to inspecting the Parquet footer schema for field id 134 (content_type)
   or 147 (tracking). The footer read is delegated to InternalParquet via
   DynMethods so core has no compile-time dependency on iceberg-parquet.

Key design choices:
- TrackedFile.schemaWithContentStats omits partition and content_stats when
  their struct types are empty (Parquet rejects empty groups).
- TrackedFileWrapper uses hasPartition/hasContentStats flags to map positions
  dynamically when either optional group is absent.
- V4Writer.add(DataFile) bypasses Delegates.suppressFirstRowId so per-entry
  firstRowId is stored in the tracking struct rather than at manifest level.
- ContentEntryReader.setEntry uses wrapAppendPreservingFirstRowId for ADDED
  entries so firstRowId read from the tracking struct is not re-suppressed.
- ContentEntryAdapter preserves firstRowId for EXISTING entries so uncommitted
  manifests can round-trip per-entry row IDs.
- ContentEntryManifestReaderAdapter applies the same committed/uncommitted
  firstRowId nullification logic as ManifestReader.idAssigner.
- ContentEntryManifestReaderAdapter.iterator tracks ordinal position and sets
  fileOrdinal and manifestLocation on each BaseFile to match Avro reader behavior.
- Parquet.readSchema(InputFile) is a new public helper that returns just the
  Iceberg-converted file schema; InternalParquet.readSchema delegates to it
  for the DynMethods entry point.
- v4 spec forbids content_type=POSITION_DELETES (PR apache#16025); three
  TestManifestReader tests that write standalone position-delete files / DV
  delete files are guarded with assumeThat isLessThan(4) and will be removed
  once PR apache#16677 (or its successor) gates v4 out of the broad parameterized
  test suite during incubation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevenzwu added a commit to stevenzwu/iceberg that referenced this pull request Jul 17, 2026
…add matching reader

V4Writer and V4DeleteWriter now emit content_entry Parquet rows via
TrackedFileWrapper/ContentEntryAdapter rather than the legacy manifest_entry
Avro shape. ContentEntryReader and ContentEntryManifestReaderAdapter project
content_entry rows back to ManifestEntry<DataFile/DeleteFile> so all downstream
consumers (ManifestGroup, MergingSnapshotProducer rewrite paths) work
unchanged.

Read-path dispatch in ManifestFiles is layered:
1. Avro manifests are always legacy (no file inspection).
2. Snapshot-tree callers thread an Integer writerFormatVersion hint through
   the new package-private read overloads: 1 routes to ContentEntryReader,
   0 routes to legacy.
3. Callers without a hint (tests writing-then-reading, ad-hoc tooling) fall
   back to inspecting the Parquet footer schema for field id 134 (content_type)
   or 147 (tracking). The footer read is delegated to InternalParquet via
   DynMethods so core has no compile-time dependency on iceberg-parquet.

Key design choices:
- TrackedFile.schemaWithContentStats omits partition and content_stats when
  their struct types are empty (Parquet rejects empty groups).
- TrackedFileWrapper uses hasPartition/hasContentStats flags to map positions
  dynamically when either optional group is absent.
- V4Writer.add(DataFile) bypasses Delegates.suppressFirstRowId so per-entry
  firstRowId is stored in the tracking struct rather than at manifest level.
- ContentEntryReader.setEntry uses wrapAppendPreservingFirstRowId for ADDED
  entries so firstRowId read from the tracking struct is not re-suppressed.
- ContentEntryAdapter preserves firstRowId for EXISTING entries so uncommitted
  manifests can round-trip per-entry row IDs.
- ContentEntryManifestReaderAdapter applies the same committed/uncommitted
  firstRowId nullification logic as ManifestReader.idAssigner.
- ContentEntryManifestReaderAdapter.iterator tracks ordinal position and sets
  fileOrdinal and manifestLocation on each BaseFile to match Avro reader behavior.
- Parquet.readSchema(InputFile) is a new public helper that returns just the
  Iceberg-converted file schema; InternalParquet.readSchema delegates to it
  for the DynMethods entry point.
- v4 spec forbids content_type=POSITION_DELETES (PR apache#16025); three
  TestManifestReader tests that write standalone position-delete files / DV
  delete files are guarded with assumeThat isLessThan(4) and will be removed
  once PR apache#16677 (or its successor) gates v4 out of the broad parameterized
  test suite during incubation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread format/spec.md
| 104 | **`file_size_in_bytes`** | `long` | *required* | *required* | Total file size in bytes. |
| 146 | **`content_stats`** | `content_stats` struct | *optional* | *optional* | Column stats. See [Column Stats Improvements](#column-stats-improvements). |
| 150 | **`manifest_info`** | `manifest_info` struct | *optional* | *optional* | See manifest_info struct below. |
| 131 | **`key_metadata`** | `binary` | *optional* | *optional* | Implementation-specific key metadata for encryption. |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we'll need a separate key_metadata for DVs to preserve the current V3 capabilities. I find it a bit questionable that we really need separate key metadata for data + DVs, but I'm biasing towards avoiding regressions and avoiding unreadable data on upgrade. If we wanted to consolidate encryption keys on upgrade to v4, we'd have to have some complicated data rewrites on upgrade and more generally there may be some security use cases where a separate key for data + DV are warranted that we want to preserve to avoid regression.

Pasting my message from a chat I had with @rdblue

if there's V3 tables out there that have separate encryption keys for data files and DV, on upgrade to V4 I think we would need to be able to have that representation otherwise tables are potentially unreadable. So as much as I want to simplify, just the chance of there being tables in this position w different DV and data file keys that we may not know about makes it difficult. Unless we impose some writer side requirement to decrypt and consolidate on a shared encryption key on upgrade or something but that seems complciated and would require rewriting data on upgrade which we want to avoid

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.

Once the entry moves to a V4 manifest doesn't the DV have to be embedded in the entry?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does but the point here is more that currently in V3 we have DVs with key_metadaata and data file entries with possibly different key metadata. So I think the entry structure needs to allow for DVs having their own key metadata. On upgrade you'd have a combined entry with a DV which has its own key metadata field, and one for the data file as well.

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.

I guess that's what has me confused. Do we have the potential for writing a V4 Manifest with an external DV? The V3 DV's should stay in V3 Delete manifests where they have their own keys?

@amogh-jahagirdar amogh-jahagirdar Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The V3 DV's should stay in V3 Delete manifests where they have their own keys?

I think V3 stays as V3 on upgrade or implementations can choose to produce new V4 leaf manifests if so desired at the time of upgrade.

Do we have the potential for writing a V4 Manifest with an external DV?

Not sure I fully understand but it would be colocated with the possibility of having the DV metadata be a separate column file.

But I think there's a separate question though of if and how do we model this in V4 to logically enable the same capabilties to prevent a regression in capability of the format. As currently modeled in this PR we only have key_metadata for the whole entry, and we don't distinguish between the data file key metadata and the DV key metadata (but as you said we have this distinction for V3). I think we need to add an additional key_metadata field for the DV to preserve this.

I think we need to preserve this largely for data created prior to upgrade but moved into the new V4 manifests, because otherwise we run the risk of that data not being readable if we force someone moving the existing entries into a single combined entry with no way to express the different encryption keys. More broadly, there may be security cases where people do want this separation (frankly I can't think of anything here since it's all just referencing the same rows in data but I'm also not a security person).

Comment thread format/spec.md
@@ -714,11 +812,10 @@ The `data_file` struct consists of the following fields:
| _required_ | _required_ | _required_ | **`102 partition`** | `struct<...>` | Partition data tuple, schema based on the partition spec output using partition field ids for the struct field ids |

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.

FYI, we recently made it optional per https://github.com/apache/iceberg/pull/17000/changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This section is for v1-v3 , the partition tuple is required to be produced by writers

Comment thread format/spec.md
| 504 | **`added_files_count`** | `long` | *required* | *required* | Count of entries with status ADDED in the manifest. |
| 505 | **`existing_files_count`** | `long` | *required* | *required* | Count of entries with status EXISTING in the manifest. |
| 506 | **`deleted_files_count`** | `long` | *required* | *required* | Count of entries with status DELETED in the manifest. |
| 520 | **`replaced_files_count`** | `long` | *required* | *required* | Count of entries with status REPLACED in the manifest. |

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.

@amogh-jahagirdar we should also introduce modified_files/rows_count fields, right?

@amogh-jahagirdar amogh-jahagirdar Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@stevenzwu I think you are right. In my recent update, I added this. Originally, I left it out because it'd be 1 to 1 with the replaced files count and so I thought it was duplicative metadata but lack of modified manifest level stats is problematic for this reason

  1. A manifest could have only modified entries and from a CDC perspective, we would want to know if we need to read that manifest and knowing if it has modified files is very relevant in order to make that decision

Alternatively we could track total_entry count and derive based on the reminder but that's just complicated and we may as well have each disjoint metric and let callers use them how they want.

The replaced files counts are equally as useful for pruning out entirely irrelevant manifests during scan planning.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

I think it makes sense to add modified_files_count for CDC purposes. Do we have a use case for modified_rows_count or are we adding it for consistency?

amogh-jahagirdar and others added 14 commits August 3, 2026 10:34
- Collapse v1/v2/v3 separate tabs into single v1-v3 tab across all manifest sections
- Add v4 tab to Data File Fields with content_entry, tracking, and deletion_vector structs using Write/Read columns
- Reconcile v4 architecture prose from v4-amt-changes: root manifest concept, Parquet format, partition spec binding, updated terms/glossary

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Aligns the field name with the Java implementation (TrackedFile.FORMAT_VERSION).
Defines the column_file element struct referenced by the column_files
field (158) in the v4 content entry, matching the ColumnFile schema
added in apache#16285.

Co-authored-by: Gabor Kaszab <gaborkaszab@gmail.com>
Co-authored-by: Anurag Mantripragada <amantripragada@apple.com>
Deletion vectors need their own encryption key metadata. Adds field 149
(key_metadata, optional binary) to the deletion_vector struct, matching
apache#17438.
Makes the column_files field type list<159: column_file> to match the
inline element-id convention used by other list fields in the content
entry, matching apache#16285.

Co-authored-by: Gabor Kaszab <gaborkaszab@gmail.com>
Co-authored-by: Anurag Mantripragada <amantripragada@apple.com>
Adds field 524 (modified_files_count) to track entries with status
MODIFIED, the next available id in the 5xx manifest-summary namespace.
Adds field 525 (modified_rows_count) to track rows in entries with
status MODIFIED, the next available id in the 5xx manifest-summary
namespace.
Comment thread format/spec.md
| 522 | **`dv`** | `binary` | *optional* | *optional* | Roaring bitmap of entry positions in the manifest that are not live in the current snapshot. |
| 523 | **`dv_cardinality`** | `long` | *optional* | *optional* | Cardinality of the manifest deletion vector. Must be set when `dv` is non-null; must be null otherwise. |

**`column_file` struct (element 159 of `column_files`, field 158)**

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added this, marked @anuragmantri @gaborkaszab as co-authors

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mostly just to see the whole entry structure together on the same PR

Comment thread format/spec.md
| 144 | **`offset`** | `long` | *required* | *required* | Offset in the file where the content starts. |
| 145 | **`size_in_bytes`** | `long` | *required* | *required* | Length of the referenced content stored in the file. |
| 156 | **`cardinality`** | `long` | *required* | *required* | Cardinality of the deletion vector. |
| 149 | **`key_metadata`** | `binary` | *optional* | *optional* | Implementation-specific key metadata for encryption. |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@gaborkaszab @anoopj @RussellSpitzer @stevenzwu I added the key_metadata to the dv struct as we discussed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tagging @gaborkaszab PR here #17438 just for reference. I think the implementation aligns with the spec (149 as a field ID for this makes sense to me, it's an open ID right after the deletion_vector top level struct)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1, field id 149 is good

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.

It's fine. And thanks for merging the linked PR :)

Comment thread format/spec.md
| Field id | Name | Type | Write | Read | Description |
|----------|------|------|-------|------|-------------|
| 161 | **`format_version`** | `int` | *required* | *required* | Format version of this column file. |
| 162 | **`field_ids`** | `list<163: int>` | *required* | *required* | Live field IDs stored in this column file. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shall we add: Each field_id must appear in at most one column_file within an entry's column_files?

Comment thread format/spec.md
- `manifest_info` must be set when `content_type` is 3 or 4; must be null otherwise.
- `deletion_vector` may only be set when `content_type` is 0; must be null otherwise.
- `equality_ids` must be set when `content_type` is 2; must be null otherwise.
- `column_files` must be null when `content_type` is not 0 or 3.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It might not be clear to the reader on why column_files are allowed on manifests (3). Should we explain it here or somewhere else?

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.

Probably not here with the constraints IMO. In case later on we'll have a dedicated section for describing how we update MDVs (and maybe other stuff in manifests) then it will give enough explanation I think.

Comment thread format/spec.md
| 165 | **`file_format`** | `string` | *required* | *required* | String file format name: `avro`, `orc`, or `parquet`. |
| 166 | **`file_size_in_bytes`** | `long` | *required* | *required* | Total column file size in bytes. |
| 167 | **`key_metadata`** | `binary` | *optional* | *optional* | Implementation-specific key metadata for encryption. |
| 168 | **`split_offsets`** | `list<169: long>` | *optional* | *optional* | Split offsets for the column file. Must be sorted ascending. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I forgot if we discussed adding record_count on column files. Is it useful to have it? @gaborkaszab do you recollect?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it required? With the dense mode of updates isn't the physical record count always going to be the same?

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.

+1 to @amogh-jahagirdar 's point: With the dense representation it's a requirement that all the column files have the same record count as the base file. Hence, record_count is enough on the base file level.

@gaborkaszab gaborkaszab left a comment

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.

Thanks for adding the column file changes @amogh-jahagirdar ! I think the column file snapshot ID is still missing from Tracking.

Comment thread format/spec.md
| 132 | **`split_offsets`** | `list<133: long>` | *optional* | *optional* | Split offsets for the data or equality delete file. Must be sorted ascending. |
| 135 | **`equality_ids`** | `list<136: int>` | *optional* | *optional* | Field ids for row equality in equality delete files. |
| 148 | **`deletion_vector`** | `deletion_vector` struct | *optional* | *optional* | Row-level deletion vector for a data file. |
| 158 | **`column_files`** | `list<159: column_file>` | *optional* | *optional* | Column update files associated with this entry. |

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.

"Column update files associated with this entry"
I wouldn't mention update, simple "Column files". In the future we might want to add column families, where column files aren't the result of an update. I'd leave this flexibility with the comment not breaking later on.

Comment thread format/spec.md
- `manifest_info` must be set when `content_type` is 3 or 4; must be null otherwise.
- `deletion_vector` may only be set when `content_type` is 0; must be null otherwise.
- `equality_ids` must be set when `content_type` is 2; must be null otherwise.
- `column_files` must be null when `content_type` is not 0 or 3.

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.

Probably not here with the constraints IMO. In case later on we'll have a dedicated section for describing how we update MDVs (and maybe other stuff in manifests) then it will give enough explanation I think.

Comment thread format/spec.md
|----------|------|------|-------|------|-------------|
| 0 | **`status`** | `int` (0: EXISTING, 1: ADDED, 2: DELETED, 3: REPLACED) | *required* | *required* | Used to track additions, deletions, and replacements. REPLACED indicates entries with data column updates or `deletion_vector` changes. Deleted entries are required when the snapshot has a non-null parent. Deletes are not used in scans. |
| 1 | **`snapshot_id`** | `long` | *optional* | *optional* | Snapshot ID where the file was added or deleted. Inherited when null. Optional for leaf manifests, required for root. |
| 5 | **`dv_snapshot_id`** | `long` | *optional* | *optional* | Snapshot ID where the deletion vector was added. Inherited when null. Must be null when `deletion_vector` is null. |

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.

If I don't miss anything, then the latest changes don't add the column file related snapshot ID to tracking. I have that in the implementation PR with field ID 160. I used the name latest_column_file_snapshot_id that might be too long, but describes exactly what it is.

Comment thread format/spec.md

| Field id | Name | Type | Write | Read | Description |
|----------|------|------|-------|------|-------------|
| 161 | **`format_version`** | `int` | *required* | *required* | Format version of this column file. |

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.

nit: "this column file" to me this sounds right in the Java class, but feels slightly odd in the spec. Maybe "the column file" instead of this? Other occasions below

Comment thread format/spec.md
| 161 | **`format_version`** | `int` | *required* | *required* | Format version of this column file. |
| 162 | **`field_ids`** | `list<163: int>` | *required* | *required* | Live field IDs stored in this column file. |
| 164 | **`location`** | `string` | *required* | *required* | Location of the column file. |
| 165 | **`file_format`** | `string` | *required* | *required* | String file format name: `avro`, `orc`, or `parquet`. |

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.

Do we want to include the list of supported file format as of now? Who know if and when these change.

Comment thread format/spec.md

When a file is added to the dataset, its content entry must set status to ADDED (1) and store the snapshot ID in which the file was added.

When a data file's deletion vector or column files are updated, the writer must record two content entries for the file in the snapshot: a REPLACED (3) entry for the prior version and a MODIFIED (4) entry for the new, live version. Both entries store the snapshot ID of the update. A MODIFIED data file entry must always have a corresponding REPLACED entry.

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.

For data file relation between REPLACED and MODIFIED entries is symmetrical, but this comment only expresses one direction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's true, we don't want to have a situation where there's a REPLACED without a MODIFIED. I'll find a better way to phrase this.

Comment thread format/spec.md
| _required_ | _required_ | **`file-footer-size-in-bytes`** | `long` | Total size of the statistics file's footer (not the footer payload size). See [Puffin file format](puffin-spec.md) for footer definition. |
| _optional_ | _optional_ | **`key-metadata`** | | Base64-encoded implementation-specific key metadata for encryption. |
| _required_ | _required_ | **`blob-metadata`** | `list<blob metadata>` (see below) | A list of the blob metadata for statistics contained in the file with structure described below. |
| _required_ | _required_ | **`blob-metadata`** | `list<blob metadata>` | A list of the blob metadata for statistics contained in the file with structure described below. |

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.

Not sure if this line of change is relevant to the PR

Comment thread format/spec.md
| 167 | **`key_metadata`** | `binary` | *optional* | *optional* | Implementation-specific key metadata for encryption. |
| 168 | **`split_offsets`** | `list<169: long>` | *optional* | *optional* | Split offsets for the column file. Must be sorted ascending. |

When a file is added to the dataset, its content entry must set status to ADDED (1) and store the snapshot ID in which the file was added.

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.

How do we cover the status transitions: e.g. a file/manifest starts with ADDED, but the next commit it flips to EXISTING (unless it got deleted/replaced). Same for MODIFIED -> EXISTING transition.

But for files that live in leaf manifests, we don't want to rewrite manifests for the sake of doing the status transition. For instance, if we modified DVs on a bunch of files, we might write a new leaf manifest with a bunch of MODIFIED entries (and the manifest status itself would have an ADDED?), but in the next commit, the leaf status might change, but the entries inside the leaf stays MODIFIED.

@amogh-jahagirdar amogh-jahagirdar Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How do we cover the status transitions: e.g. a file/manifest starts with ADDED, but the next commit it flips to EXISTING (unless it got deleted/replaced). Same for MODIFIED -> EXISTING transition.

I'm not sure if we explicitly need to call out the ADDED/MODIFIED -> EXISTING transitions? I think the only state transitions we need to call out are the ones where it's a net new change in a given snapshot.

If you look at the current spec (not these changes but the ones for v1-v3), we never really needed to call out the explicit EXISTING transition because anything that was a new change, like ADDED/DELETED had explicit mentions. After defining those, it's very reasonably implied and via the naming of the status EXISTING that any carry over should just be marked as EXISTING.

If we want to have EXISTING be this explicit transition I think we can do it but we'll want to be not overly-specified to the point where we prevent certain write patterns (more on that in the next response).

But for files that live in leaf manifests, we don't want to rewrite manifests for the sake of doing the status transition. For instance, if we modified DVs on a bunch of files, we might write a new leaf manifest with a bunch of MODIFIED entries (and the manifest status itself would have an ADDED?), but in the next commit, the leaf status might change, but the entries inside the leaf stays MODIFIED.

Agree, we definitley don't want to have to rewrite manifests just to update statuses, that would be a crazy bad requirement but I don't know if that means we need to codify explicit transition rules for EXISTING in the spec. I just worry that if we do that too strictly it may be misinterpreted and prevent valid write scenarios.

Every additional statement we make in the spec bears the risk of misinterpretation which is where I'm coming from that we need to be careful.

Concretely:

Status just indicates what the status of that file in the tree is (whether it's a data file or a manifest is inconsequential)

Assume we stick with the status quo of not epxlicitly defining when to state transition to Existing and rather it's implied that anything that's not a new change (not ADDED/DELETED/MODIFIED/REPLACED) is EXISTING:
Both options of the following options are valid:

1/ In the scenario you described, a writer can choose to mark the leaf manifest as EXISTING
or
2/ A writer can choose to actually just write a new manifest with EXISTING entries and mark the old manifest as DELETED.

Could we explicitly enumerate every single state transition or find some wording that generalizes across use cases? Probably, but I think I'd rather just be explicit cases where we know specific statuses are required to be produce (like DV changes/column updates producing REPLACED + MODIFIED, file removals explicitly producing DELETED).

That's a smaller state to enumerate and less chance of misinterpretation preventing legitimate write scenarios. Every other status is just naturally implied and readers/change detection should correctly handle those, unless there's some ambiguous case we're worried about.

Let me know what you think @anoopj , cc @stevenzwu @RussellSpitzer @rdblue

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 feel like how we manage the state transitions is rather straightforward: transitions only happen when the metadata is modified. So if a leaf file is not being rewritten, all of the status remain as they were when originally produced.

The important aspect of this is that the status of entries within the manifest file represents the state as of the snapshot where the manifest was written. If you later rewrite a manifest and see that there were entries that had a ADDED, MODIFIED or REPLACED they would then transition to EXISTING within the new manifest and that is now the effective state as of that new snapshot.

This isn't different than how v3 and prior work. Consider an append only table that only produces new files and new metadata files (no metadata rewrite). All entries will have a status of ADDED forever. We don't go back and rewrite all of the entries for every commit to be EXISTING.

We shouldn't have to rewrite older metadata, rather we should interpret in context of the when it was written.

dhruvarya-db added a commit to dhruvarya-db/iceberg that referenced this pull request Aug 7, 2026
Opening this to drive discussion on what the REST catalog spec needs for
format version 4. It is a proposal rather than a merge candidate: parts of it
depend on table spec changes that are still in review, and one change is
deliberately breaking. Details and open questions below.

Format version 4 restructures table metadata, and three constraints in the
current spec prevent a catalog from serving a v4 table at all: format-version
is capped at 3, Snapshot requires manifest-list, and CommitTableResponse
requires metadata-location.

Changes:

- TableMetadata: raise the format-version maximum to 4, and document that
  location is optional in v4, where it may be supplied by the catalog rather
  than carried in metadata.

- Snapshot: add root-manifest and make manifest-list optional, with a oneOf
  requiring exactly one of the two. A snapshot with neither is invalid and must
  not be read as a snapshot with no files. Which of the two applies is
  determined by format-version, which cannot be expressed here because that
  field lives in the enclosing TableMetadata, so it is stated normatively.

- LoadTableResult and CommitTableResponse: add table-location, which carries the
  table's base location for resolving relative paths. Format version 4 makes
  TableMetadata.location optional while allowing relative paths, so a table can
  have metadata that is unreadable without a catalog-supplied base.
  CommitTableResponse no longer requires metadata-location, and both responses
  document that it may be absent when the catalog is the source of truth for
  table state and no client-visible metadata pointer exists.

- UnregisterTableResult: add table-location, so the endpoint that hands back a
  metadata location cannot return metadata with no base to resolve against.

- Move the table endpoints to /v2. Every path whose response can carry table
  metadata moves: tables, tables/{table}, register, and tables/{table}/unregister.

Open questions:

- root-manifest is not in the ratified spec. It comes from the adaptive metadata
  tree proposal (apache#16025), which is still open, so the field name and shape may
  change. Note also that SnapshotParser currently writes manifest-list for v4
  tables, so the oneOf as written would reject snapshots this repository
  produces today. Whether to keep the field, defer it, or make it purely
  additive is the main thing worth discussing.

- Moving the table endpoints to /v2 rather than adding /v2 alongside /v1 is a
  breaking change, and assumes v1 is no longer supported. An additive version
  that leaves /v1 in place is the obvious alternative if that assumption does
  not hold.

- Whether path versioning is the right mechanism at all, versus a header or a
  content-type parameter.

- Relative path support and the typed content stats are already in the spec for
  v4; snapshot offloading is not yet, and would interact with the snapshots
  query parameter on loadTable if it lands later.

make lint and make generate both pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Iceberg V4 Iceberg Table Format Version 4 Specification Issues that may introduce spec changes.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

7 participants