Manifest: Use int for equality_ids in manifest schema per Iceberg spec (#3840)

[hedger9487]

Description

Fixes #3840.

In the Iceberg specification (format/spec.md), Java reference implementation, and iceberg-cpp, equality_ids (field 135) is defined as a list of int (list<136: int>). However, pyiceberg previously declared field 136 with LongType(), causing manifests written by PyIceberg to be rejected by other spec-conformant readers (e.g. iceberg-cpp).

This PR:

1. Updates DATA_FILE_TYPE in pyiceberg/manifest.py for table format versions 1, 2, and 3 to use IntegerType() for element id 136 in equality_ids.
2. Updates ReadSchemaResolver.primitive in pyiceberg/avro/resolver.py to allow promoting LongType in file schema to IntegerType in read schema for Avro decoding (since both are encoded as zigzag varints on the wire), ensuring backward compatibility when reading historical manifests written by earlier versions of PyIceberg.
3. Adds unit tests verifying that newly written manifests serialize equality_ids with element type int, and that historical manifests with element type long can be read seamlessly.

Testing

• Added unit tests in tests/utils/test_manifest.py.
• All 42 manifest tests and full test suites pass locally.

[Copilot]

Pull request overview

Updates manifest equality_ids to use Iceberg’s specified int type while supporting legacy manifests.

Changes:

• Corrects manifest schemas for format versions 1–3.
• Adds legacy long-to-int decoding support.
• Adds serialization and compatibility tests.
• Resolver compatibility must be scoped to equality_ids; the current broad handling can accept out-of-range values.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Summary
tests/utils/test_manifest.py	Tests current serialization and legacy manifest reading.
pyiceberg/manifest.py	Uses IntegerType for equality_ids.
pyiceberg/avro/resolver.py	Adds long-to-int compatibility, but the implementation is overly broad.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[rambleraptor]

Oof. This isn't great. I'd like to do some digging to verify all of this.

Here's my initial thoughts:

• We should start writing int if that's the correct behavior.
• We need a config flag to write longs (to conserve backwards compatibility)
• If we see a long, we should log a warning message. These are non-compliant manifests. There should also be a deprecated message.
• We should remove the long support completely within the next two versions.

[hedger9487]

Thanks for the thoughtful feedback @rambleraptor!

I have updated the PR accordingly:

1. Default behavior: Writes int (IntegerType) for equality_ids per the Iceberg spec.
2. Backwards compatibility flag: Added write.manifest.legacy-long-equality-ids table property and legacy_equality_ids: bool = False parameter in write_manifest / ManifestWriter so users who require legacy long equality_ids during migration can still opt in.
3. Deprecation warning & logging: Emits a DeprecationWarning when reading manifests with legacy long equality_ids, informing users that long equality_ids are non-compliant and that support will be dropped in a future release.
4. Added comprehensive unit tests covering both the deprecation warning on read and the legacy write config option.

All tests pass cleanly. Ready for another review!

[kevinjqliu]

thanks for the PR @hedger9487
I'd like to propose this long-term fix: #3840 (comment)

The main difference with the current PR is to not introduce a new compatibility flag. And instead allow pyiceberg to read equality_ids with long type only so that we are able to fix forward and rewrite those erroneous files

Please let me know what you think!

[hedger9487]

Thanks for the suggestion, @kevinjqliu!

Connecting this fix with a future rewrite_manifest maintenance action is a great point that I hadn't considered.

I have updated the PR to align with this proposal:

1. Write path: Corrected equality_ids in DATA_FILE_TYPE to IntegerType (element ID 136).
2. Read path: Tolerated Long -> Int strictly for manifest DataFile equality_ids (element ID 136) in ReadSchemaResolver.primitive, ensuring backwards compatibility for legacy manifests without leaking type relaxation to user tables or other fields.
3. Tests: Added coverage for writing compliant int manifests, reading legacy long manifests, and verifying resolver isolation.

All tests and pre-commit checks are passing cleanly. I'll also be working on the rewrite_manifests maintenance operation in a follow-up PR next.

Ready for another review whenever convenient, thanks!

[kevinjqliu]

seems like there are a few unrelated changes in the PR

we should tighten the scope to include

• changing default from long -> int
• fallback read for long type w/ tests

we could split up (1) into its own PR. for (2), im curious what the best way to do this, i'll check with my agent 😄

[kevinjqliu]

Thanks for iterating on this @hedger9487! I took a pass at simplifying the read-side fallback and pushed a reference branch: main...kevinjqliu:iceberg-python:claude/schema-long-int-avro-fallback-4ec219

The main differences from this PR:

• Match on the exact field id path [2, 135, 136] (data_file.equality_ids.element) instead of 136 in self.context plus the read_types name check. This keeps the exception scoped without the resolver needing to know about DataFile.
• Return the existing IntegerReader for the legacy case, since ints and longs share the same Avro encoding. No new reader class needed.
• One test that writes a manifest with the legacy list<long> schema and reads it back, replacing the flag-based tests.

Would you mind adopting that shape here? Happy to take it over if you'd prefer, just let me know. Once this lands we can follow up with the rewrite_manifests maintenance action from the plan in #3840.

[hedger9487]

Thanks @kevinjqliu! That’s so much cleaner and more scoped.

I've adopted your reference branch shape directly:

1. Matched on the exact field ID path [2, 135, 136], decoupling the resolver from DataFile.
2. Directly return IntegerReader() for the legacy fallback.
3. Replaced earlier tests with the monkeypatched end-to-end legacy manifest round-trip test, and reverted the unrelated cache/fixture diffs.

All pre-commit hooks, linters, and unit tests are green locally. I've rebased onto the latest main, credited you as co-author, and force-pushed the updated branch!