ArulJerald opened a new pull request, #3985: URL: https://github.com/apache/iceberg-python/pull/3985
DecimalType only bounded precision (1-38) via a model validator; scale was accepted unbounded, including negative or larger than precision, and flowed unchecked into downstream type construction and binding. Add an equivalent check enforcing 0 <= scale <= precision, and fix pre-existing test fixtures across the suite that relied on scale > precision or negative scale. Closes #3981 # Rationale for this change `DecimalType.check_precision` bounds `precision` to `[1, 38]`, but there was no equivalent check for `scale`. A `DecimalType` with a negative scale, or a scale greater than its precision, was silently accepted and could flow into downstream type construction, binding, and (de)serialization without ever being rejected. This adds a `check_scale` model validator (mirroring `check_precision`'s shape and error style) enforcing `0 <= scale <= precision`, matching the decimal semantics used by Parquet and Avro. ## Are these changes tested? Yes. Added `test_decimal_scale_validation` in `tests/test_types.py`, covering: - negative scale raises `ValidationError` - scale greater than precision raises `ValidationError` - the `scale == precision` and `scale == 0` boundary cases remain valid A number of pre-existing fixtures elsewhere in the suite constructed `DecimalType`s with scale > precision (e.g. `DecimalType(19, 25)`) or negative scale (e.g. `DecimalType(9, -20)`), which are now invalid under this rule. Updated to use valid parameters while preserving each test's original intent: `tests/avro/test_reader.py`, `tests/avro/test_resolver.py`, `tests/avro/test_writer.py`, `tests/table/test_partitioning.py`, `tests/test_conversions.py`, `tests/test_schema.py`, `tests/utils/test_schema_conversion.py`. ## Are there any user-facing changes? Yes. Constructing or parsing (e.g. from table metadata JSON) a `DecimalType` with a negative scale, or a scale greater than its precision, now raises `ValidationError` immediately instead of being silently accepted. -- 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]
