You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by gi...@apache.org on 2026/09/18 21:25:22 UTC
(iceberg-python) branch main updated: fix(types): validate DecimalType scale, not just precision (#3985)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
The following commit(s) were added to refs/heads/main by this push:
new 0d584073f fix(types): validate DecimalType scale, not just precision (#3985)
0d584073f is described below
commit 0d584073f67948c46f34d4c2a1672455bc10b5c9
Author: Arul Jerald <12...@users.noreply.github.com>
AuthorDate: Fri Sep 18 20:18:36 2026 +0000
fix(types): validate DecimalType scale, not just precision (#3985)
* 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
* trigger CI rerun
---
pyiceberg/types.py | 9 +++++++++
tests/avro/test_reader.py | 2 +-
tests/avro/test_resolver.py | 6 +++---
tests/avro/test_writer.py | 2 +-
tests/table/test_partitioning.py | 2 +-
tests/test_conversions.py | 4 ++--
tests/test_schema.py | 2 +-
tests/test_types.py | 26 +++++++++++++++++++-------
tests/utils/test_schema_conversion.py | 4 ++--
9 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/pyiceberg/types.py b/pyiceberg/types.py
index 525ec21dc..3295beb66 100644
--- a/pyiceberg/types.py
+++ b/pyiceberg/types.py
@@ -336,6 +336,15 @@ class DecimalType(PrimitiveType):
raise ValidationError(f"Decimal precision must be between 1 and 38 (inclusive), got: {precision}")
return self
+ @model_validator(mode="after")
+ def check_scale(self) -> DecimalType:
+ precision = getattr(self, "precision", None) or self.root[0]
+ scale = getattr(self, "scale", None) or self.root[1]
+
+ if not (0 <= scale <= precision):
+ raise ValidationError(f"Decimal scale must be between 0 and the precision {precision} (inclusive), got: {scale}")
+ return self
+
@model_serializer
def ser_model(self) -> str:
"""Serialize the model to a string."""
diff --git a/tests/avro/test_reader.py b/tests/avro/test_reader.py
index 320fc3379..678f536e8 100644
--- a/tests/avro/test_reader.py
+++ b/tests/avro/test_reader.py
@@ -282,7 +282,7 @@ def test_fixed_reader() -> None:
def test_decimal_reader() -> None:
- assert construct_reader(DecimalType(19, 25)) == DecimalReader(19, 25)
+ assert construct_reader(DecimalType(25, 19)) == DecimalReader(25, 19)
def test_boolean_reader() -> None:
diff --git a/tests/avro/test_resolver.py b/tests/avro/test_resolver.py
index fc47ffec0..2611f9652 100644
--- a/tests/avro/test_resolver.py
+++ b/tests/avro/test_resolver.py
@@ -205,7 +205,7 @@ def test_resolve_float_to_double() -> None:
def test_resolve_decimal_to_decimal() -> None:
# DecimalType(P, S) to DecimalType(P2, S) where P2 > P
- assert resolve_reader(DecimalType(19, 25), DecimalType(22, 25)) == DecimalReader(19, 25)
+ assert resolve_reader(DecimalType(19, 10), DecimalType(22, 10)) == DecimalReader(19, 10)
def test_struct_not_aligned() -> None:
@@ -251,9 +251,9 @@ def test_decimal_not_aligned() -> None:
def test_resolve_decimal_to_decimal_reduce_precision() -> None:
# DecimalType(P, S) to DecimalType(P2, S) where P2 > P
with pytest.raises(ResolveError) as exc_info:
- _ = resolve_reader(DecimalType(19, 25), DecimalType(10, 25)) == DecimalReader(22, 25)
+ _ = resolve_reader(DecimalType(19, 10), DecimalType(10, 10)) == DecimalReader(22, 10)
- assert "Cannot reduce precision from decimal(19, 25) to decimal(10, 25)" in str(exc_info.value)
+ assert "Cannot reduce precision from decimal(19, 10) to decimal(10, 10)" in str(exc_info.value)
def test_resolve_decimal_to_decimal_change_scale() -> None:
diff --git a/tests/avro/test_writer.py b/tests/avro/test_writer.py
index c655156c2..4c3284e31 100644
--- a/tests/avro/test_writer.py
+++ b/tests/avro/test_writer.py
@@ -81,7 +81,7 @@ def test_fixed_writer() -> None:
def test_decimal_writer() -> None:
- assert construct_writer(DecimalType(19, 25)) == DecimalWriter(19, 25)
+ assert construct_writer(DecimalType(25, 19)) == DecimalWriter(25, 19)
def test_boolean_writer() -> None:
diff --git a/tests/table/test_partitioning.py b/tests/table/test_partitioning.py
index 57d3bc1c2..494addaef 100644
--- a/tests/table/test_partitioning.py
+++ b/tests/table/test_partitioning.py
@@ -225,7 +225,7 @@ def test_partition_type_missing_source_field(table_schema_simple: Schema) -> Non
[
(IntegerType(), 22),
(LongType(), 22),
- (DecimalType(5, 9), Decimal(19.25)),
+ (DecimalType(9, 5), Decimal(19.25)),
(DateType(), datetime.date(1925, 5, 22)),
(TimeType(), datetime.time(19, 25, 00)),
(TimestampType(), datetime.datetime(2022, 5, 1, 22, 1, 1)),
diff --git a/tests/test_conversions.py b/tests/test_conversions.py
index 9b73b2db8..ec04d393c 100644
--- a/tests/test_conversions.py
+++ b/tests/test_conversions.py
@@ -569,7 +569,7 @@ def test_datetime_obj_to_bytes(primitive_type: PrimitiveType, value: datetime |
(DoubleType(), 1.0, 1.0),
(DecimalType(9, 4), Decimal("123.4500"), "123.4500"),
(DecimalType(9, 0), Decimal("2"), "2"),
- (DecimalType(9, -20), Decimal("2E+20"), "2E+20"),
+ (DecimalType(9, 9), Decimal("0.123456789"), "0.123456789"),
(DateType(), date(2017, 11, 16), "2017-11-16"),
(TimeType(), time(22, 31, 8, 123456), "22:31:08.123456"),
(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
(DoubleType(), 1.0),
(DecimalType(9, 4), Decimal("123.4500")),
(DecimalType(9, 0), Decimal("2")),
- (DecimalType(9, -20), Decimal("2E+20")),
+ (DecimalType(9, 9), Decimal("0.123456789")),
(DateType(), date(2017, 11, 16)),
(TimeType(), time(22, 31, 8, 123456)),
(TimestampType(), datetime(2017, 11, 16, 22, 31, 8, 123456)),
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 872e95ce3..dcd54cd49 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -1105,7 +1105,7 @@ def test_add_nested_lists(primitive_fields: NestedField, table_v2: Table) -> Non
element_id=7,
element_type=ListType(
element_id=8,
- element_type=ListType(element_id=9, element_type=DecimalType(precision=11, scale=20)),
+ element_type=ListType(element_id=9, element_type=DecimalType(precision=20, scale=11)),
element_required=False,
),
element_required=False,
diff --git a/tests/test_types.py b/tests/test_types.py
index 375fb9cdb..eb8ae2ea5 100644
--- a/tests/test_types.py
+++ b/tests/test_types.py
@@ -504,14 +504,14 @@ def test_repr_binary() -> None:
def test_serialization_decimal() -> None:
- assert DecimalType(19, 25).model_dump_json() == '"decimal(19, 25)"'
+ assert DecimalType(25, 19).model_dump_json() == '"decimal(25, 19)"'
def test_deserialization_decimal() -> None:
- decimal = DecimalType.model_validate_json('"decimal(19, 25)"')
- assert decimal == DecimalType(19, 25)
- assert decimal.precision == 19
- assert decimal.scale == 25
+ decimal = DecimalType.model_validate_json('"decimal(25, 19)"')
+ assert decimal == DecimalType(25, 19)
+ assert decimal.precision == 25
+ assert decimal.scale == 19
def test_deserialization_decimal_failure() -> None:
@@ -522,11 +522,11 @@ def test_deserialization_decimal_failure() -> None:
def test_str_decimal() -> None:
- assert str(DecimalType(19, 25)) == "decimal(19, 25)"
+ assert str(DecimalType(25, 19)) == "decimal(25, 19)"
def test_repr_decimal() -> None:
- assert repr(DecimalType(19, 25)) == "DecimalType(precision=19, scale=25)"
+ assert repr(DecimalType(25, 19)) == "DecimalType(precision=25, scale=19)"
def test_repr_nested_field_default_nones_should_not_appear() -> None:
@@ -935,3 +935,15 @@ def test_decimal_precision_validation() -> None:
with pytest.raises(ValidationError, match="Decimal precision must be between 1 and 38"):
DecimalType(-5, 2)
+
+
+def test_decimal_scale_validation() -> None:
+ """Test that DecimalType rejects a scale that is negative or exceeds the precision."""
+ assert DecimalType(9, 9).scale == 9
+ assert DecimalType(9, 0).scale == 0
+
+ with pytest.raises(ValidationError, match="Decimal scale must be between 0 and the precision"):
+ DecimalType(10, -1)
+
+ with pytest.raises(ValidationError, match="Decimal scale must be between 0 and the precision"):
+ DecimalType(5, 10)
diff --git a/tests/utils/test_schema_conversion.py b/tests/utils/test_schema_conversion.py
index 44322849d..b110f92bd 100644
--- a/tests/utils/test_schema_conversion.py
+++ b/tests/utils/test_schema_conversion.py
@@ -317,9 +317,9 @@ def test_avro_list_missing_element_id() -> None:
def test_convert_decimal_type() -> None:
- avro_decimal_type = {"type": "bytes", "logicalType": "decimal", "precision": 19, "scale": 25}
+ avro_decimal_type = {"type": "bytes", "logicalType": "decimal", "precision": 25, "scale": 19}
actual = AvroSchemaConversion()._convert_logical_type(avro_decimal_type)
- expected = DecimalType(precision=19, scale=25)
+ expected = DecimalType(precision=25, scale=19)
assert actual == expected