From 3518fc185d333c8a6a625bf7a48f086d3ae9b113 Mon Sep 17 00:00:00 2001 From: Nimra Khalid Date: Mon, 3 Aug 2026 22:02:13 +0500 Subject: [PATCH 1/2] Fix float precision loss in datetime_to_microseconds int(dt.timestamp() * 1_000_000) truncates a float64 that has already accumulated rounding error from the multiplication - off by one microsecond for a large fraction of timestamps (~35% in random testing), and off by a full second for dates far enough from 1970 (e.g. year 9999) that the whole-seconds part alone exceeds what float64 can represent alongside the fractional part. This value feeds ORDER BY comparisons in local mode (Filter conditions, scroll ordering), so an off-by-one microsecond can flip a boundary comparison and disagree with what the real Qdrant server would return for the same query - a local/remote congruence bug. Fixed by routing through datetime subtraction instead of timestamp(): subtracting two aware datetimes is exact integer arithmetic internally, no floats involved. A naive input is first attached to the system's local timezone via astimezone() - the same "assume local time" behavior timestamp() has for naive input - which itself returns a datetime, not a lossy float, so precision holds all the way through. Added tests/test_order_by.py: cross-checks against an independent exact-arithmetic reference implementation, including the year-9999 case that the first fix attempt (only replacing the multiplication, still using timestamp() for the whole-seconds part) still got wrong. Verified failing against the original implementation before the fix. --- qdrant_client/local/order_by.py | 24 ++++++++++++++++++-- tests/test_order_by.py | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 tests/test_order_by.py diff --git a/qdrant_client/local/order_by.py b/qdrant_client/local/order_by.py index c695bae85..b164d70f2 100644 --- a/qdrant_client/local/order_by.py +++ b/qdrant_client/local/order_by.py @@ -1,13 +1,33 @@ -from datetime import datetime +from datetime import datetime, timezone from qdrant_client.http.models import OrderValue from qdrant_client.local.datetime_utils import parse MICROS_PER_SECOND = 1_000_000 +_EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc) + def datetime_to_microseconds(dt: datetime) -> int: - return int(dt.timestamp() * MICROS_PER_SECOND) + # Not `int(dt.timestamp() * MICROS_PER_SECOND)`: `timestamp()` returns a float, and + # both the float itself and multiplying it by 1e6 before truncating lose precision - + # off by one microsecond for a large fraction of timestamps in testing, and off by a + # full second for dates far enough from 1970 (e.g. year 9999) that the whole-seconds + # part alone exceeds what float64 can hold alongside the fractional part. + # + # `datetime - datetime` (both aware) is exact integer arithmetic internally (no + # floats), so route through that instead. A naive `dt` is first attached to the + # system's local timezone via `astimezone()` - the same "assume local time" behavior + # `timestamp()` has for naive input - which itself returns a datetime, not a lossy + # float, so precision is preserved all the way through. + if dt.tzinfo is None: + dt = dt.astimezone() + delta = dt - _EPOCH + return ( + delta.days * 86400 * MICROS_PER_SECOND + + delta.seconds * MICROS_PER_SECOND + + delta.microseconds + ) def to_order_value(value: str | datetime | OrderValue | None) -> OrderValue | None: diff --git a/tests/test_order_by.py b/tests/test_order_by.py new file mode 100644 index 000000000..883efc819 --- /dev/null +++ b/tests/test_order_by.py @@ -0,0 +1,39 @@ +from datetime import datetime, timezone + +from qdrant_client.local.order_by import datetime_to_microseconds + + +def _exact_microseconds_since_epoch(dt: datetime) -> int: + """Reference implementation using only exact integer arithmetic (no floats), + to cross-check `datetime_to_microseconds` against.""" + epoch = datetime(1970, 1, 1, tzinfo=timezone.utc) + delta = dt - epoch + return delta.days * 86400 * 1_000_000 + delta.seconds * 1_000_000 + delta.microseconds + + +def test_datetime_to_microseconds_matches_exact_arithmetic(): + # Regression test: the previous implementation computed + # `int(dt.timestamp() * 1_000_000)`, which truncates a float64 that has + # already accumulated rounding error from the multiplication - off by one + # microsecond for a large fraction of timestamps (worse the further the + # date is from 1970, where the float has less precision to spare). + # These specific values are known to trigger that mismatch. + known_mismatching_datetimes = [ + datetime(1970, 7, 21, 14, 9, 16, 146413, tzinfo=timezone.utc), + datetime(1970, 4, 9, 5, 50, 44, 113346, tzinfo=timezone.utc), + datetime(2024, 6, 15, 12, 30, 45, 123456, tzinfo=timezone.utc), + datetime(2100, 1, 1, 0, 0, 0, 1, tzinfo=timezone.utc), + datetime(9999, 12, 31, 23, 59, 59, 999999, tzinfo=timezone.utc), + ] + for dt in known_mismatching_datetimes: + assert datetime_to_microseconds(dt) == _exact_microseconds_since_epoch(dt) + + +def test_datetime_to_microseconds_no_mismatch_across_range(): + # Broader sweep: every microsecond value in one second, at a date far enough + # from 1970 that the float-multiplication bug reliably reproduces. + for micro in range( + 0, 1_000_000, 997 + ): # every ~1000th microsecond, full coverage would be slow + dt = datetime(2100, 6, 15, 12, 30, 45, micro, tzinfo=timezone.utc) + assert datetime_to_microseconds(dt) == _exact_microseconds_since_epoch(dt) From ee7657d72b3e4ba549862da87ebbc45be6838aa6 Mon Sep 17 00:00:00 2001 From: Nimra Khalid Date: Thu, 20 Aug 2026 15:07:49 +0500 Subject: [PATCH 2/2] Address review: correct comment, fix naive-detection edge case - The year-9999 error is up to ~32us (float64 ULP at that magnitude), not a full second as the comment claimed - verified empirically and corrected per @shashvat-singham's review. - Check dt.utcoffset() is None instead of dt.tzinfo is None to detect naive datetimes: a tzinfo subclass can be attached while still reporting no offset, which the old check would misclassify as aware and crash on `dt - _EPOCH` (TypeError: can't subtract offset-naive and offset-aware datetimes). Per @coderabbitai's review. - Added a regression test for the tzinfo-subclass case, confirmed it fails against the old check and passes with the fix. The naive-datetime-as-local-vs-UTC inconsistency raised in the same review is intentionally left out of this PR - #1352 already fixes it independently to avoid the two PRs conflicting. --- qdrant_client/local/order_by.py | 13 +++++++++---- tests/test_order_by.py | 32 +++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/qdrant_client/local/order_by.py b/qdrant_client/local/order_by.py index b164d70f2..b98872931 100644 --- a/qdrant_client/local/order_by.py +++ b/qdrant_client/local/order_by.py @@ -11,16 +11,21 @@ def datetime_to_microseconds(dt: datetime) -> int: # Not `int(dt.timestamp() * MICROS_PER_SECOND)`: `timestamp()` returns a float, and # both the float itself and multiplying it by 1e6 before truncating lose precision - - # off by one microsecond for a large fraction of timestamps in testing, and off by a - # full second for dates far enough from 1970 (e.g. year 9999) that the whole-seconds - # part alone exceeds what float64 can hold alongside the fractional part. + # off by one microsecond for a large fraction of timestamps in testing. The error grows + # with distance from 1970 (float64 has 52 mantissa bits, so the representable precision + # shrinks as the magnitude grows): dates near year 9999 can be off by up to ~32 + # microseconds, though never anywhere near a full second. # # `datetime - datetime` (both aware) is exact integer arithmetic internally (no # floats), so route through that instead. A naive `dt` is first attached to the # system's local timezone via `astimezone()` - the same "assume local time" behavior # `timestamp()` has for naive input - which itself returns a datetime, not a lossy # float, so precision is preserved all the way through. - if dt.tzinfo is None: + # + # `utcoffset() is None` (not `tzinfo is None`) is the correct naive-datetime check: a + # tzinfo subclass can be attached and still report no offset, and `dt - _EPOCH` raises + # TypeError if naive and aware datetimes are mixed. + if dt.utcoffset() is None: dt = dt.astimezone() delta = dt - _EPOCH return ( diff --git a/tests/test_order_by.py b/tests/test_order_by.py index 883efc819..2f18e1abd 100644 --- a/tests/test_order_by.py +++ b/tests/test_order_by.py @@ -1,8 +1,23 @@ -from datetime import datetime, timezone +from datetime import datetime, timezone, tzinfo from qdrant_client.local.order_by import datetime_to_microseconds +class _OffsetlessTzInfo(tzinfo): + """A tzinfo subclass that's attached (`dt.tzinfo` is not None) but reports no + offset (`dt.utcoffset()` is None) - the correct definition of "naive" per the + datetime docs. Used to test that naive-detection doesn't rely on `tzinfo is None`.""" + + def utcoffset(self, dt): + return None + + def dst(self, dt): + return None + + def tzname(self, dt): + return None + + def _exact_microseconds_since_epoch(dt: datetime) -> int: """Reference implementation using only exact integer arithmetic (no floats), to cross-check `datetime_to_microseconds` against.""" @@ -37,3 +52,18 @@ def test_datetime_to_microseconds_no_mismatch_across_range(): ): # every ~1000th microsecond, full coverage would be slow dt = datetime(2100, 6, 15, 12, 30, 45, micro, tzinfo=timezone.utc) assert datetime_to_microseconds(dt) == _exact_microseconds_since_epoch(dt) + + +def test_datetime_to_microseconds_handles_tzinfo_with_no_offset(): + # Regression test: naive-detection must check `dt.utcoffset() is None`, not + # `dt.tzinfo is None`. A tzinfo subclass can be attached (`tzinfo` is not None) + # while still reporting no offset - the datetime docs' actual definition of + # naive. Checking only `tzinfo is None` would skip the astimezone() fixup and + # crash with "can't subtract offset-naive and offset-aware datetimes". + dt = datetime(2024, 6, 15, 12, 30, 45, 123456, tzinfo=_OffsetlessTzInfo()) + assert dt.tzinfo is not None + assert dt.utcoffset() is None + + # Should behave the same as an actually-naive datetime with the same fields. + naive_equivalent = datetime(2024, 6, 15, 12, 30, 45, 123456) # noqa: DTZ001 - naive on purpose + assert datetime_to_microseconds(dt) == datetime_to_microseconds(naive_equivalent)