This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-4133-58df2cca5c82c0e6cdb2668da7f5834b88d3bee9 in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
commit 2ecd87b222f5c7fe020c93816f8c3830412b7034 Author: Ankit Raj <[email protected]> AuthorDate: Sat Oct 24 07:21:29 2026 +0000 Glue: Prune removed table properties from Glue Parameters (#4133) * Glue: Prune removed table properties from Glue Parameters Closes #3977 * Use keyword arguments for _construct_parameters * Prevent table properties from overwriting reserved Glue parameters A table property named table_type, metadata_location, or previous_metadata_location overwrote the catalog-managed Glue parameter of the same name, which could point Glue at a bogus metadata file and break load_table. Skip reserved keys when copying table properties, matching the Hive catalog. Co-authored-by: Copilot App <[email protected]> --------- Co-authored-by: Kevin Liu <[email protected]> Co-authored-by: Copilot App <[email protected]> --- pyiceberg/catalog/glue.py | 19 ++++++++++++-- tests/catalog/test_glue.py | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pyiceberg/catalog/glue.py b/pyiceberg/catalog/glue.py index 977876918..a7d0683a0 100644 --- a/pyiceberg/catalog/glue.py +++ b/pyiceberg/catalog/glue.py @@ -147,15 +147,22 @@ def _construct_parameters( glue_table: Optional["TableTypeDef"] = None, prev_metadata_location: str | None = None, metadata_properties: Properties | None = None, + prev_metadata_properties: Properties | None = None, ) -> Properties: new_parameters = glue_table.get("Parameters", {}) if glue_table else {} + if prev_metadata_properties: + for key in prev_metadata_properties: + if not metadata_properties or key not in metadata_properties: + new_parameters.pop(key, None) new_parameters.update({TABLE_TYPE: ICEBERG.upper(), METADATA_LOCATION: metadata_location}) if prev_metadata_location: new_parameters[PREVIOUS_METADATA_LOCATION] = prev_metadata_location if metadata_properties: for key, value in metadata_properties.items(): - new_parameters[key] = str(value) + # Reserved parameters are managed by the catalog and must not be overwritten by table properties + if key not in (TABLE_TYPE, METADATA_LOCATION, PREVIOUS_METADATA_LOCATION): + new_parameters[key] = str(value) return new_parameters @@ -243,11 +250,18 @@ def _construct_table_input( metadata: TableMetadata, glue_table: Optional["TableTypeDef"] = None, prev_metadata_location: str | None = None, + prev_metadata_properties: Properties | None = None, ) -> "TableInputTypeDef": table_input: TableInputTypeDef = { "Name": table_name, "TableType": EXTERNAL_TABLE, - "Parameters": _construct_parameters(metadata_location, glue_table, prev_metadata_location, properties), + "Parameters": _construct_parameters( + metadata_location=metadata_location, + glue_table=glue_table, + prev_metadata_location=prev_metadata_location, + metadata_properties=properties, + prev_metadata_properties=prev_metadata_properties, + ), "StorageDescriptor": { "Columns": _to_columns(metadata), "Location": metadata.location, @@ -693,6 +707,7 @@ class GlueCatalog(MetastoreCatalog): metadata=updated_staged_table.metadata, glue_table=current_glue_table, prev_metadata_location=current_table.metadata_location, + prev_metadata_properties=current_table.properties, ) self._update_glue_table( database_name=database_name, diff --git a/tests/catalog/test_glue.py b/tests/catalog/test_glue.py index fd476c978..8943858b8 100644 --- a/tests/catalog/test_glue.py +++ b/tests/catalog/test_glue.py @@ -859,6 +859,68 @@ def test_commit_table_properties( assert table_info["Table"]["Parameters"]["test_c"] == "test_c" +@mock_aws +def test_commit_table_prunes_removed_properties( + _glue: boto3.client, + _bucket_initialize: None, + moto_endpoint_url: str, + table_schema_nested: Schema, + database_name: str, + table_name: str, +) -> None: + catalog_name = "glue" + identifier = (database_name, table_name) + test_catalog = GlueCatalog( + catalog_name, client=None, **{"s3.endpoint": moto_endpoint_url, "warehouse": f"s3://{BUCKET_NAME}"} + ) + test_catalog.create_namespace(namespace=database_name) + table = test_catalog.create_table( + identifier=identifier, schema=table_schema_nested, properties={"test_a": "test_a", "test_b": "test_b"} + ) + + transaction = table.transaction() + transaction.remove_properties("test_b") + transaction.commit_transaction() + + assert table.metadata.properties == {"test_a": "test_a"} + + table_info = _glue.get_table(DatabaseName=database_name, Name=table_name) + assert table_info["Table"]["Parameters"]["test_a"] == "test_a" + assert "test_b" not in table_info["Table"]["Parameters"] + + +@mock_aws +def test_commit_table_properties_do_not_overwrite_reserved_parameters( + _glue: boto3.client, + _bucket_initialize: None, + moto_endpoint_url: str, + table_schema_nested: Schema, + database_name: str, + table_name: str, +) -> None: + identifier = (database_name, table_name) + test_catalog = GlueCatalog("glue", client=None, **{"s3.endpoint": moto_endpoint_url, "warehouse": f"s3://{BUCKET_NAME}"}) + test_catalog.create_namespace(namespace=database_name) + table = test_catalog.create_table(identifier=identifier, schema=table_schema_nested) + previous_metadata_location = table.metadata_location + + table.transaction().set_properties( + table_type="bogus", + metadata_location="s3://bogus/metadata.json", + previous_metadata_location="s3://bogus/previous.metadata.json", + ).commit_transaction() + + parameters = _glue.get_table(DatabaseName=database_name, Name=table_name)["Table"]["Parameters"] + assert parameters["table_type"] == "ICEBERG" + assert parameters["metadata_location"] == table.metadata_location + assert parameters["previous_metadata_location"] == previous_metadata_location + assert test_catalog.load_table(identifier).metadata_location == table.metadata_location + + # Removing the properties must not prune the reserved parameters either + table.transaction().remove_properties("table_type", "metadata_location", "previous_metadata_location").commit_transaction() + assert test_catalog.load_table(identifier).metadata_location == table.metadata_location + + @mock_aws def test_commit_append_table_snapshot_properties( _bucket_initialize: None, moto_endpoint_url: str, table_schema_simple: Schema, database_name: str, table_name: str
