diff --git a/core/dr-catalog/src/schema.rs b/core/dr-catalog/src/schema.rs index e78d407..7fbe885 100644 --- a/core/dr-catalog/src/schema.rs +++ b/core/dr-catalog/src/schema.rs @@ -15,7 +15,7 @@ use rusqlite::Connection; use crate::error::CatalogError; /// Schema version this build writes and understands. -pub const SCHEMA_VERSION: i64 = 19; +pub const SCHEMA_VERSION: i64 = 20; /// Apply migrations up to [`SCHEMA_VERSION`]. /// @@ -188,9 +188,98 @@ pub fn migrate(conn: &Connection) -> Result { tx.commit()?; } + if from < 20 { + let tx = conn.unchecked_transaction()?; + v20_markers_name_the_detector_that_found_the_faces(&tx)?; + tx.pragma_update(None, "user_version", 20)?; + tx.commit()?; + } + Ok(from) } +// V20 -- TRACES: FR-CAT-7 +// +// Run markers that named the wrong detector, put right. +// +// `faces::record_updates` -- the write behind the quality, eye and crop +// passes -- re-marked an image under the pipeline the pass ran as, while +// the faces it had updated kept the id of the detector that found them. +// A marker of `scrfd_10g+w600k_mbf` over faces spelled `w600k_mbf` reads, +// to every consumer, as the thorough detector having examined the image: +// the upgrade repair skips it, and `face_shard::export_to_shards` selects +// its faces by the marker's id, finds none, and tells every other device +// that the thorough detector found nothing there. The desktop's shard index +// held 54 such entries over photographs with named faces, and the tablet's +// eye pass over faces it had adopted from the desktop had made 430 more. +// +// The write is fixed to keep the marker under the faces' own id. This puts +// the markers already written right, with a fresh time so the export sends +// each image again under an entry newer than the empty one -- which is what +// `held_model` orders by. Where the right marker is still there beside the +// wrong one (the old write inserted rather than replaced), the wrong one +// goes and the right one is refreshed for the same reason: its entry in +// the shards is older than the empty one, and a device that has neither +// would take the empty one. An image V14 left with faces and no marker at +// all is not touched: that state is the quality pass's cue, and the fixed +// write marks it correctly when the pass reaches it. +// +// Restated in Rust rather than SQL because the embedder half of a pipeline +// id is `faces::embedder_sql`, which this must agree with. +fn v20_markers_name_the_detector_that_found_the_faces(tx: &Connection) -> Result<(), CatalogError> { + let fi = crate::faces::embedder_sql("face_index.model_id"); + let f = crate::faces::embedder_sql("f.model_id"); + // A marker is wrong when the image holds faces of its embedder under + // another id. First the wrong ones that sit beside a right one -- the + // update below would collide with it -- then the rest are renamed. + let wrong = format!( + "EXISTS (SELECT 1 FROM faces f + WHERE f.image_id = face_index.image_id + AND {f} = {fi} + AND f.model_id != face_index.model_id)" + ); + let found_by = format!( + "(SELECT MIN(f.model_id) FROM faces f + WHERE f.image_id = face_index.image_id AND {f} = {fi})" + ); + let now = crate::faces::now_secs(); + tx.execute( + &format!( + "UPDATE face_index + SET indexed_at = ?1 + WHERE model_id = {found_by} + AND EXISTS (SELECT 1 FROM face_index w + WHERE w.image_id = face_index.image_id + AND w.model_id != face_index.model_id + AND {} = {fi})", + crate::faces::embedder_sql("w.model_id") + ), + [now], + )?; + tx.execute( + &format!( + "DELETE FROM face_index + WHERE {wrong} + AND EXISTS (SELECT 1 FROM face_index o + WHERE o.image_id = face_index.image_id + AND o.model_id = {found_by})" + ), + [], + )?; + tx.execute( + &format!( + "UPDATE face_index + SET model_id = {found_by}, + faces_found = (SELECT COUNT(*) FROM faces f + WHERE f.image_id = face_index.image_id AND {f} = {fi}), + indexed_at = ?1 + WHERE {wrong}" + ), + [now], + )?; + Ok(()) +} + /// The seven columns V16 adds to `faces`, in the order the readers name them. /// /// Named once because three places have to agree on them: this migration, @@ -1854,6 +1943,90 @@ mod tests { assert_eq!(faces, 2); } + #[test] + fn v20_renames_markers_to_the_detector_that_found_the_faces() { + let c = mem(); + c.pragma_update(None, "user_version", 0).unwrap(); + migrate(&c).unwrap(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'test')", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES (1,1,'a',0),(2,1,'b',0),(3,1,'c',0),(4,1,'d',0),(5,1,'e',0)", + [], + ) + .unwrap(); + // 1: the desktop's case -- old faces, re-marked as thorough. + // 2: the tablet's case -- adopted thorough faces, re-marked int8, + // and the right marker still beside it (refreshed, so it is + // exported again over the empty entry). + // 3: right already. 4: examined and empty. 5: V14's state, faces + // and no marker. + for (image, model) in [ + (1, "scrfd_10g+w600k_mbf"), + (2, "scrfd_10g_i8+w600k_mbf"), + (2, "scrfd_10g+w600k_mbf"), + (3, "scrfd_10g+w600k_mbf"), + (4, "scrfd_10g+w600k_mbf"), + ] { + c.execute( + "INSERT INTO face_index(image_id, model_id, indexed_at, faces_found, source_edge) + VALUES (?1, ?2, 100, 0, 6000)", + rusqlite::params![image, model], + ) + .unwrap(); + } + for (image, model) in [ + (1, "w600k_mbf"), + (1, "w600k_mbf"), + (2, "scrfd_10g+w600k_mbf"), + (3, "scrfd_10g+w600k_mbf"), + (5, "w600k_mbf"), + ] { + c.execute( + "INSERT INTO faces + (image_id, x, y, w, h, landmarks, detector_confidence, embedding, + crop_px, model_id, detected_at) + VALUES (?1, 0.1, 0.1, 0.2, 0.2, X'00', 0.9, X'00', 180.0, ?2, 0)", + rusqlite::params![image, model], + ) + .unwrap(); + } + c.pragma_update(None, "user_version", 19).unwrap(); + + migrate(&c).unwrap(); + + let markers: Vec<(i64, String, i64, bool)> = c + .prepare( + "SELECT image_id, model_id, faces_found, indexed_at > 100 + FROM face_index ORDER BY image_id, model_id", + ) + .unwrap() + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?))) + .unwrap() + .map(Result::unwrap) + .collect(); + assert_eq!( + markers, + vec![ + (1, "w600k_mbf".to_string(), 2, true), + (2, "scrfd_10g+w600k_mbf".to_string(), 0, true), + (3, "scrfd_10g+w600k_mbf".to_string(), 0, false), + (4, "scrfd_10g+w600k_mbf".to_string(), 0, false), + ] + ); + // Re-enterable: nothing left to rename. + c.pragma_update(None, "user_version", 19).unwrap(); + migrate(&c).unwrap(); + let n: i64 = c + .query_row("SELECT count(*) FROM face_index", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 4); + } + #[test] fn job_uniqueness_coalesces_rather_than_duplicating() { let c = mem();