Repository navigation
Skip unknown nested fields when resolving Avro read schemas - #4289
Conversation
Fields are resolved by ID, so a file field that is not in the read schema should be read and dropped. This already worked for primitives, but a file-only map, list, or struct raised ResolveError, e.g. when a writer adds extra map fields to data_file in manifests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
AP24110011695
left a comment
There was a problem hiding this comment.
Reviewed the diff against ReadSchemaResolver and ran the new test locally (Windows, Python 3.12, pure-Python decoder fallback): test_read_manifest_entry_with_unknown_nested_fields passes.
I also probed the mismatch guarantees that the new None pass-through must not weaken — all four still raise on this branch, including the field-level paths the modified accessors serve (file map vs read string, file list vs read map, both top-level and nested inside a struct).
One suggestion: none of the tests here pin a field-level mismatch as still raising — the existing not-aligned tests in test_resolver.py all resolve at the top level, which bypasses field_partner + the container accessors this PR touches. Since the change widens what those accessors accept, a small regression test would lock the boundary:
def test_field_level_nested_mismatch_still_raises() -> None:
file_struct = StructType(
NestedField(10, "x", MapType(11, IntegerType(), 12, BinaryType()), required=False)
)
read_struct = StructType(NestedField(10, "x", StringType(), required=False))
with pytest.raises(ResolveError):
resolve_reader(file_struct, read_struct)Also confirmed the fallback direction reads correctly: inside a file-only parent there is no read partner to mismatch against, so nested children resolving to read-and-drop is the right call.
Fokko
left a comment
There was a problem hiding this comment.
Good call @haizhou-zhao Thanks!
…of-structs * upstream/main: (78 commits) fix(io): keep null lists of structs when projecting Arrow data (apache#4065) Skip unknown nested fields when resolving Avro read schemas (apache#4289) fix(expressions): reject non-finite floats on conversion to Decimal (apache#4287) fix: handle negative-scale Decimals in increment, decrement and truncate_decimal (apache#4285) Match NaN partition values with `IsNaN` in partition predicates (apache#4281) Fix append when a data column is named like a partition helper column (apache#4344) Widen decimal literal precision in scan filters (apache#4348) Add row-lineage support with Spark tests (apache#3972) Serialize purgeRequested as a lowercase boolean in REST drop_table (apache#4341) Validate each branch against its own commits on retry (apache#4317) Core: Compare upsert rows column by column instead of cell by cell (apache#4334) Core: Write added files under the partition spec they were planned with (apache#4338) Core: Check whether a failed last commit attempt landed before cleanup (apache#4339) REST: Use the standard User-Agent format (apache#4330) SQL: Accept an existing SQLAlchemy engine in SqlCatalog (apache#4336) Expose catalog labels on Table (apache#4321) Respect REST endpoint capabilities when checking view existence (apache#4337) Reject existing ref names in create_branch and create_tag (apache#4319) Assert every ref a transaction writes, not just the first (apache#4315) feat: send Idempotency-Key on REST mutations when the server advertises it (apache#4310) ... # Conflicts: # pyiceberg/io/pyarrow.py # tests/io/test_pyarrow.py
Closes #4288
Rationale for this change
The spec resolves fields by ID and asks readers to be permissive: its manifest read rules say a field the reader's version doesn't define should be ignored ("Ignore the field"). Iceberg Java/Spark, DuckDB, and Snowflake skip unknown manifest fields of any type. PyIceberg skipped them only when they were primitives. A file-only map, list, or struct instead raised
ResolveError: File/read schema are not aligned for map, got None, becauseSchemaPartnerAccessorraised whenever the partner wasNone. As a result, PyIceberg couldn't scan tables whose manifests carry extra nested fields, which other readers handle.With this change, the accessor returns
Nonefor the children of a field that has no partner, and the existingNonehandling inReadSchemaResolverreads and drops them. A real type mismatch, such as a map in the file and a string in the read schema, still raises.Are these changes tested?
Yes.
test_read_manifest_entry_with_unknown_nested_fieldswrites a v2 manifest entry whosedata_filehas an unknown map, list, and struct (with a nested map) placed before known fields. It then checks that the entry reads back unchanged. The test fails onmainwith theResolveErrorabove.Are there any user-facing changes?
Yes, a bug fix: PyIceberg can read manifests that contain unknown nested fields, matching other Iceberg implementations.