Skip to content

fix(inspect): handle 4-byte bounds in _readable_bound after int/float type promotion (#3744) - #3771

Open
ArjunPakhan wants to merge 2 commits into
apache:mainfrom
ArjunPakhan:fix/inspect-promoted-type-bounds
Open

fix(inspect): handle 4-byte bounds in _readable_bound after int/float type promotion (#3744)#3771
ArjunPakhan wants to merge 2 commits into
apache:mainfrom
ArjunPakhan:fix/inspect-promoted-type-bounds

Conversation

@ArjunPakhan

Copy link
Copy Markdown

Fixes #3744

Summary

When a table undergoes spec-allowed type promotion (intlong or floatdouble), pre-existing manifest files retain their 4-byte bounds (IntegerType / FloatType). Currently, _readable_bound passes these bytes directly to from_bytes(field.field_type, bound). Because field.field_type is updated to LongType() / DoubleType(), from_bytes attempts an 8-byte unpack (_LONG_STRUCT.unpack(b)), raising struct.error: unpack requires a buffer of 8 bytes.

Changes Made

  • Updated _readable_bound in pyiceberg/table/inspect.py to inspect byte lengths before decoding.
  • If field_type is LongType / DoubleType and len(bound) == 4, decode using IntegerType() / FloatType().
  • Added unit tests in tests/table/test_inspect.py covering promoted bound deserialization for LongType and DoubleType.

Testing

  • python -m pytest tests/table/test_inspect.py (5/5 tests passed).

@tasty0tomato

Copy link
Copy Markdown

Thanks for picking this up @ArjunPakhan!

The diff here only contains tests/table/test_inspect.py — there's no change to
pyiceberg/table/inspect.py. On main, _readable_bound is still a plain
from_bytes(field_type, bound), so test_readable_bound_type_promotions should
fail with the same struct.error. Looks like the source change didn't make it
into the commit.

Two notes for when it does:

  • _readable_bound covers files() / entries() / data_files() /
    all_files(), but manifests() decodes partition field summaries directly
    via from_bytes (inspect.py L420-435) and would fail the same way if the
    promoted column is an identity partition source.
  • An end-to-end test doing a real promotion then calling inspect.files() (the
    repro in the issue — SqlCatalog + temp dir, no network) would keep the call
    sites from drifting apart again.

Happy to test a revised branch against the S3 Tables table where I hit this.

@ArjunPakhan

Copy link
Copy Markdown
Author

Hi @tasty0tomato! 👋 Thanks for the thorough review!

I've updated the PR branch:

  1. Pushed pyiceberg/table/inspect.py with _readable_bound handling 4-byte bounds after int -> long and float -> double type promotions.
  2. Applied _readable_bound to partition summaries in manifests() to handle promoted partition source columns cleanly.
  3. Added an end-to-end SqlCatalog integration test in tests/table/test_inspect.py verifying inspect.files(), inspect.entries(), and inspect.manifests() after column promotion.

All 18 CI checks have passed! Ready for review when you have a moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Metadata inspection APIs fail with struct.error after int→long / float→double type promotion

2 participants