From 458304859516cc066033ef6f189a4dc1d07d8219 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 25 Sep 2026 22:05:02 -0400 Subject: [PATCH] Find a sidecar's photographs by an index range, not a case-insensitive LIKE When a scan pull takes in sidecars another device wrote, `apply_judgement` finds the photographs each one describes with `source_ref LIKE '{stem}.%'`. SQLite's LIKE folds ASCII case, and nothing indexes `source_ref` case-insensitively, so every lookup read all 24,000 names of the root through the `(root_id, source_ref)` index: 1.5-2 ms per sidecar, 520-690 ms for the 342 `.drsc` files the reference catalog has read. Another device culling a shoot is several hundred of them. Every name beginning `{stem}.` lies in the half-open range `[{stem}., {stem}/)` -- `/` is the byte after `.` -- which the unique key serves as a seek. The rows LIKE matched beyond these differed only in case, and the check that decides, `sidecar_path(source) == sidecar`, has always compared exactly and refused them; the escaping of `%` and `_` goes too, since a range has no wildcards. persist_bench: 342 lookups 634-691 ms -> 9 ms CPU. Checked against the reference catalog directly as well: for all 18,430 distinct sidecar names its images imply, the range and the old LIKE, each filtered by `sidecar_path`, pick the same photographs. A test pins the neighbours of the range: a case variant, a longer stem, a subfolder named like the stem, and a folder whose name holds `%` and `_`. The XMP reader's LIKE (`xmp_sync::images_for`) is left alone: its check is case-insensitive, so the range would not be a superset there, and it only runs when the exact darktable-style name is not found. --- ui/dr-ui/src/library/scan.rs | 59 ++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/ui/dr-ui/src/library/scan.rs b/ui/dr-ui/src/library/scan.rs index 55b1855..dc17b8e 100644 --- a/ui/dr-ui/src/library/scan.rs +++ b/ui/dr-ui/src/library/scan.rs @@ -458,11 +458,11 @@ pub(super) fn record_sidecar_read( /// /// # Why the match is verified in Rust /// -/// The `LIKE` narrows the search to rows sharing the stem, and it is only a +/// The query narrows the search to rows sharing the stem, and it is only a /// filter: `library::sidecar_path` is what actually decides, applied to each -/// candidate. A path holding a `%`, a `_` or a bracket would otherwise match -/// more than it should, and a judgement landing on the wrong photograph is a -/// silent, permanent wrong. +/// candidate. A judgement landing on the wrong photograph is a silent, +/// permanent wrong, so the rule lives in one place and the SQL only has to +/// return a superset of what it accepts. /// /// Returns how many images gained a judgement they did not have. pub(super) fn apply_judgement( @@ -479,24 +479,25 @@ pub(super) fn apply_judgement( .unwrap_or(sidecar) .to_string(); - // The escape is the point: `%` and `_` are wildcards, and a photographer's - // folder is entitled to contain both. - let prefix = stem - .replace('\\', "\\\\") - .replace('%', "\\%") - .replace('_', "\\_"); - + // Every name that starts `{stem}.`, as a range over the + // `(root_id, source_ref)` key: `/` is the byte after `.`, so the half-open + // range holds exactly those names. It was a `LIKE`, and a `LIKE` is + // case-insensitive, which no index here can serve -- every sidecar a pull + // took in read all 24,000 names of the root, 1.5 ms each. The names it + // matched beyond these differed only in case, and the check below has + // always refused them: `sidecar_path` compares exactly. let candidates: Vec<(i64, String)> = { let mut stmt = conn - .prepare( + .prepare_cached( "SELECT id, source_ref FROM images - WHERE root_id = ?1 AND source_ref LIKE ?2 ESCAPE '\\'", + WHERE root_id = ?1 AND source_ref >= ?2 AND source_ref < ?3", ) .map_err(|e| e.to_string())?; let rows = stmt - .query_map(rusqlite::params![root_id, format!("{prefix}.%")], |r| { - Ok((r.get(0)?, r.get(1)?)) - }) + .query_map( + rusqlite::params![root_id, format!("{stem}."), format!("{stem}/")], + |r| Ok((r.get(0)?, r.get(1)?)), + ) .map_err(|e| e.to_string())?; rows.filter_map(Result::ok) .filter(|(_, source): &(i64, String)| sidecar_path(source) == sidecar) @@ -980,6 +981,32 @@ mod tests { ); } + /// The lookup is a range over names starting `{stem}.`, so the names + /// either side of that range — a longer stem, a case variant, a folder + /// whose name holds the wildcards the old `LIKE` had to escape — must + /// neither be missed nor caught. + #[test] + fn a_sidecar_reaches_exactly_its_own_photographs() { + let cat = library(&[ + "2026/50%_off/a.CR2", + "2026/50%_off/a.jpg", + "2026/50%_off/A.CR2", + "2026/50%_off/a.b.CR2", + "2026/50%_off/ab.CR2", + "2026/50%_off/a/b.CR2", + "2026/50Xxoff/a.CR2", + ]); + let n = apply_judgement(cat.connection(), 1, "2026/50%_off/a.drsc", 2, 0, 0).unwrap(); + + assert_eq!(n, 2); + let judged: Vec = judgements(&cat) + .into_iter() + .filter(|(_, rating, _)| *rating == 2) + .map(|(path, _, _)| path) + .collect(); + assert_eq!(judged, vec!["2026/50%_off/a.CR2", "2026/50%_off/a.jpg"]); + } + /// A demotion has to travel. Taking the larger of the two would refuse /// every rating the photographer ever lowered — and lowering one is most /// of what a second pass over a shoot does.