SQL Catalog: Filter on iceberg_type in commit_table (#3337)

[hedger9487]

Description

Fixes #3337.

In SqlCatalog, _iceberg_type_filter() was added in #3263 to filter on iceberg_type (matching TABLE or NULL) and avoid operating on VIEW rows written by other Iceberg implementations (e.g., Java or Rust). While load_table, drop_table, rename_table, and list_tables incorporate this filter, commit_table omitted type_filter in its SQL update statement.

This PR applies type_filter = self._iceberg_type_filter() to the commit_table update query in pyiceberg/catalog/sql.py.

Testing

• Added unit test test_commit_table_ignores_view_rows in tests/catalog/test_sql.py.
• All 31 catalog unit tests pass locally.

[Copilot]

Pull request overview

This PR fixes SqlCatalog.commit_table so it consistently filters SQL catalog rows by iceberg_type (matching TABLE or NULL), preventing accidental operations on VIEW rows created by other Iceberg implementations.

Changes:

• Apply self._iceberg_type_filter() to the commit_table UPDATE statement (rowcount-based path) and the SELECT ... FOR UPDATE query (fallback path) in pyiceberg/catalog/sql.py.
• Add a regression unit test ensuring commits fail (and do not modify the row) when the underlying row has iceberg_type='VIEW'.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pyiceberg/catalog/sql.py	Adds the iceberg_type predicate to commit_table’s update/locking queries to avoid touching VIEW rows.
tests/catalog/test_sql.py	Adds a unit test covering the commit_table behavior when the backing row is tampered into a VIEW.

Suppressed comments (1)

tests/catalog/test_sql.py:413

• The assertion query filters only on table_name, which can become ambiguous if additional rows with the same name exist (e.g., different namespaces). Include catalog_name and table_namespace in the WHERE clause to ensure the test checks the intended row.

    with catalog.engine.connect() as conn:
        row = conn.execute(text("SELECT iceberg_type FROM iceberg_tables WHERE table_name = 'a_view'")).fetchone()
    assert row is not None

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ebyhr]

I don't think this is a valid regression test. It passes even if I revert sql.py‎'s change.

[hedger9487]

Good catch! @ebyhr

The previous test was failing early in load_table (which already had the type filter applied during requirement checking), before ever reaching the SQL UPDATE statement.

I have updated test_commit_table_ignores_view_rows to simulate the concurrent modification race condition (where load_table succeeded prior to the row becoming a VIEW). Now, if the type_filter in commit_table's SQL UPDATE is reverted, the test fails with DID NOT RAISE CommitFailedException as expected. Thanks for pointing this out!