[hzmsh]

Rationale for this change

If you stop partitioning by a column and then drop that column, the next delete() breaks the table. The commit succeeds, but after that neither PyIceberg nor Spark can read the table.

import pyarrow as pa

from pyiceberg.catalog.memory import InMemoryCatalog

catalog = InMemoryCatalog("default")
catalog.create_namespace("default")
data = pa.Table.from_pydict({"id": [1, 2], "category": ["a", "b"]})
tbl = catalog.create_table("default.t", schema=data.schema)
with tbl.update_spec() as update:
    update.add_identity("category")
tbl.append(data)

with tbl.update_spec() as update:
    update.remove_field("category")   # 1. stop partitioning by category
with tbl.update_schema() as update:
    update.delete_column("category")  # 2. drop the column

tbl.delete("id == 1")                 # 3. commits fine
tbl.scan().to_arrow()                 # main: IndexError, expected: id = [2]

What goes wrong

• The old partition spec still has a category field, but the column is gone, so its type becomes unknown.
• When step 3 rewrites the manifest, unknown maps to Avro "null", so the optional field becomes the union ["null", "null"], which Avro does not allow. Reading that manifest back fails: IndexError in PyIceberg, Duplicate in union:null in Spark.

When it started

• Before Register to_bytes for UnknownType #3899 (including the 0.12.0 release): step 3 raises an error. The delete fails, but the table is untouched and still readable.
• After Register to_bytes for UnknownType #3899 (merged to main, not released yet): step 3 succeeds but writes a broken manifest, so the whole table becomes unreadable.

Fix

The spec says unknown is not stored in data files, and its Avro mapping is "null or omitted". This PR omits it:

• Write (ManifestWriter._with_partition): unknown partition fields are left out of the manifest's Avro schema. The in-memory partition record keeps all fields, and the Avro writer already matches file fields to record positions by field ID, so nothing else changes on the write path.
• Read (fetch_manifest_entry): today the partition tuple is read with only the fields in the file. If category is omitted from a (category, region) spec, the tuple comes back as Record('y') instead of Record(None, 'y'), so reading region at index 1 fails with IndexError (or reads a later field's value if more fields follow). The reader now takes the field list from the partition-spec in the manifest header and adds any omitted field back as an optional field, which reads as None. Manifests without omitted fields are read exactly as before. To do this, AvroFile now also accepts a function that builds the read schema from the file header.
• Java passes the table's partition specs (specsById) into its manifest reader. Reading the spec from the header is its fallback, which Java marks as deprecated (ManifestReader.java#L156-L164). fetch_manifest_entry has no spec argument and has 12 call sites, so this PR reads the header and leaves the signature unchanged. Passing specs in could be a follow-up if preferred.

Reports from users

• Removing partitionField and dropping partition columns will cause operations with delete files to fail. iceberg#14343: same steps in Java (remove partition field, drop the column, then delete). Fixed in Core, Spark: Handle unknown type during deletes iceberg#14356.
• Spark: Dropping partition column from old partition table corrupts entire table iceberg#10234: Spark users also had their tables break after dropping an old partition column (an older, different error).

Are these changes tested?

Yes.

• New tests (14 cases across catalogs and v1/v2; 13 fail on main and pass here, and 1 is a regression test that already passes on main):
  • tests/catalog/test_catalog_behaviors.py::test_delete_after_dropping_old_partition_source_column: the example above (v1 and v2, every catalog fixture), and the omitted field reads back as None.
  • tests/catalog/test_catalog_behaviors.py::test_filter_after_dropping_earlier_partition_source_column: partitioned by category and region, category dropped. Filtering on region returns the right rows after the first delete, and again after a second delete that reads and rewrites the same manifest.
  • tests/utils/test_manifest.py::test_write_manifest_skips_unknown_partition_field: the unknown field is not in the written Avro schema and reads back as None, with the next field in place.
  • tests/utils/test_manifest.py::test_read_manifest_with_unknown_partition_field_written_as_null: a manifest that writes the unknown field as Avro "null" (as Java does) still reads correctly.
• Unit tests (-m unit, with the Cython decoder built) on Linux: 4500 passed vs 4486 on main; the difference is the 14 new cases. 12 other failures are the same on both (optional packages I don't have, for Kerberos and Azure).
• Integration tests (Docker): 1352 passed. The other 10 need ray or daft, which I don't have installed, and fail the same way on main.
• Spark check (REST catalog, v1 and v2) with the two-field example: after PyIceberg's delete, Spark returns ids [2, 3] for region = 'y'. After a Spark DELETE of id = 2 (Java writes the field as Avro "null"), PyIceberg returns [3]. Spark also reads [2] for the single-field example above (v1 and v2).
• make lint passes (ruff, ruff format, mypy, codespell).
• Reading 300 manifests, partitioned or not, takes about the same time as on main (within 3%, best of 5).

Are there any user-facing changes?

Yes, a bug fix.

• delete() and overwrite() after dropping an old partition column now write readable manifests.
• Not covered:
  • On v1 tables, overwrite() in this case still fails earlier with Could not find field with id: 2 (the dropped category column), with or without this change.
  • tbl.inspect.entries() and tbl.inspect.partitions() fail with the same error after dropping an old partition column, also on main and with or without a delete. That's a separate issue.
  • Files already written as ["null", "null"] by an unreleased main build are not repaired.
  • Open PRs Support writing V3 manifests and manifest lists #3624 and Support row lineage assignment on commit #4102 each add a v3 manifest writer with its own new_writer. If either merges first, I'll add skip_unknown=True there in this PR; if this PR merges first, they need the same one-line change.

Since #3899 isn't released yet, could this go into the 0.13.0 milestone?

---

Written with AI assistance (Claude Code, Claude Opus 5.5). The Java references were read from apache/iceberg main, and the tests above were run on this branch.

[Fokko]

Thanks @hzmsh for fixing this. The underlying problem is that we should not write null to the Avro file:

So we should skip over it when writing, and read it as null.

[hzmsh]

Hi @Fokko, thank you very much for the review and for pointing me to the spec. That makes sense, and I'm happy to change the PR to skip the field when writing and read it back as null.

For the "read it back as null" part, I noticed that fetch_manifest_entry reads the partition struct with only the fields that are in the file. Once an earlier field is skipped, the partition record comes back one field short (Record('y') instead of Record(None, 'y')), and the partition fields after it shift:

Example

data = pa.Table.from_pydict({"id": [1, 2, 3], "category": ["a", "b", "a"], "region": ["x", "y", "y"]})
tbl = catalog.create_table("default.t", schema=data.schema)
with tbl.update_spec() as u:
    u.add_identity("category")
    u.add_identity("region")
tbl.append(data)
with tbl.update_spec() as u:
    u.remove_field("category")
with tbl.update_schema() as u:
    u.delete_column("category")
tbl.delete("id == 1")

tbl.scan().to_arrow()                              # works either way
tbl.scan(row_filter="region == 'y'").to_arrow()    # with the field skipped: IndexError in the partition evaluator

So I'll also update fetch_manifest_entry to add omitted fields back as null using the partition-spec in the manifest header, and add a test for the case above. Thanks again for your time!

[hzmsh]

Hi @Fokko, I updated the PR as you suggested: unknown partition fields are now skipped when writing the manifest, and fetch_manifest_entry adds them back from the partition-spec in the manifest header, so they read as null. Tests cover one and two partition fields. Ready for another look when you have a moment. Thank you!