Support writing V3 manifests and manifest lists#3624
Open
moomindani wants to merge 4 commits into
Open
Conversation
Add ManifestWriterV3 and ManifestListWriterV3. The manifest writer uses the V3 record layout so the V3-only data file fields (first_row_id, referenced_data_file, content_offset, content_size_in_bytes) are written, rebinding V2-layout records when needed. The manifest list writer implements the spec's First Row ID Assignment: preserving existing first_row_id values, never assigning one to delete manifests, and assigning a running value advanced by existing and added row counts to unassigned data manifests. It also exposes next_row_id for the commit path to update the table's next-row-id. Closes apache#3620
moomindani
force-pushed
the
moomindani/v3-manifest-write
branch
from
July 8, 2026 12:05
36f5b56 to
d161e78
Compare
Carry an optional first_row_id through ManifestWriterV3 into the produced manifest file (used when rewriting manifests whose first_row_id is known), matching ManifestFiles.newWriter in Java. Expand tests for parity with TestManifestWriterVersions and TestManifestListVersions: reader compatibility of V3-written files, metrics field round-trips, null first_row_id when reading V2 manifest lists with the V3 layout, and preservation of writer-carried first_row_id without advancing next_row_id.
Assigning a first-row-id while treating unknown existing/added row counts as zero would let subsequent manifests overlap row ID ranges. Raise a clear error instead, matching the reference implementation which requires the counts. Also drop the always-None value from the first-row-id-required error message and document the record layout invariant behind the V3 rebinding.
abnobdoss
reviewed
Jul 19, 2026
abnobdoss
left a comment
abnobdoss
left a comment
Contributor
There was a problem hiding this comment.
Thanks for picking up the v3 writer work! The first-row-id assignment lines up with the Java writer nicely; I left a few comments, mostly on the read version handling.
Contributor
There was a problem hiding this comment.
Per the comment on _wrap_data_file: we likely need to go through all the uses of DEFAULT_READ_VERSION, including this one. Reading a v3 manifest list here drops first_row_id, which is what makes a round trip fail. fetch_manifest_entry has the same issue for the new data file fields.
| so that is the layout to zip the positional data against. | ||
| """ | ||
| if len(manifest_file._data) >= len(MANIFEST_LIST_FILE_SCHEMAS[3].fields): | ||
| return copy(manifest_file) |
Contributor
There was a problem hiding this comment.
copy(manifest_file) doesn't seem to work in its current form: the copy and the original share the same underlying _data list, so writing to one writes to both.
| if len(data_file._data) >= len(DATA_FILE_TYPE[3].fields): | ||
| return data_file | ||
| args = { | ||
| field.name: value for field, value in zip(DATA_FILE_TYPE[DEFAULT_READ_VERSION].fields, data_file._data, strict=True) |
Contributor
There was a problem hiding this comment.
Should this be DEFAULT_READ_VERSION? It assumes any record that isn't v3 is v2, but DATA_FILE_TYPE has 15 fields for v1, 16 for v2 and 20 for v3, so a v1-bound DataFile would fail the strict zip with a confusing error.
Reading V2/V3 manifests and manifest lists used a fixed V2 read schema, so V3-only fields (first_row_id, referenced_data_file, content_offset, content_size_in_bytes) were silently dropped by Avro schema resolution, breaking V3 round trips. Read with the latest known schema instead, keeping the V2 default for constructing and writing records unchanged. Also fixes ManifestListWriterV3._wrap sharing _data with the source record via a shallow copy, and rebinding a record to the V3 layout against a fixed V2 field count, which raised on V1-bound records.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3620 (part of #1551)
Rationale for this change
Adds the writer side for V3 manifests and manifest lists (the V3 read schemas already exist):
ManifestWriterV3: writes manifest entries using the V3 record layout so the V3-only data file fields (first_row_id,referenced_data_file,content_offset,content_size_in_bytes) are actually written — with the default V2 record layout they would silently serialize as null. Data files bound to the V2 layout are rebound to V3.ManifestListWriterV3: implements the spec's First Row ID Assignment: existingfirst_row_idvalues are preserved, delete manifests are never assigned one, and unassigned data manifests get a running value starting at the snapshot'sfirst-row-id, advanced byexisting_rows_count + added_rows_count. The writer exposesnext_row_idso the commit path can update the table'snext-row-id(wiring is Support row lineage assignment on commit #3621). Metadata carriesfirst-row-idandformat-version: 3, matching the JavaManifestListWriter.V3Writer.This is writer-level only: nothing produces V3 snapshots yet (that's #3621, with table-level enablement in #3551 / #3622).
Are these changes tested?
Yes, five new tests: a V3 manifest round-trip verifying the V3 data file fields are written (including the V2-to-V3 rebinding path), first-row-id assignment across unassigned/preserved/delete/second-unassigned manifests (asserting
[1000, 77, null, 1125]andnext_row_id == 1130), and validation thatfirst_row_idis required. All fail without the implementation and pass with it. No new failures intests/table,tests/catalog,tests/avro.Are there any user-facing changes?
No user-facing behavior changes; this adds writer capabilities used by upcoming V3 write support.
This pull request and its description were written by Claude Fable 5.