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.
This commit is contained in:
parent
de2a94cb1e
commit
8093a8faab
|
|
@ -5,7 +5,7 @@ edition.workspace = true
|
||||||
description = "Entity CRUD and change tracking for SQL databases with SQLx"
|
description = "Entity CRUD and change tracking for SQL databases with SQLx"
|
||||||
|
|
||||||
[workspace.package]
|
[workspace.package]
|
||||||
version = "0.3.8"
|
version = "0.3.9"
|
||||||
edition = "2021"
|
edition = "2021"
|
||||||
|
|
||||||
[dependencies]
|
[dependencies]
|
||||||
|
|
|
||||||
|
|
@ -1141,13 +1141,26 @@ fn generate_update_impl(
|
||||||
quote! {}
|
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 {
|
let updated_at_increment = if has_updated_at {
|
||||||
quote! {
|
quote! {
|
||||||
// Only auto-set updated_at if not already set in form or via expression
|
// 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") {
|
if self.updated_at.is_none() && !self._exprs.contains_key("updated_at") {
|
||||||
parts.push(format!("updated_at = {}", ::sqlx_record::prelude::placeholder(idx)));
|
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;
|
idx += 1;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -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}");
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue