Skip to content

Commit 401c582

Browse files
Raise on view-table collisions in create/register/rename
- Add `_raise_if_view_exists` helper that maps `view_exists(id) == True` at the target identifier to `TableAlreadyExistsError`. Catalogs without view support (most non-REST impls) raise `NotImplementedError` from `view_exists`; treat that as "no view at this identifier". - Wire the check into `create_table`, `register_table`, and `rename_table` on SqlCatalog, HiveCatalog, GlueCatalog, DynamoDbCatalog, and BigQueryMetastoreCatalog (skipping methods that already raise `NotImplementedError`). Also call it from `MetastoreCatalog.create_table_transaction`. - For `rename_table`, the check is on the destination identifier. - RestCatalog intentionally unchanged: the server's 409 response is the authority and an extra client-side HEAD per call would be wasted. - Re-uses `TableAlreadyExistsError` so existing `create_table_if_not_exists` callers see the same exception type they already catch. - Tests: unit tests for the helper (returns True / False / raises `NotImplementedError`) in `tests/catalog/test_base.py`, plus parametrized behavior tests over InMemoryCatalog + SqlCatalog in `tests/catalog/test_catalog_behaviors.py` that monkeypatch `view_exists` to assert each call site invokes the helper.
1 parent 43d1f1f commit 401c582

8 files changed

Lines changed: 138 additions & 2 deletions

File tree

‎pyiceberg/catalog/__init__.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,20 @@ def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> No
323323
deleted_files[path] = True
324324

325325

326+
def _raise_if_view_exists(catalog: Catalog, identifier: str | Identifier) -> None:
327+
"""Raise `TableAlreadyExistsError` if a view exists at the given identifier.
328+
329+
Catalogs that don't support views raise `NotImplementedError` from `view_exists` —
330+
treat that as "no view at this identifier".
331+
"""
332+
try:
333+
view_collision = catalog.view_exists(identifier)
334+
except NotImplementedError:
335+
view_collision = False
336+
if view_collision:
337+
raise TableAlreadyExistsError(f"View with same name already exists: {identifier}")
338+
339+
326340
def _import_catalog(name: str, catalog_impl: str, properties: Properties) -> Catalog | None:
327341
try:
328342
path_parts = catalog_impl.split(".")
@@ -920,6 +934,7 @@ def create_table_transaction(
920934
sort_order: SortOrder = UNSORTED_SORT_ORDER,
921935
properties: Properties = EMPTY_DICT,
922936
) -> CreateTableTransaction:
937+
_raise_if_view_exists(self, identifier)
923938
return CreateTableTransaction(
924939
self._create_staged_table(identifier, schema, location, partition_spec, sort_order, properties)
925940
)

‎pyiceberg/catalog/bigquery_metastore.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
from google.oauth2 import service_account
2929
from typing_extensions import override
3030

31-
from pyiceberg.catalog import WAREHOUSE_LOCATION, MetastoreCatalog, PropertiesUpdateSummary
31+
from pyiceberg.catalog import WAREHOUSE_LOCATION, MetastoreCatalog, PropertiesUpdateSummary, _raise_if_view_exists
3232
from pyiceberg.exceptions import NamespaceAlreadyExistsError, NoSuchNamespaceError, NoSuchTableError, TableAlreadyExistsError
3333
from pyiceberg.io import load_file_io
3434
from pyiceberg.partitioning import UNPARTITIONED_PARTITION_SPEC, PartitionSpec
@@ -134,6 +134,7 @@ def create_table(
134134
schema: Schema = self._convert_schema_if_needed(schema) # type: ignore
135135

136136
dataset_name, table_name = self.identifier_to_database_and_table(identifier)
137+
_raise_if_view_exists(self, identifier)
137138

138139
location = self._resolve_table_location(location, dataset_name, table_name)
139140
provider = load_location_provider(table_location=location, table_properties=properties)
@@ -295,6 +296,7 @@ def register_table(self, identifier: str | Identifier, metadata_location: str, o
295296
if overwrite:
296297
raise NotImplementedError("`overwrite` isn't supported")
297298

299+
_raise_if_view_exists(self, identifier)
298300
dataset_name, table_name = self.identifier_to_database_and_table(identifier)
299301

300302
dataset_ref = DatasetReference(project=self.project_id, dataset_id=dataset_name)

‎pyiceberg/catalog/dynamodb.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
TABLE_TYPE,
3535
MetastoreCatalog,
3636
PropertiesUpdateSummary,
37+
_raise_if_view_exists,
3738
)
3839
from pyiceberg.exceptions import (
3940
ConditionalCheckFailedException,
@@ -187,6 +188,7 @@ def create_table(
187188
)
188189

189190
database_name, table_name = self.identifier_to_database_and_table(identifier)
191+
_raise_if_view_exists(self, identifier)
190192

191193
location = self._resolve_table_location(location, database_name, table_name)
192194
provider = load_location_provider(table_location=location, table_properties=properties)
@@ -313,6 +315,7 @@ def rename_table(self, from_identifier: str | Identifier, to_identifier: str | I
313315
"""
314316
from_database_name, from_table_name = self.identifier_to_database_and_table(from_identifier, NoSuchTableError)
315317
to_database_name, to_table_name = self.identifier_to_database_and_table(to_identifier)
318+
_raise_if_view_exists(self, to_identifier)
316319

317320
from_table_item = self._get_iceberg_table_item(database_name=from_database_name, table_name=from_table_name)
318321

‎pyiceberg/catalog/glue.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
TABLE_TYPE,
4040
MetastoreCatalog,
4141
PropertiesUpdateSummary,
42+
_raise_if_view_exists,
4243
)
4344
from pyiceberg.exceptions import (
4445
CommitFailedException,
@@ -571,6 +572,7 @@ def create_table(
571572
572573
"""
573574
database_name, table_name = self.identifier_to_database_and_table(identifier)
575+
_raise_if_view_exists(self, identifier)
574576

575577
if self._is_s3tables_database(database_name):
576578
return self._create_table_s3tables(
@@ -621,6 +623,7 @@ def register_table(self, identifier: str | Identifier, metadata_location: str, o
621623
if overwrite:
622624
raise NotImplementedError("`overwrite` isn't supported")
623625

626+
_raise_if_view_exists(self, identifier)
624627
database_name, table_name = self.identifier_to_database_and_table(identifier)
625628
properties = EMPTY_DICT
626629
io = self._load_file_io(location=metadata_location)
@@ -772,6 +775,7 @@ def rename_table(self, from_identifier: str | Identifier, to_identifier: str | I
772775
"""
773776
from_database_name, from_table_name = self.identifier_to_database_and_table(from_identifier, NoSuchTableError)
774777
to_database_name, to_table_name = self.identifier_to_database_and_table(to_identifier)
778+
_raise_if_view_exists(self, to_identifier)
775779
try:
776780
get_table_response = self.glue.get_table(DatabaseName=from_database_name, Name=from_table_name)
777781
except self.glue.exceptions.EntityNotFoundException as e:

‎pyiceberg/catalog/hive.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
URI,
6262
MetastoreCatalog,
6363
PropertiesUpdateSummary,
64+
_raise_if_view_exists,
6465
)
6566
from pyiceberg.exceptions import (
6667
CommitFailedException,
@@ -413,6 +414,7 @@ def create_table(
413414
ValueError: If the identifier is invalid.
414415
"""
415416
properties = {**DEFAULT_PROPERTIES, **properties}
417+
_raise_if_view_exists(self, identifier)
416418
staged_table = self._create_staged_table(
417419
identifier=identifier,
418420
schema=schema,
@@ -461,6 +463,7 @@ def register_table(self, identifier: str | Identifier, metadata_location: str, o
461463
if overwrite:
462464
raise NotImplementedError("`overwrite` isn't supported")
463465

466+
_raise_if_view_exists(self, identifier)
464467
database_name, table_name = self.identifier_to_database_and_table(identifier)
465468
io = self._load_file_io(location=metadata_location)
466469
metadata_file = io.new_input(metadata_location)
@@ -700,6 +703,7 @@ def rename_table(self, from_identifier: str | Identifier, to_identifier: str | I
700703

701704
if self.table_exists(to_identifier):
702705
raise TableAlreadyExistsError(f"Table already exists: {to_table_name}")
706+
_raise_if_view_exists(self, to_identifier)
703707

704708
try:
705709
with self._client as open_client:

‎pyiceberg/catalog/sql.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
Catalog,
4747
MetastoreCatalog,
4848
PropertiesUpdateSummary,
49+
_raise_if_view_exists,
4950
)
5051
from pyiceberg.exceptions import (
5152
CommitFailedException,
@@ -211,6 +212,7 @@ def create_table(
211212
table_name = Catalog.table_name_from(identifier)
212213
if not self.namespace_exists(namespace_identifier):
213214
raise NoSuchNamespaceError(f"Namespace does not exist: {namespace_identifier}")
215+
_raise_if_view_exists(self, identifier)
214216

215217
namespace = Catalog.namespace_to_string(namespace_identifier)
216218
location = self._resolve_table_location(location, namespace, table_name)
@@ -263,6 +265,7 @@ def register_table(self, identifier: str | Identifier, metadata_location: str, o
263265
table_name = Catalog.table_name_from(identifier)
264266
if not self.namespace_exists(namespace):
265267
raise NoSuchNamespaceError(f"Namespace does not exist: {namespace}")
268+
_raise_if_view_exists(self, identifier)
266269

267270
with Session(self.engine) as session:
268271
try:
@@ -376,6 +379,7 @@ def rename_table(self, from_identifier: str | Identifier, to_identifier: str | I
376379
to_table_name = Catalog.table_name_from(to_identifier)
377380
if not self.namespace_exists(to_namespace):
378381
raise NoSuchNamespaceError(f"Namespace does not exist: {to_namespace}")
382+
_raise_if_view_exists(self, to_identifier)
379383
with Session(self.engine) as session:
380384
try:
381385
if self.engine.dialect.supports_sane_rowcount:

‎tests/catalog/test_base.py‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222

2323
import pytest
2424

25-
from pyiceberg.catalog import Catalog, load_catalog
25+
from pyiceberg.catalog import Catalog, _raise_if_view_exists, load_catalog
2626
from pyiceberg.catalog.memory import InMemoryCatalog
27+
from pyiceberg.exceptions import TableAlreadyExistsError
2728
from pyiceberg.io import WAREHOUSE
2829
from pyiceberg.schema import Schema
2930
from pyiceberg.types import NestedField, StringType
@@ -69,6 +70,38 @@ def test_catalog_repr(catalog: InMemoryCatalog) -> None:
6970
assert s == "test.in_memory.catalog (<class 'pyiceberg.catalog.memory.InMemoryCatalog'>)"
7071

7172

73+
class _StubCatalog:
74+
def __init__(self, *, returns: bool | None = None, raises: type[Exception] | None = None) -> None:
75+
self._returns = returns
76+
self._raises = raises
77+
self.calls: list[object] = []
78+
79+
def view_exists(self, identifier: object) -> bool:
80+
self.calls.append(identifier)
81+
if self._raises is not None:
82+
raise self._raises
83+
assert self._returns is not None
84+
return self._returns
85+
86+
87+
class TestRaiseIfViewExists:
88+
def test_raises_when_view_exists(self) -> None:
89+
stub = _StubCatalog(returns=True)
90+
with pytest.raises(TableAlreadyExistsError, match="View with same name already exists: ns.t"):
91+
_raise_if_view_exists(stub, "ns.t") # type: ignore[arg-type]
92+
assert stub.calls == ["ns.t"]
93+
94+
def test_no_raise_when_view_absent(self) -> None:
95+
stub = _StubCatalog(returns=False)
96+
_raise_if_view_exists(stub, ("ns", "t")) # type: ignore[arg-type]
97+
assert stub.calls == [("ns", "t")]
98+
99+
def test_not_implemented_treated_as_no_view(self) -> None:
100+
stub = _StubCatalog(raises=NotImplementedError)
101+
_raise_if_view_exists(stub, "ns.t") # type: ignore[arg-type]
102+
assert stub.calls == ["ns.t"]
103+
104+
72105
class TestCatalogClose:
73106
"""Test catalog close functionality."""
74107

‎tests/catalog/test_catalog_behaviors.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,77 @@ def test_rename_table_to_missing_namespace(
461461
catalog.rename_table(test_table_identifier, another_table_identifier)
462462

463463

464+
# View-collision tests
465+
#
466+
# InMemoryCatalog and SqlCatalog don't support views (`view_exists` raises
467+
# `NotImplementedError`), so we monkeypatch it to assert the call sites invoke
468+
# the helper that maps a view at the target identifier to `TableAlreadyExistsError`.
469+
470+
471+
def test_create_table_raises_when_view_exists_at_identifier(
472+
catalog: Catalog,
473+
test_table_identifier: Identifier,
474+
table_schema_simple: Schema,
475+
monkeypatch: pytest.MonkeyPatch,
476+
) -> None:
477+
namespace = Catalog.namespace_from(test_table_identifier)
478+
catalog.create_namespace(namespace)
479+
monkeypatch.setattr(
480+
catalog, "view_exists", lambda identifier: Catalog.identifier_to_tuple(identifier) == test_table_identifier
481+
)
482+
with pytest.raises(TableAlreadyExistsError, match="View with same name already exists"):
483+
catalog.create_table(test_table_identifier, table_schema_simple)
484+
485+
486+
def test_create_table_transaction_raises_when_view_exists_at_identifier(
487+
catalog: Catalog,
488+
test_table_identifier: Identifier,
489+
table_schema_simple: Schema,
490+
monkeypatch: pytest.MonkeyPatch,
491+
) -> None:
492+
namespace = Catalog.namespace_from(test_table_identifier)
493+
catalog.create_namespace(namespace)
494+
monkeypatch.setattr(
495+
catalog, "view_exists", lambda identifier: Catalog.identifier_to_tuple(identifier) == test_table_identifier
496+
)
497+
with pytest.raises(TableAlreadyExistsError, match="View with same name already exists"):
498+
catalog.create_table_transaction(test_table_identifier, table_schema_simple)
499+
500+
501+
def test_register_table_raises_when_view_exists_at_identifier(
502+
catalog: Catalog,
503+
test_table_identifier: Identifier,
504+
metadata_location: str,
505+
monkeypatch: pytest.MonkeyPatch,
506+
) -> None:
507+
namespace = Catalog.namespace_from(test_table_identifier)
508+
catalog.create_namespace(namespace)
509+
monkeypatch.setattr(
510+
catalog, "view_exists", lambda identifier: Catalog.identifier_to_tuple(identifier) == test_table_identifier
511+
)
512+
with pytest.raises(TableAlreadyExistsError, match="View with same name already exists"):
513+
catalog.register_table(test_table_identifier, metadata_location)
514+
515+
516+
def test_rename_table_raises_when_view_exists_at_destination(
517+
catalog: Catalog,
518+
table_schema_simple: Schema,
519+
test_table_identifier: Identifier,
520+
another_table_identifier: Identifier,
521+
monkeypatch: pytest.MonkeyPatch,
522+
) -> None:
523+
from_namespace = Catalog.namespace_from(test_table_identifier)
524+
to_namespace = Catalog.namespace_from(another_table_identifier)
525+
catalog.create_namespace(from_namespace)
526+
catalog.create_namespace(to_namespace)
527+
catalog.create_table(test_table_identifier, table_schema_simple)
528+
monkeypatch.setattr(
529+
catalog, "view_exists", lambda identifier: Catalog.identifier_to_tuple(identifier) == another_table_identifier
530+
)
531+
with pytest.raises(TableAlreadyExistsError, match="View with same name already exists"):
532+
catalog.rename_table(test_table_identifier, another_table_identifier)
533+
534+
464535
# Drop table tests
465536

466537

0 commit comments

Comments
 (0)