Skip to content

feat(manifest): support writing delete manifests in v2 - #4002

Open
ghoshp83 wants to merge 1 commit into
apache:mainfrom
ghoshp83:manifest-writer-delete-content
Open

ghoshp83 wants to merge 1 commit into
apache:mainfrom
ghoshp83:manifest-writer-delete-content

Conversation

@ghoshp83

Copy link
Copy Markdown

Rationale

ManifestWriterV2 hardcodes its content type: content() returns
ManifestContent.DATA unconditionally and _meta writes "content": "data".
PyIceberg can therefore only ever write data manifests, even though it reads
delete manifests fine and ManifestFile.content is a first-class field.

The asymmetry is already visible in the codebase: ManifestListWriterV1.prepare_manifest
raises Cannot store delete manifests in a v1 table, a guard that only makes sense
if a v2 table were expected to be able to produce one.

The practical consequence is that any maintenance operation that needs to rewrite
delete manifests has to subclass ManifestWriterV2 and override two private members
to do it. #3925 describes one such case.

Change

  • write_manifest(...) takes content: ManifestContent = ManifestContent.DATA
    and passes it to ManifestWriterV2.
  • ManifestWriterV2 stores it; content() returns it and _meta emits
    "data" or "deletes" to match.
  • format_version=1 with a non-data content raises ValidationError, mirroring
    the existing manifest-list guard — v1 has no delete files.

The default is unchanged, so every existing caller keeps writing data manifests.

Tests

Added to tests/utils/test_manifest.py:

  • test_write_manifest_content — parametrized over DATA/DELETES; asserts the
    round-tripped ManifestFile.content and the Avro content metadata key agree
    with what was requested.
  • test_write_manifest_defaults_to_data_content — callers that pass nothing still
    get a data manifest.
  • test_write_manifest_v1_rejects_delete_content — v1 refuses loudly.

tests/utils/test_manifest.py: 48 passed. Lint and format clean.

Note

#3624 (writing V3 manifests) adds a ManifestWriterV3 subclass and touches the
same dispatch function, but does not change the hardcoded content — the two are
adjacent rather than overlapping. Whichever lands second will need a trivial
rebase.

ManifestWriterV2 hardcoded content() to ManifestContent.DATA and wrote
"content": "data" into the Avro metadata, so PyIceberg could not produce a
delete manifest even though it reads them and ManifestListWriterV1 already
guards against storing one in a v1 table.

Thread an optional content through write_manifest and ManifestWriterV2,
defaulting to DATA so existing callers are unaffected, and reject a non-data
content for format version 1.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant