Repository navigation
Core: Preserve null operations in inspect.snapshots - #3707
fallintoplace wants to merge 3 commits into
Conversation
6ffcda5 to
786fd6f
Compare
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
Fokko
left a comment
There was a problem hiding this comment.
Reasonable change, thanks @fallintoplace
|
@fallintoplace it looks like this is ready to merge. Can you fix the linters so we can get a successful CI run? |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
Rationale for this change
The
snapshotsmetadata table declaresoperationas nullable because snapshots without a summary do not have an operation. However,InspectTable.snapshots()converted the missing value withstr(None), returning the literal string"None"instead of null.This change passes the operation directly to PyArrow, preserving null semantics and matching Apache Iceberg Java's snapshots metadata table.
Are these changes tested?
Yes. A regression test creates a snapshot without a summary and verifies that
table.inspect.snapshots()returns a null operation.The following checks pass locally:
PYTHONPATH=. uv run pytest tests/table/test_inspect.py -q(5 passed)PYTHONPATH=. uv run --extra datafusion --extra pyiceberg-core pytest tests/table -q(328 passed)make lintAre there any user-facing changes?
Yes. For snapshots without a summary,
table.inspect.snapshots()now returns null in theoperationcolumn instead of the string"None". There are no API changes.