to_bytes silently rescales a Decimal with a negative scale

Open Beginner friendly
#3,996 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
databases

Research direction

Start at the pyiceberg.conversions.to_bytes entry point and decimal_to_unscaled, then run the supplied Decimal reproduction for DecimalType(10, 2). Done means mismatched signed scales are rejected rather than rescaled, while values already matching the type scale still round-trip correctly; check the impact on pyiceberg/manifest.py and pyiceberg/io/pyarrow.py.

Written by the indexing model from the issue text.

Description

to_bytes for DecimalType takes the absolute value of the exponent before comparing it
against the type's scale:

_, digits, exponent = value.as_tuple()
exponent = abs(int(exponent))
if exponent != primitive_type.scale:
    raise ValueError(...)

A Decimal carries the negated scale as its exponent, so a value with a negative scale
passes this check as if it had the matching positive one. decimal_to_unscaled then uses
only the digits, and the exponent is dropped:

sign, digits, _ = value.as_tuple()
return int(Decimal((sign, digits, 0)).to_integral_value())

The value is written four orders of magnitude off, with no error.

Reproduction

from decimal import Decimal
from pyiceberg.conversions import to_bytes, from_bytes
from pyiceberg.types import DecimalType

t = DecimalType(10, 2)
print(from_bytes(t, to_bytes(t, Decimal("1E+2"))))   # 0.01, expected 100.00
print(from_bytes(t, to_bytes(t, Decimal("5E+1"))))   # 0.50 for decimal(10, 1) -> 0.5, expected 50.0

This is not an exotic input. Decimal.normalize() produces exactly this form:

Decimal("100").normalize()   # Decimal('1E+2')

so a value that has been normalized, or that comes out of arithmetic that trims trailing
zeros, hits it.

Impact

to_bytes writes the lower_bounds and upper_bounds of a data file
(pyiceberg/manifest.py, _write_data_file_statistics), and the same conversion is used
in pyiceberg/io/pyarrow.py. A bound written as 0.01 instead of 100.00 makes scan
planning prune files that do hold matching rows, so a query silently returns fewer rows
than it should.

Values with a positive scale are unaffected: Decimal("100.00") round-trips correctly.

Suggested fix

Compare the signed scale, -exponent, against primitive_type.scale, so a mismatching
value is rejected instead of being silently rescaled. Rescaling the value to the type's
scale would be the other option, but that widens the contract of a function that today
requires an exact match.

Happy to open a PR.

Dominant language
Python
Stars
1.1k
Forks
589
Avg merge
2d 4h
Merged PRs (30d)
72

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from apache/iceberg-python

All issues in apache/iceberg-python

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.