anxkhn opened a new pull request, #3617:
URL: https://github.com/apache/iceberg-python/pull/3617
# Rationale for this change
The `@singledispatch` base of `_to_partition_representation` in
`pyiceberg/partitioning.py` returned its error instead of raising it:
```python
return TypeError(f"Unsupported partition field type: {type}")
```
For an unsupported (non-primitive) partition source type, the base handler
is the
one that runs, and it handed the `TypeError` instance back as the partition
value
rather than raising it. The exception object would then flow downstream as
the
"iceberg-typed" partition value (through `partition_record_value` ->
`PartitionKey.partition` -> `Record`) and only blow up later during
serialization,
far from the real cause, or write a garbage partition record.
This is the only `return <Exception>(...)` in the `pyiceberg/` tree. Every
sibling
registered handler for this function raises (the `TimestampType` /
`DateType` /
`TimeType` / `UUIDType` / `PrimitiveType` registrations all `raise
ValueError`), as
does every other `singledispatch` base handler in the codebase (for example
the
`to_bytes` / `from_bytes` / `to_json` / `from_json` bases in
`pyiceberg/conversions.py`). So `raise` is the correct, convention-matching
behavior; the fix is a one-word correction (`return` -> `raise`).
In normal use `check_compatible` blocks non-primitive partition sources
upstream, so
this base handler is reached only in edge cases: this is a latent robustness
fix with
no behavioral downside on the happy path.
## Are these changes tested?
Yes. A focused regression test was added in
`tests/table/test_partitioning.py`
(`test_to_partition_representation_unsupported_type`) that asserts the base
handler
raises `TypeError` for a non-primitive (`StructType`) source type. It fails
on the
old `return` (`DID NOT RAISE TypeError`) and passes after the change to
`raise`.
```
python -m pytest tests/table/test_partitioning.py \
-k to_partition_representation_unsupported_type -v
# -> 1 passed
```
The linters (ruff, ruff-format, mypy and the rest of the pre-commit suite)
pass on
both changed files. No dependencies changed, so `uv.lock` is untouched.
## Are there any user-facing changes?
No public API changes. The only observable difference is that an unsupported
partition source type now raises `TypeError` at its source (as it was always
meant
to) instead of silently yielding an exception instance as the partition
value.
--
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]