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]

Reply via email to