fix(avro): keep the table readable after dropping an old partition column

[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.
• unknown is stored in Avro as "null".
• When the manifest is rewritten, PyIceberg makes the field optional by wrapping it as ["null", <type>], which gives ["null", "null"].
• Avro doesn't allow the same type twice in a union. PyIceberg fails with IndexError, Spark with Duplicate in union:null.

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.
• So today this only affects people running main. Once 0.13.0 is released, it would affect everyone who upgrades.

Fix

• If a field's type is unknown, write it as plain "null" with no union. It's changed in the 3 places that build the Avro schema and the writers.
• This is what Java does: unknown maps to NULL (TypeToSchema.java), and NULL is never wrapped in a union (AvroSchemaUtil.java).
• Java has written unknown this way since Avro: Support timestamp(9) and unknown types iceberg#12455. Register to_bytes for UnknownType #3899 ported a different part of Java's fix for this bug (Core, Spark: Handle unknown type during deletes iceberg#14356), but not this one.

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:
  • tests/avro/test_file.py::test_write_optional_unknown_type: write an optional unknown field, read it back.
  • tests/catalog/test_catalog_behaviors.py::test_delete_after_dropping_old_partition_source_column: the example above, v1 and v2, every catalog fixture.
  • All 7 fail on main and pass with this change (pure Python and Cython Avro decoders).
• Unit tests on Linux: 4134 passed vs 4127 on main; the difference is the 7 new tests. 9 other failures are the same on both (optional packages I don't have).
• Integration tests (Docker): 1348 passed. The other 10 need ray or daft, which I don't have installed, and fail the same way on main.
• Spark check: ran the example on the REST catalog and read the table with Spark. main: Duplicate in union:null. This PR: [2] for v1 and v2.
• ruff check and ruff format --check pass.

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, with or without this change. That's a separate issue.
• Files already written as ["null", "null"] by an unreleased main build are not repaired.

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.

I tried it locally first and ran into one case I wasn't sure how to handle, so I'd really appreciate your guidance. Skipping the field works for a plain scan. But when the dropped field comes before another partition field, the partition record read back from the manifest is one field short, so the fields after it shift, and filtering on them fails.

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

With this PR as it is (unknown written as null), both scans return ids 2 and 3.

If I understand correctly, reading the skipped field back as null would mean giving manifest reads the spec's partition type, which they don't have today (fetch_manifest_entry reads the partition struct from the file's own schema). I'd be glad to add that to this PR.

In case it's useful: I originally went with null because the Avro type mapping in the spec lists unknown as "null or omitted", and Java writes it as null (TypeToSchema.java). I should have mentioned that in the description, sorry about that.

Would you prefer that I go ahead with skipping the field and update the manifest reads, or keep the null form for now? I'll follow whichever you think is best. Thanks again for your time!