This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
The following commit(s) were added to refs/heads/main by this push:
new 2ecd87b22 Glue: Prune removed table properties from Glue Parameters
(#4133)
2ecd87b22 is described below
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
