Skip to content

fix: pad sub-microsecond digits when parsing nanosecond timestamps#3614

Open
anxkhn wants to merge 1 commit into
apache:mainfrom
anxkhn:patch-13
Open

fix: pad sub-microsecond digits when parsing nanosecond timestamps#3614
anxkhn wants to merge 1 commit into
apache:mainfrom
anxkhn:patch-13

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 7, 2026

Copy link
Copy Markdown
Contributor
# 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:

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

timestamp_to_nanos and timestamptz_to_nanos read the sub-microsecond
digits of the fraction (regex group 3 of ISO_TIMESTAMP_NANO /
ISO_TIMESTAMPTZ_NANO) as a bare integer. For a 7- or 8-digit fraction
that group is 1 or 2 digits and represents positions 7-8 of the
fraction, so it must be right-padded to 3 digits before being
interpreted as nanoseconds. Without padding, .1234567 was parsed as
+7 ns instead of +700 ns, silently corrupting the value.

Right-pad the group to 3 digits with ljust so all fraction widths
round-trip correctly, and add 7- and 8-digit cases to the tests, which
previously only covered 6- and 9-digit fractions.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
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.

1 participant