From e2951d8e1fcd4165027c57900b4223ad3f407a47 Mon Sep 17 00:00:00 2001 From: Ankit Raj Date: Wed, 21 Oct 2026 17:32:48 +0000 Subject: [PATCH 1/3] Glue: Prune removed table properties from Glue Parameters Closes #3977 --- pyiceberg/catalog/glue.py | 11 ++++++++++- tests/catalog/test_glue.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/pyiceberg/catalog/glue.py b/pyiceberg/catalog/glue.py index 977876918c..6808cd91ba 100644 --- a/pyiceberg/catalog/glue.py +++ b/pyiceberg/catalog/glue.py @@ -147,8 +147,13 @@ 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 @@ -243,11 +248,14 @@ 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, glue_table, prev_metadata_location, properties, prev_metadata_properties + ), "StorageDescriptor": { "Columns": _to_columns(metadata), "Location": metadata.location, @@ -693,6 +701,7 @@ def commit_table( 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 fd476c978a..5b7beb7bd6 100644 --- a/tests/catalog/test_glue.py +++ b/tests/catalog/test_glue.py @@ -859,6 +859,36 @@ 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_append_table_snapshot_properties( _bucket_initialize: None, moto_endpoint_url: str, table_schema_simple: Schema, database_name: str, table_name: str From c3a49346bb5fbc81a42caefa20a266a45544bb83 Mon Sep 17 00:00:00 2001 From: Ankit Raj Date: Thu, 22 Oct 2026 20:57:31 +0000 Subject: [PATCH 2/3] Use keyword arguments for _construct_parameters --- pyiceberg/catalog/glue.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pyiceberg/catalog/glue.py b/pyiceberg/catalog/glue.py index 6808cd91ba..a2cc6821af 100644 --- a/pyiceberg/catalog/glue.py +++ b/pyiceberg/catalog/glue.py @@ -254,7 +254,11 @@ def _construct_table_input( "Name": table_name, "TableType": EXTERNAL_TABLE, "Parameters": _construct_parameters( - metadata_location, glue_table, prev_metadata_location, properties, prev_metadata_properties + 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), From 3194838e249ad31d195c89bb11e06b0405d182a3 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 00:08:25 -0700 Subject: [PATCH 3/3] 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 <223556219+Copilot@users.noreply.github.com> --- pyiceberg/catalog/glue.py | 4 +++- tests/catalog/test_glue.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pyiceberg/catalog/glue.py b/pyiceberg/catalog/glue.py index a2cc6821af..a7d0683a07 100644 --- a/pyiceberg/catalog/glue.py +++ b/pyiceberg/catalog/glue.py @@ -160,7 +160,9 @@ def _construct_parameters( 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 diff --git a/tests/catalog/test_glue.py b/tests/catalog/test_glue.py index 5b7beb7bd6..8943858b88 100644 --- a/tests/catalog/test_glue.py +++ b/tests/catalog/test_glue.py @@ -889,6 +889,38 @@ def test_commit_table_prunes_removed_properties( 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