Skip to content

Core: Preserve null operations in inspect.snapshots - #3707

Open
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:fix/inspect-snapshots-null-operation
Open

Core: Preserve null operations in inspect.snapshots#3707
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:fix/inspect-snapshots-null-operation

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

The snapshots metadata table declares operation as nullable because snapshots without a summary do not have an operation. However, InspectTable.snapshots() converted the missing value with str(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 lint

Are there any user-facing changes?

Yes. For snapshots without a summary, table.inspect.snapshots() now returns null in the operation column instead of the string "None". There are no API changes.

Comment thread tests/table/test_inspect.py Outdated


def test_inspect_snapshots_preserves_null_operation(catalog: InMemoryCatalog) -> None:
schema = Schema(NestedField(1, "s", StringType(), required=False))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required=False looks redundant.

Comment thread tests/table/test_inspect.py Outdated
def test_inspect_snapshots_preserves_null_operation(catalog: InMemoryCatalog) -> None:
schema = Schema(NestedField(1, "s", StringType(), required=False))
tbl = catalog.create_table("default.snapshot_without_summary", schema)
tbl.append(pa.table({"s": ["value"]}, schema=pa.schema([pa.field("s", pa.large_string(), nullable=True)])))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nullable=True looks redundant.

@fallintoplace
fallintoplace force-pushed the fix/inspect-snapshots-null-operation branch from 6ffcda5 to 786fd6f Compare July 31, 2026 10:32
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.

2 participants