From 8093a8faab13c52a456ed04b599dd0169cce09ba Mon Sep 17 00:00:00 2001 From: Michael Netshipise Date: Sun, 6 Sep 2026 15:30:53 +0200 Subject: [PATCH] fix(derive): auto-stamped updated_at is SECONDS, not milliseconds The derive fills `updated_at` on an update whenever the form leaves it unset, and it filled it with `timestamp_millis()`. Every hand-written caller stamps these columns from a seconds clock, and `created_at` is not auto-filled at all, so a single INSERT-then-UPDATE left one row carrying `created_at` in seconds and `updated_at` in milliseconds. Nothing errors - the column is a bare i64 - so the rows quietly stop being comparable, and anything ordering, diffing or syncing on `updated_at` reads an auto-stamped row as roughly 56,000 years in the future. Found in Leptin on 2026-09-06 across 9 tables and 4152 rows, including 33 of 34 meals, and only because two columns on one row disagreed. Michael: "make this a default on sqlx-record-derive, seconds should be default." Seconds is the DEFAULT, not an override: a caller that sets updated_at itself still wins, so paths that already stamp deliberately are unaffected. Guarded by tests over the emitting source - the bug is a unit, not a behaviour, and a DB-backed test would need a fixture for something a string comparison settles exactly. Verified non-vacuous. Consumers should repair existing rows: any updated_at > 1e11 is milliseconds and divides by 1000. --- Cargo.toml | 2 +- sqlx-record-derive/src/lib.rs | 17 +++++- .../tests/updated_at_is_seconds.rs | 58 +++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 sqlx-record-derive/tests/updated_at_is_seconds.rs diff --git a/Cargo.toml b/Cargo.toml index 64e1eaf..72b9cdd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ edition.workspace = true description = "Entity CRUD and change tracking for SQL databases with SQLx" [workspace.package] -version = "0.3.8" +version = "0.3.9" edition = "2021" [dependencies] diff --git a/sqlx-record-derive/src/lib.rs b/sqlx-record-derive/src/lib.rs index 01a8b6f..b318146 100644 --- a/sqlx-record-derive/src/lib.rs +++ b/sqlx-record-derive/src/lib.rs @@ -1141,13 +1141,26 @@ fn generate_update_impl( quote! {} }; - // Auto-update updated_at timestamp (only if not manually set) + // Auto-update updated_at timestamp (only if not manually set). + // + // SECONDS, NOT MILLISECONDS. This used to emit `timestamp_millis()`, which + // silently disagreed with every hand-written path: application code stamps + // these columns from a seconds clock, and `created_at` is not auto-filled + // at all, so a single INSERT-then-UPDATE left one row carrying `created_at` + // in seconds and `updated_at` in milliseconds. Nothing errors — the column + // is a bare i64 — so the rows just quietly stop being comparable, and any + // ordering, diffing or sync keyed on `updated_at` sees every auto-stamped + // row as ~56000 years in the future. + // + // Found in Leptin 2026-09-06: 9 tables, 4152 rows, including 33 of 34 + // meals. The unit is a contract with the schema, and seconds is what the + // schema and the rest of the codebase mean. let updated_at_increment = if has_updated_at { quote! { // Only auto-set updated_at if not already set in form or via expression if self.updated_at.is_none() && !self._exprs.contains_key("updated_at") { parts.push(format!("updated_at = {}", ::sqlx_record::prelude::placeholder(idx))); - values.push(::sqlx_record::prelude::Value::Int64(chrono::Utc::now().timestamp_millis())); + values.push(::sqlx_record::prelude::Value::Int64(chrono::Utc::now().timestamp())); idx += 1; } } diff --git a/sqlx-record-derive/tests/updated_at_is_seconds.rs b/sqlx-record-derive/tests/updated_at_is_seconds.rs new file mode 100644 index 0000000..000e29b --- /dev/null +++ b/sqlx-record-derive/tests/updated_at_is_seconds.rs @@ -0,0 +1,58 @@ +//! The auto-stamped `updated_at` must be SECONDS. +//! +//! The derive fills `updated_at` on an update whenever the form leaves it +//! unset. It used to fill it with `timestamp_millis()`, which silently +//! disagreed with every hand-written path: callers stamp these columns from a +//! seconds clock, and `created_at` is not auto-filled at all. A single +//! INSERT-then-UPDATE therefore left one row with `created_at` in seconds and +//! `updated_at` in milliseconds. +//! +//! Nothing errors — the column is a bare `i64` — so rows just quietly stop +//! being comparable, and anything ordering, diffing or syncing on `updated_at` +//! reads an auto-stamped row as roughly 56,000 years in the future. It was +//! found in Leptin on 2026-09-06 across 9 tables and 4152 rows, including 33 +//! of 34 meals, and only because two columns on one row disagreed. +//! +//! Asserted against the emitting source rather than a live database: the bug +//! is a unit, not a behaviour, and a DB-backed test would need a fixture for +//! something a string comparison settles exactly. + +const DERIVE_SRC: &str = include_str!("../src/lib.rs"); + +/// The block that emits the auto-stamp, isolated so the assertions below +/// cannot be satisfied by an unrelated mention elsewhere in the file. +fn auto_stamp_block() -> &'static str { + let start = DERIVE_SRC + .find("let updated_at_increment") + .expect("the auto-stamp block should exist"); + let rest = &DERIVE_SRC[start..]; + let end = rest.find("quote! {}").expect("block should terminate"); + &rest[..end] +} + +#[test] +fn auto_stamped_updated_at_uses_seconds() { + assert!( + auto_stamp_block().contains("Utc::now().timestamp()"), + "the derive must stamp updated_at in seconds", + ); +} + +#[test] +fn auto_stamped_updated_at_is_never_milliseconds() { + assert!( + !auto_stamp_block().contains("timestamp_millis"), + "updated_at must not be stamped in milliseconds — it disagrees with \ + created_at and with every caller that uses a seconds clock", + ); +} + +#[test] +fn the_stamp_still_defers_to_an_explicit_value() { + // Seconds is the DEFAULT, not an override: a caller that sets updated_at + // itself must still win, or fixing the unit would break every path that + // already stamps deliberately. + let block = auto_stamp_block(); + assert!(block.contains("self.updated_at.is_none()"), "{block}"); + assert!(block.contains("_exprs.contains_key(\"updated_at\")"), "{block}"); +}