Skip to content

Commit 91006d1

Browse files
committed
fix(types): validate DecimalType scale, not just precision
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 #3981
1 parent 1312c55 commit 91006d1

9 files changed

Lines changed: 39 additions & 18 deletions

File tree

‎pyiceberg/types.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,15 @@ def check_precision(self) -> DecimalType:
334334
raise ValidationError(f"Decimal precision must be between 1 and 38 (inclusive), got: {precision}")
335335
return self
336336

337+
@model_validator(mode="after")
338+
def check_scale(self) -> DecimalType:
339+
precision = getattr(self, "precision", None) or self.root[0]
340+
scale = getattr(self, "scale", None) or self.root[1]
341+
342+
if not (0 <= scale <= precision):
343+
raise ValidationError(f"Decimal scale must be between 0 and the precision {precision} (inclusive), got: {scale}")
344+
return self
345+
337346
@model_serializer
338347
def ser_model(self) -> str:
339348
"""Serialize the model to a string."""

‎tests/avro/test_reader.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ def test_fixed_reader() -> None:
282282

283283

284284
def test_decimal_reader() -> None:
285-
assert construct_reader(DecimalType(19, 25)) == DecimalReader(19, 25)
285+
assert construct_reader(DecimalType(25, 19)) == DecimalReader(25, 19)
286286

287287

288288
def test_boolean_reader() -> None:

‎tests/avro/test_resolver.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ def test_resolve_float_to_double() -> None:
205205

206206
def test_resolve_decimal_to_decimal() -> None:
207207
# DecimalType(P, S) to DecimalType(P2, S) where P2 > P
208-
assert resolve_reader(DecimalType(19, 25), DecimalType(22, 25)) == DecimalReader(19, 25)
208+
assert resolve_reader(DecimalType(19, 10), DecimalType(22, 10)) == DecimalReader(19, 10)
209209

210210

211211
def test_struct_not_aligned() -> None:
@@ -251,9 +251,9 @@ def test_decimal_not_aligned() -> None:
251251
def test_resolve_decimal_to_decimal_reduce_precision() -> None:
252252
# DecimalType(P, S) to DecimalType(P2, S) where P2 > P
253253
with pytest.raises(ResolveError) as exc_info:
254-
_ = resolve_reader(DecimalType(19, 25), DecimalType(10, 25)) == DecimalReader(22, 25)
254+
_ = resolve_reader(DecimalType(19, 10), DecimalType(10, 10)) == DecimalReader(22, 10)
255255

256-
assert "Cannot reduce precision from decimal(19, 25) to decimal(10, 25)" in str(exc_info.value)
256+
assert "Cannot reduce precision from decimal(19, 10) to decimal(10, 10)" in str(exc_info.value)
257257

258258

259259
def test_resolve_decimal_to_decimal_change_scale() -> None:

‎tests/avro/test_writer.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def test_fixed_writer() -> None:
8181

8282

8383
def test_decimal_writer() -> None:
84-
assert construct_writer(DecimalType(19, 25)) == DecimalWriter(19, 25)
84+
assert construct_writer(DecimalType(25, 19)) == DecimalWriter(25, 19)
8585

8686

8787
def test_boolean_writer() -> None:

‎tests/table/test_partitioning.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ def test_partition_type_missing_source_field(table_schema_simple: Schema) -> Non
225225
[
226226
(IntegerType(), 22),
227227
(LongType(), 22),
228-
(DecimalType(5, 9), Decimal(19.25)),
228+
(DecimalType(9, 5), Decimal(19.25)),
229229
(DateType(), datetime.date(1925, 5, 22)),
230230
(TimeType(), datetime.time(19, 25, 00)),
231231
(TimestampType(), datetime.datetime(2022, 5, 1, 22, 1, 1)),

‎tests/test_conversions.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ def test_datetime_obj_to_bytes(primitive_type: PrimitiveType, value: datetime |
569569
(DoubleType(), 1.0, 1.0),
570570
(DecimalType(9, 4), Decimal("123.4500"), "123.4500"),
571571
(DecimalType(9, 0), Decimal("2"), "2"),
572-
(DecimalType(9, -20), Decimal("2E+20"), "2E+20"),
572+
(DecimalType(9, 9), Decimal("0.123456789"), "0.123456789"),
573573
(DateType(), date(2017, 11, 16), "2017-11-16"),
574574
(TimeType(), time(22, 31, 8, 123456), "22:31:08.123456"),
575575
(TimestampType(), datetime(2017, 11, 16, 22, 31, 8, 123456), "2017-11-16T22:31:08.123456"),
@@ -594,7 +594,7 @@ def test_json_single_serialization(primitive_type: PrimitiveType, value: Any, ex
594594
(DoubleType(), 1.0),
595595
(DecimalType(9, 4), Decimal("123.4500")),
596596
(DecimalType(9, 0), Decimal("2")),
597-
(DecimalType(9, -20), Decimal("2E+20")),
597+
(DecimalType(9, 9), Decimal("0.123456789")),
598598
(DateType(), date(2017, 11, 16)),
599599
(TimeType(), time(22, 31, 8, 123456)),
600600
(TimestampType(), datetime(2017, 11, 16, 22, 31, 8, 123456)),

‎tests/test_schema.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1105,7 +1105,7 @@ def test_add_nested_lists(primitive_fields: NestedField, table_v2: Table) -> Non
11051105
element_id=7,
11061106
element_type=ListType(
11071107
element_id=8,
1108-
element_type=ListType(element_id=9, element_type=DecimalType(precision=11, scale=20)),
1108+
element_type=ListType(element_id=9, element_type=DecimalType(precision=20, scale=11)),
11091109
element_required=False,
11101110
),
11111111
element_required=False,

‎tests/test_types.py‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -504,14 +504,14 @@ def test_repr_binary() -> None:
504504

505505

506506
def test_serialization_decimal() -> None:
507-
assert DecimalType(19, 25).model_dump_json() == '"decimal(19, 25)"'
507+
assert DecimalType(25, 19).model_dump_json() == '"decimal(25, 19)"'
508508

509509

510510
def test_deserialization_decimal() -> None:
511-
decimal = DecimalType.model_validate_json('"decimal(19, 25)"')
512-
assert decimal == DecimalType(19, 25)
513-
assert decimal.precision == 19
514-
assert decimal.scale == 25
511+
decimal = DecimalType.model_validate_json('"decimal(25, 19)"')
512+
assert decimal == DecimalType(25, 19)
513+
assert decimal.precision == 25
514+
assert decimal.scale == 19
515515

516516

517517
def test_deserialization_decimal_failure() -> None:
@@ -522,11 +522,11 @@ def test_deserialization_decimal_failure() -> None:
522522

523523

524524
def test_str_decimal() -> None:
525-
assert str(DecimalType(19, 25)) == "decimal(19, 25)"
525+
assert str(DecimalType(25, 19)) == "decimal(25, 19)"
526526

527527

528528
def test_repr_decimal() -> None:
529-
assert repr(DecimalType(19, 25)) == "DecimalType(precision=19, scale=25)"
529+
assert repr(DecimalType(25, 19)) == "DecimalType(precision=25, scale=19)"
530530

531531

532532
def test_repr_nested_field_default_nones_should_not_appear() -> None:
@@ -935,3 +935,15 @@ def test_decimal_precision_validation() -> None:
935935

936936
with pytest.raises(ValidationError, match="Decimal precision must be between 1 and 38"):
937937
DecimalType(-5, 2)
938+
939+
940+
def test_decimal_scale_validation() -> None:
941+
"""Test that DecimalType rejects a scale that is negative or exceeds the precision."""
942+
assert DecimalType(9, 9).scale == 9
943+
assert DecimalType(9, 0).scale == 0
944+
945+
with pytest.raises(ValidationError, match="Decimal scale must be between 0 and the precision"):
946+
DecimalType(10, -1)
947+
948+
with pytest.raises(ValidationError, match="Decimal scale must be between 0 and the precision"):
949+
DecimalType(5, 10)

‎tests/utils/test_schema_conversion.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,9 +317,9 @@ def test_avro_list_missing_element_id() -> None:
317317

318318

319319
def test_convert_decimal_type() -> None:
320-
avro_decimal_type = {"type": "bytes", "logicalType": "decimal", "precision": 19, "scale": 25}
320+
avro_decimal_type = {"type": "bytes", "logicalType": "decimal", "precision": 25, "scale": 19}
321321
actual = AvroSchemaConversion()._convert_logical_type(avro_decimal_type)
322-
expected = DecimalType(precision=19, scale=25)
322+
expected = DecimalType(precision=25, scale=19)
323323
assert actual == expected
324324

325325

0 commit comments

Comments
 (0)