hzmsh opened a new pull request, #4115:
URL: https://github.com/apache/iceberg-python/pull/4115
# 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.
```python
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 #3899 (including the 0.12.0 release): step 3 raises an error. The
delete fails, but the table is untouched and still readable.
- After #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](https://github.com/apache/iceberg/blob/9120cfa789d4ea29acd22c95973774f6a813a212/core/src/main/java/org/apache/iceberg/avro/TypeToSchema.java#L216-L217)),
and `NULL` is never wrapped in a union
([AvroSchemaUtil.java](https://github.com/apache/iceberg/blob/9120cfa789d4ea29acd22c95973774f6a813a212/core/src/main/java/org/apache/iceberg/avro/AvroSchemaUtil.java#L181-L192)).
- Java has written unknown this way since apache/iceberg#12455. #3899 ported
a different part of Java's fix for this bug (apache/iceberg#14356), but not
this one.
**Reports from users**
- apache/iceberg#14343: same steps in Java (remove partition field, drop the
column, then delete). Fixed in apache/iceberg#14356.
- apache/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.
<!-- In the case of user-facing changes, please add the changelog label. -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
