Skip to content

fix(sqlite): correct sub-second decoding of pre-epoch REAL datetimes#4340

Open
zelinewang wants to merge 1 commit into
transact-rs:mainfrom
zelinewang:fix/sqlite-julianday-pre-epoch-subsecond
Open

fix(sqlite): correct sub-second decoding of pre-epoch REAL datetimes#4340
zelinewang wants to merge 1 commit into
transact-rs:mainfrom
zelinewang:fix/sqlite-julianday-pre-epoch-subsecond

Conversation

@zelinewang

Copy link
Copy Markdown

SQLite can store a datetime as a REAL holding a Julian day number, for example
the result of julianday(...). When such a value is read back into a chrono
type, decode_datetime_from_float converts the Julian day to a UNIX timestamp
and then splits it into whole seconds and nanoseconds:

let seconds = timestamp.trunc() as i64;
let nanos = (timestamp.fract() * 1E9).abs() as u32;
Utc.fix().timestamp_opt(seconds, nanos).single()

timestamp_opt always adds nanos forward in time. For timestamps before
the epoch the fractional part is negative, so trunc() (which rounds toward
zero) together with fract().abs() moves the result in the wrong direction.
The decoded instant ends up as much as ~2 seconds off, and for values in the
last second before the epoch it lands on the wrong side of it entirely. For
instance 1969-12-31 23:59:59.5 decodes as 1970-01-01 00:00:00.5.

The fix rounds the seconds toward negative infinity with floor() and takes
the sub-second remainder relative to that floor, so nanos is always a valid
non-negative forward offset:

let seconds = timestamp.floor();
let nanos = ((timestamp - seconds) * 1E9) as u32;
Utc.fix().timestamp_opt(seconds as i64, nanos).single()

Values at or after the epoch are unchanged, since there floor() equals
trunc() and the fraction is already non-negative. The integer decode path
(decode_datetime_from_int) was already exact for whole seconds; this brings
the REAL path in line with it.

Added unit tests in sqlx-sqlite/src/types/chrono.rs covering a pre-epoch
value with a sub-second component (which fails before this change and passes
after) and a post-epoch value (guarding against a regression). I also
confirmed the end-to-end path with an in-memory
SELECT julianday('1969-12-31 23:59:59.500') read as DateTime<Utc>: before
the change it returns 1970-01-01 00:00:00.500, after it returns
1969-12-31 23:59:59.500 (to within f64 round-off).

Does your PR solve an issue?

No linked issue. I came across this while looking at how the REAL/Julian-day
storage class is decoded; the existing datetime tests only cover the TEXT
formats, so this path was untested.

Is this a breaking change?

No. This is a correctness fix. It does change the decoded value for pre-1970
datetimes stored as REAL Julian day numbers, but only from an incorrect value
to the correct one; the public API is unchanged and post-epoch values are
unaffected.

Datetimes stored as a REAL Julian day number (e.g. via SQLite's
julianday()) were decoded by splitting the UNIX timestamp with
trunc()/fract().abs(). timestamp_opt() always adds the nanoseconds
forward in time, so for values before 1970 this pushed the result up to
~2 seconds off and could move it onto the wrong side of the epoch
(e.g. 1969-12-31 23:59:59.5 decoded as 1970-01-01 00:00:00.5).

Round seconds toward negative infinity with floor() and take the
sub-second remainder relative to that, so nanos is always a valid
forward offset. Post-epoch values are unaffected. Adds unit tests for
the pre- and post-epoch paths.
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