avison9 opened a new pull request, #3709: URL: https://github.com/apache/iceberg-python/pull/3709
<!-- Closes #3337 #3263 --> # 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`](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/jdbc/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:** ```python catalog.drop_table(("ns", "sales_summary")) # 'sales_summary' is actually a VIEW row # No error — the view's row is silently deleted. ``` **After:** ```python 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. <!-- In the case of user-facing changes, please add the changelog label. --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
