Skip to content

Commit d793777

Browse files
Tighten replace_table_transaction input validation
- Wrap int() parse of the format-version property in try/except so a non-numeric value yields a domain-meaningful ValueError instead of Python's opaque "invalid literal for int()". - Reject a resolved location of empty string. Previously a caller passing location="/" silently produced location="" after rstrip, which would later land as SetLocationUpdate(location="").
1 parent a810a15 commit d793777

2 files changed

Lines changed: 27 additions & 8 deletions

File tree

pyiceberg/catalog/__init__.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -505,14 +505,19 @@ def _replace_staged_table(
505505
existing_table = self.load_table(identifier)
506506
existing_metadata = existing_table.metadata
507507

508-
requested_format_version = properties.get(TableProperties.FORMAT_VERSION)
509-
if requested_format_version is not None and int(requested_format_version) < existing_metadata.format_version:
510-
raise ValueError(
511-
f"Cannot downgrade format-version from {existing_metadata.format_version} to {requested_format_version}"
512-
)
513-
resolved_format_version = (
514-
int(requested_format_version) if requested_format_version is not None else existing_metadata.format_version
515-
)
508+
raw_format_version = properties.get(TableProperties.FORMAT_VERSION)
509+
if raw_format_version is not None:
510+
try:
511+
requested_format_version = int(raw_format_version)
512+
except (TypeError, ValueError) as exc:
513+
raise ValueError(f"Invalid format-version property: {raw_format_version!r}") from exc
514+
if requested_format_version < existing_metadata.format_version:
515+
raise ValueError(
516+
f"Cannot downgrade format-version from {existing_metadata.format_version} to {requested_format_version}"
517+
)
518+
resolved_format_version = requested_format_version
519+
else:
520+
resolved_format_version = existing_metadata.format_version
516521
iceberg_schema = self._convert_schema_if_needed(schema, cast(TableVersion, resolved_format_version))
517522
iceberg_schema.check_format_version_compatibility(cast(TableVersion, resolved_format_version))
518523

@@ -533,6 +538,8 @@ def _replace_staged_table(
533538
fresh_sort_order = assign_fresh_sort_order_ids(sort_order, iceberg_schema, fresh_schema)
534539

535540
resolved_location = location.rstrip("/") if location else existing_metadata.location
541+
if not resolved_location:
542+
raise ValueError("Resolved table location must not be empty")
536543

537544
staged_table = StagedTable(
538545
identifier=existing_table.name(),

tests/catalog/test_catalog_behaviors.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,18 @@ def test_replace_table_rejects_format_version_downgrade(catalog: Catalog, test_t
638638
catalog.replace_table_transaction(test_table_identifier, schema=schema, properties={"format-version": "1"})
639639

640640

641+
def test_replace_table_rejects_non_numeric_format_version(catalog: Catalog, test_table_identifier: Identifier) -> None:
642+
_, schema = _create_simple_table(catalog, test_table_identifier)
643+
with pytest.raises(ValueError, match="Invalid format-version property"):
644+
catalog.replace_table_transaction(test_table_identifier, schema=schema, properties={"format-version": "two"})
645+
646+
647+
def test_replace_table_rejects_empty_location(catalog: Catalog, test_table_identifier: Identifier) -> None:
648+
_, schema = _create_simple_table(catalog, test_table_identifier)
649+
with pytest.raises(ValueError, match="location must not be empty"):
650+
catalog.replace_table_transaction(test_table_identifier, schema=schema, location="/")
651+
652+
641653
@pytest.mark.parametrize("location_kind", ["inherit", "explicit", "trailing-slash"])
642654
def test_replace_table_location(catalog: Catalog, test_table_identifier: Identifier, tmp_path: Path, location_kind: str) -> None:
643655
_, schema = _create_simple_table(catalog, test_table_identifier)

0 commit comments

Comments
 (0)