Skip to content

fix(sqlite): return an error instead of panicking when decoding an out-of-range INTEGER as PrimitiveDateTime - #4433

Open
breken-ai wants to merge 1 commit into
transact-rs:mainfrom
breken-ai:fix/sqlite-time-integer-out-of-range
Open

breken-ai wants to merge 1 commit into
transact-rs:mainfrom
breken-ai:fix/sqlite-time-integer-out-of-range

Conversation

@breken-ai

Copy link
Copy Markdown

Does your PR solve an issue?

No open issue.

Decoding time::PrimitiveDateTime from an SQLite INTEGER value panics if the value is outside time's range. A common case is a millisecond Unix timestamp, e.g. a column filled from JavaScript's Date.now():

let dt: PrimitiveDateTime = sqlx::query_scalar("SELECT 1700000000000").fetch_one(&mut conn).await?;
// thread panicked at sqlx-sqlite/src/types/time.rs:168:78:
// called `Result::unwrap()` on an `Err` value: ComponentRange { name: "timestamp", is_conditional: false }

The integer branch of decode_datetime calls OffsetDateTime::from_unix_timestamp(..).unwrap(). It has done this since at least 0.7.0. The same file's OffsetDateTime decode uses ? there, and the chrono impls return None, which becomes a decode error. So only PrimitiveDateTime takes down the calling task instead of returning an Err the application can handle.

The fix replaces .unwrap() with ?. It adds a regression test to tests/sqlite/types.rs (it_errors_on_out_of_range_integer_primitive_date_time).

  • Before the fix: DATABASE_URL=sqlite://tests/sqlite/sqlite.db cargo test --no-default-features --features sqlite,runtime-tokio,time,macros,migrate --test sqlite-types panics in the new test.
  • After the fix: 53/53 pass. cargo test -p sqlx-sqlite --all-features passes, cargo fmt --check is clean, and clippy shows no new warnings in sqlx-sqlite.

I used an AI assistant (Claude) to help find and write this fix. I reviewed the change and ran the tests above myself.

Is this a breaking change?

No. A value that used to panic now returns a decode error. Values in range decode the same as before.

…EGER PrimitiveDateTime

Decoding `time::PrimitiveDateTime` from an INTEGER column called
`OffsetDateTime::from_unix_timestamp(..).unwrap()`, so any value outside
`time`'s range (e.g. a millisecond Unix timestamp such as 1700000000000)
panicked while decoding the row. `OffsetDateTime` and the chrono types
already return an error here; do the same.

@abonander abonander left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you rebase it should fix the CI failure.

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.

2 participants