anxkhn opened a new pull request, #3602:
URL: https://github.com/apache/iceberg-python/pull/3602
# Rationale for this change
`DynamoDbCatalog` stores each namespace property under a key prefixed with
`PROPERTY_KEY_PREFIX` (`"p."`) via `_add_property_prefix`, and decodes it on
read
with `_remove_property_prefix`:
```python
def _remove_property_prefix(prop: str) -> str:
return prop.lstrip(PROPERTY_KEY_PREFIX)
```
`str.lstrip` treats its argument as a *set of characters* to strip, not a
literal
prefix. With `PROPERTY_KEY_PREFIX == "p."` it removes every leading `p` and
`.`
from the decoded key, so any namespace property whose name starts with one of
those characters is corrupted when read back through
`load_namespace_properties`:
| stored property | returned key (before) |
| --- | --- |
| `provider` | `rovider` |
| `path` | `ath` |
| `partition` | `artition` |
| `ppp` | `` (empty) |
| `.hidden` | `hidden` |
`update_namespace_properties` reads the same decoded properties, so it
operates on
the corrupted keys too. Property names that do not start with `p` or `.`
(such as
`comment`, `owner`, `location`) round-trip correctly, which is why the
existing
tests, whose fixtures avoid those leading characters, never surfaced the bug.
The fix uses `str.removeprefix`, which strips only the literal `"p."`
prefix. The
single caller (`_get_namespace_properties`) already guards each key with
`startswith(PROPERTY_KEY_PREFIX)`, so behavior for every valid key is
unchanged
and only the previously corrupted keys are affected. `str.removeprefix` is
available in Python 3.9+ and this project requires `>=3.10`.
## Are these changes tested?
Yes. Added `test_load_namespace_properties_with_prefix_char_keys` in
`tests/catalog/test_dynamodb.py`, a `create_namespace` /
`load_namespace_properties`
round trip (backed by `moto`'s `mock_aws`) over keys `provider`, `path`,
`ppp`,
`.hidden`, and `comment`. The test fails on the previous `lstrip`
implementation
(keys come back corrupted) and passes with `removeprefix`.
`tests/catalog/test_dynamodb.py` passes (40 tests), and `make lint` (ruff,
ruff-format, mypy, and the other pre-commit hooks) is clean. The DynamoDB
catalog
has no separate integration test path; behavior is covered by the mocked
unit test.
## Are there any user-facing changes?
Yes: a bug fix. Reading namespace properties from a DynamoDB-backed catalog
now
returns property keys that begin with `p` or `.` correctly instead of
truncating
their leading characters. No public API changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
