From 743e5faac1880c03c8562d59a829e20ebee53f6c Mon Sep 17 00:00:00 2001 From: Anas Khan <83116240+anxkhn@users.noreply.github.com> Date: Sat, 4 Jul 2026 16:43:08 +0530 Subject: [PATCH 1/2] fix: use removeprefix to decode DynamoDB namespace property keys _remove_property_prefix used str.lstrip(PROPERTY_KEY_PREFIX), which treats "p." as a set of characters and strips every leading "p"/"." from the key instead of removing the literal "p." prefix. Namespace property keys that begin with those characters were therefore corrupted on read: "provider" became "rovider", "path" became "ath", and "ppp" became "". Existing tests did not catch this because their fixture keys avoid a leading "p" or ".". Switch to str.removeprefix, which strips only the literal prefix. The single caller already guards with startswith(PROPERTY_KEY_PREFIX), so the behavior for all valid keys is unchanged. Add a round-trip regression test covering keys that start with "p" and ".". --- pyiceberg/catalog/dynamodb.py | 2 +- tests/catalog/test_dynamodb.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pyiceberg/catalog/dynamodb.py b/pyiceberg/catalog/dynamodb.py index 74c0be6c9a..ee0e5e23de 100644 --- a/pyiceberg/catalog/dynamodb.py +++ b/pyiceberg/catalog/dynamodb.py @@ -891,4 +891,4 @@ def _add_property_prefix(prop: str) -> str: def _remove_property_prefix(prop: str) -> str: - return prop.lstrip(PROPERTY_KEY_PREFIX) + return prop.removeprefix(PROPERTY_KEY_PREFIX) diff --git a/tests/catalog/test_dynamodb.py b/tests/catalog/test_dynamodb.py index 5933e7d472..97ee824812 100644 --- a/tests/catalog/test_dynamodb.py +++ b/tests/catalog/test_dynamodb.py @@ -502,6 +502,26 @@ def test_load_namespace_properties(_bucket_initialize: None, database_name: str) assert v == test_properties[k] +@mock_aws +def test_load_namespace_properties_with_prefix_char_keys(_bucket_initialize: None, database_name: str) -> None: + # Property keys starting with a character in the "p." prefix (e.g. "p" or ".") must + # survive a round trip. The stored keys are prefixed with PROPERTY_KEY_PREFIX ("p."), + # so decoding must strip only that literal prefix, not any leading "p"/"." characters. + test_properties = { + "provider": "s3", + "path": f"s3://{BUCKET_NAME}/{database_name}.db", + "ppp": "3", + ".hidden": "yes", + "comment": "not affected", + } + test_catalog = DynamoDbCatalog("test_ddb_catalog") + test_catalog.create_namespace(database_name, test_properties) + listed_properties = test_catalog.load_namespace_properties(database_name) + for k, v in test_properties.items(): + assert k in listed_properties + assert listed_properties[k] == v + + @mock_aws def test_load_non_exist_namespace_properties(_bucket_initialize: None, database_name: str) -> None: test_catalog = DynamoDbCatalog("test_ddb_catalog") From f3e94aa62065e6e78c8fdd623991dd676322e328 Mon Sep 17 00:00:00 2001 From: Anas Khan <83116240+anxkhn@users.noreply.github.com> Date: Tue, 14 Jul 2026 17:21:45 +0530 Subject: [PATCH 2/2] fix: inline DynamoDB property prefix removal Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com> --- pyiceberg/catalog/dynamodb.py | 9 ++++----- tests/catalog/test_dynamodb.py | 20 -------------------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/pyiceberg/catalog/dynamodb.py b/pyiceberg/catalog/dynamodb.py index ee0e5e23de..10f74fb9a0 100644 --- a/pyiceberg/catalog/dynamodb.py +++ b/pyiceberg/catalog/dynamodb.py @@ -837,7 +837,10 @@ def _get_update_database_item(namespace_item: dict[str, Any], updated_properties def _get_namespace_properties(namespace_dict: dict[str, str]) -> Properties: - return {_remove_property_prefix(key): val for key, val in namespace_dict.items() if key.startswith(PROPERTY_KEY_PREFIX)} + # removeprefix removes the literal prefix, unlike lstrip which removes any leading prefix characters + return { + key.removeprefix(PROPERTY_KEY_PREFIX): val for key, val in namespace_dict.items() if key.startswith(PROPERTY_KEY_PREFIX) + } def _convert_dynamo_item_to_regular_dict(dynamo_json: dict[str, Any]) -> dict[str, str]: @@ -888,7 +891,3 @@ def _convert_dynamo_item_to_regular_dict(dynamo_json: dict[str, Any]) -> dict[st def _add_property_prefix(prop: str) -> str: return PROPERTY_KEY_PREFIX + prop - - -def _remove_property_prefix(prop: str) -> str: - return prop.removeprefix(PROPERTY_KEY_PREFIX) diff --git a/tests/catalog/test_dynamodb.py b/tests/catalog/test_dynamodb.py index 97ee824812..5933e7d472 100644 --- a/tests/catalog/test_dynamodb.py +++ b/tests/catalog/test_dynamodb.py @@ -502,26 +502,6 @@ def test_load_namespace_properties(_bucket_initialize: None, database_name: str) assert v == test_properties[k] -@mock_aws -def test_load_namespace_properties_with_prefix_char_keys(_bucket_initialize: None, database_name: str) -> None: - # Property keys starting with a character in the "p." prefix (e.g. "p" or ".") must - # survive a round trip. The stored keys are prefixed with PROPERTY_KEY_PREFIX ("p."), - # so decoding must strip only that literal prefix, not any leading "p"/"." characters. - test_properties = { - "provider": "s3", - "path": f"s3://{BUCKET_NAME}/{database_name}.db", - "ppp": "3", - ".hidden": "yes", - "comment": "not affected", - } - test_catalog = DynamoDbCatalog("test_ddb_catalog") - test_catalog.create_namespace(database_name, test_properties) - listed_properties = test_catalog.load_namespace_properties(database_name) - for k, v in test_properties.items(): - assert k in listed_properties - assert listed_properties[k] == v - - @mock_aws def test_load_non_exist_namespace_properties(_bucket_initialize: None, database_name: str) -> None: test_catalog = DynamoDbCatalog("test_ddb_catalog")