anxkhn opened a new pull request, #3614:
URL: https://github.com/apache/iceberg-python/pull/3614
```markdown
# Rationale for this change
`timestamp_to_nanos` and `timestamptz_to_nanos` in
`pyiceberg/utils/datetime.py`
silently corrupt the nanosecond value for timestamp strings whose fractional
second has 7 or 8 digits.
`ISO_TIMESTAMP_NANO` / `ISO_TIMESTAMPTZ_NANO` split the fraction into
`group(2)` `(.\d{1,6})?` (the microsecond digits) and `group(3)` `(\d{1,3})?`
(the sub-microsecond digits, i.e. fraction positions 7-9). Those functions
read
`group(3)` and add it as a bare integer:
```python
ns_str = match.group(3) or "0"
...
return datetime_to_nanos(...) + int(ns_str)
```
When the fraction has exactly 7 or 8 digits, `group(3)` is only 1 or 2
digits and
represents fraction positions 7-8, so it has to be right-padded to 3 digits
before
being interpreted as a number of nanoseconds. Without padding, the digits are
scaled as if they were the last positions of the fraction, so the value is
off by
up to ~999 ns:
```python
from pyiceberg.utils.datetime import timestamp_to_nanos
timestamp_to_nanos("2025-02-23T20:21:44.3756127")
# fraction .3756127 s == 375612700 ns
# expected: 1740342104375612700
# actual: 1740342104375612007 (off by -693 ns)
timestamp_to_nanos("2025-02-23T20:21:44.37561278")
# expected: 1740342104375612780
# actual: 1740342104375612078 (off by -702 ns)
```
6-digit and full 9-digit fractions were already correct, which is why the
bug went
unnoticed. `timestamptz_to_nanos` has the identical defect.
# Are these changes tested?
Yes. Both functions now right-pad `group(3)` to 3 digits with `ljust(3, "0")`
before it is read as nanoseconds, which is correct for every fraction width
(the
`group(3) is None` case is unchanged: `"0".ljust(3, "0") == "000"`).
`tests/utils/test_datetime.py::test_timestamp_to_nanos` and
`::test_timestamptz_to_nanos` previously only parametrized 6-digit and
9-digit
fractions, so the 7/8-digit boundary was untested. I added a 7-digit
(`.3756127`) and an 8-digit (`.37561278`) case to each. Those new cases fail
against the current code and pass with this fix; the full
`tests/utils/test_datetime.py` file passes (53 tests), and `make lint` is
clean.
# Are there any user-facing changes?
Yes, a bug fix: parsing a nanosecond timestamp string with a 7- or 8-digit
fractional second now returns the correct nanosecond value instead of a
silently
corrupted one. There is no API change.
```
<!-- Note for maintainers: this corrects silent numeric corruption on a
public
helper, so it may warrant the `changelog` label; I cannot set labels as
an
external contributor. -->
--
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]
