Skip to content

fix(types): validate DecimalType scale, not just precision - #3985

Open
ArulJerald wants to merge 2 commits into
apache:mainfrom
ArulJerald:issue-3981-decimal-type-scale-validation
Open

ArulJerald wants to merge 2 commits into
apache:mainfrom
ArulJerald:issue-3981-decimal-type-scale-validation

Conversation

@ArulJerald

Copy link
Copy Markdown

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 DecimalTypes 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.

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.

Fixes apache#3981
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DecimalType validates precision but not scale

1 participant