Skip to content

Fix/sqlcatalog iceberg type filter#3709

Open
avison9 wants to merge 4 commits into
apache:mainfrom
avison9:fix/sqlcatalog-iceberg-type-filter
Open

Fix/sqlcatalog iceberg type filter#3709
avison9 wants to merge 4 commits into
apache:mainfrom
avison9:fix/sqlcatalog-iceberg-type-filter

Conversation

@avison9

@avison9 avison9 commented Jul 25, 2026

Copy link
Copy Markdown

Rationale for this change

SqlCatalog's iceberg_tables table can be shared with iceberg-java/iceberg-rust clients, which
store both tables and views there using an iceberg_type column (TABLE/VIEW), filtering every
lookup with WHERE (iceberg_type = 'TABLE' OR iceberg_type IS NULL) (see
JdbcUtil.java#L168).
PyIceberg had neither the column nor the filter, so a view row written by another implementation
would be silently treated as a table by load_table, drop_table, rename_table, and commit_table.

Before:

catalog.drop_table(("ns", "sales_summary"))  # 'sales_summary' is actually a VIEW row
# No error — the view's row is silently deleted.

After:

catalog.drop_table(("ns", "sales_summary"))
# raises NoSuchTableError: Table does not exist: ns.sales_summary

This PR builds on #3263 (cherry-picked here, unmerged, authored by @rchowell), which adds the
iceberg_type column, an opt-in idempotent v0→v1 migration, and filters list_tables. On top of
that, it extracts a small _iceberg_type_filter() helper (as suggested in #3337) and applies it to
load_table, drop_table, and rename_table. commit_table needed no direct change since it
already delegates its existence check to load_table.

Note on scope: a prior attempt at #3337 (#3525) tried to add the column and the filtering in one
PR, but was closed after review feedback preferred keeping #3263's column/migration approach
separate from the filtering logic. This PR keeps that separation by adopting #3263's commits as-is
and adding only the filtering on top.

Are these changes tested?

Yes, i added test_load_table_ignores_view_rows, test_drop_table_ignores_view_rows, and
test_rename_table_ignores_view_rows in tests/catalog/test_sql.py, each inserting a raw VIEW
row and confirming the affected operation now raises NoSuchTableError instead of touching it.
Verified all three fail without the fix. Full suite: 3796 passed, 0 failed.

Are there any user-facing changes?

Yes! SqlCatalog.load_table, drop_table, and rename_table now correctly ignore VIEW rows
written by other Iceberg implementations sharing the same catalog table, instead of silently
reading, deleting, or renaming them. New optional schema_version catalog property (from #3263)
controls opt-in migration of existing v0 catalogs. Please apply the changelog label.

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