Put a keyword on a photograph, not only search for one
The catalog has been able to *find* by keyword since v1 — query.rs joins the keywords table, matches it exactly, and substring-matches it for free text — and nothing anywhere could ever put a word there. A user could filter to a keyword they had no way to apply. This is the missing half: create, rename, delete, list, assign, unassign, and the two reads a panel needs. Bulk-only for assignment, because keywording a selection is the common case rather than the exception — the photographer picks out the frames with the puffin in them and applies "puffin" once, in one transaction. Schema v6 adds `keyword_terms`, and deliberately does *not* touch the v1 join. The assignment keeps the word as text because the catalog is a rebuildable index and the durable copies of that fact — the sidecar, XMP dc:subject — both carry a string; a foreign key would mean a catalog rebuilt from sidecars had to invent identity rows before it could record anything, and would break the query path that already works. So the text is the fact, and the new table is only the identity a rename and a deletion can be keyed on. `keyword_terms.name` carries no unique index, which looks like an oversight and is not: two devices that each type "Iceland" are both right until they meet, and a constraint would abort the merge at that moment. Uniqueness is converged upon instead — create resolves an existing name, fuse_duplicates collapses a cross-device pair onto the smaller uuid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -15,7 +15,7 @@ use rusqlite::Connection;
|
||||
use crate::error::CatalogError;
|
||||
|
||||
/// Schema version this build writes and understands.
|
||||
pub const SCHEMA_VERSION: i64 = 5;
|
||||
pub const SCHEMA_VERSION: i64 = 6;
|
||||
|
||||
/// Apply migrations up to [`SCHEMA_VERSION`].
|
||||
///
|
||||
@@ -66,6 +66,12 @@ pub fn migrate(conn: &Connection) -> Result<i64, CatalogError> {
|
||||
tx.pragma_update(None, "user_version", 5)?;
|
||||
tx.commit()?;
|
||||
}
|
||||
if from < 6 {
|
||||
let tx = conn.unchecked_transaction()?;
|
||||
tx.execute_batch(V6)?;
|
||||
tx.pragma_update(None, "user_version", 6)?;
|
||||
tx.commit()?;
|
||||
}
|
||||
|
||||
Ok(from)
|
||||
}
|
||||
@@ -103,6 +109,20 @@ pub fn backfill(conn: &Connection) -> Result<Vec<(&'static str, usize)>, Catalog
|
||||
out.push(("default_versions", n));
|
||||
}
|
||||
|
||||
// v6: a vocabulary row for every word some image already carries.
|
||||
//
|
||||
// Three ways a catalog arrives holding assignments with no term behind
|
||||
// them, and all three are normal rather than exceptional: a library
|
||||
// keyworded by a build that predates this table, a catalog rebuilt from
|
||||
// sidecars (which carry the word and not the identity), and an import from
|
||||
// Lightroom or darktable (FR-CAT-14). Without this the words are
|
||||
// searchable but absent from the vocabulary list, which reads as the
|
||||
// keywords having been lost.
|
||||
let n = crate::keywords::adopt_orphan_terms(conn)?;
|
||||
if n > 0 {
|
||||
out.push(("keyword_terms", n));
|
||||
}
|
||||
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
@@ -134,15 +154,45 @@ pub fn configure(conn: &Connection) -> Result<(), CatalogError> {
|
||||
/// in [`V1`]: every `CREATE TABLE`/`CREATE INDEX` must name its object
|
||||
/// unqualified, which they do.
|
||||
pub fn v1_for_attached(schema_name: &str) -> String {
|
||||
V1.replace("CREATE TABLE ", &format!("CREATE TABLE {schema_name}."))
|
||||
rewrite_for_attached(V1, schema_name)
|
||||
// REFERENCES within an attached schema resolve to that schema already,
|
||||
// so foreign keys need no rewriting — but the ON clause of an index
|
||||
// does, and `CREATE INDEX x.name ON table` is the correct form.
|
||||
}
|
||||
|
||||
/// Every table this build knows about, rewritten to target an attached
|
||||
/// database.
|
||||
///
|
||||
/// [`v1_for_attached`] is kept alongside this rather than replaced by it: a
|
||||
/// remote catalog written by an older build genuinely has only the v1 tables,
|
||||
/// and the merge has to keep working against one (see
|
||||
/// [`crate::merge::merge_keywords`]). Building that case in a test needs a way
|
||||
/// to say "v1 and no more".
|
||||
///
|
||||
/// Only the migrations that *create* objects appear here. V2 through V5 are
|
||||
/// `ALTER TABLE ... ADD COLUMN`, and the columns they add are local index
|
||||
/// state — shadowing, trashing, cache pinning — that a merge never reads
|
||||
/// across the attachment.
|
||||
pub fn for_attached(schema_name: &str) -> String {
|
||||
format!(
|
||||
"{}\n{}",
|
||||
rewrite_for_attached(V1, schema_name),
|
||||
rewrite_for_attached(V6, schema_name)
|
||||
)
|
||||
}
|
||||
|
||||
/// Qualify every object a `CREATE` statement names with `schema_name`.
|
||||
///
|
||||
/// The rewrite is textual and therefore only as good as the naming discipline
|
||||
/// in the batches it is given: every `CREATE TABLE`/`CREATE INDEX` must name
|
||||
/// its object unqualified, which they do.
|
||||
fn rewrite_for_attached(sql: &str, schema_name: &str) -> String {
|
||||
sql.replace("CREATE TABLE ", &format!("CREATE TABLE {schema_name}."))
|
||||
.replace("CREATE INDEX ", &format!("CREATE INDEX {schema_name}."))
|
||||
.replace(
|
||||
"CREATE UNIQUE INDEX ",
|
||||
&format!("CREATE UNIQUE INDEX {schema_name}."),
|
||||
)
|
||||
// REFERENCES within an attached schema resolve to that schema already,
|
||||
// so foreign keys need no rewriting — but the ON clause of an index
|
||||
// does, and `CREATE INDEX x.name ON table` is the correct form.
|
||||
}
|
||||
|
||||
/// Mark each JPEG that sits beside a RAW of the same name.
|
||||
@@ -226,6 +276,63 @@ fn stem_of(path: &str) -> &str {
|
||||
}
|
||||
}
|
||||
|
||||
const V6: &str = r#"
|
||||
-- TRACES: FR-CAT-5 | FR-CAT-6 | FR-NC-9
|
||||
-- Keywords gain an identity, so that renaming and deleting one can cross
|
||||
-- between devices.
|
||||
--
|
||||
-- The v1 `keywords` table is the *assignment*: one row per (version, word),
|
||||
-- and the word is stored as text. That stays exactly as it is, and this
|
||||
-- migration adds nothing to it, for a reason that is easy to get backwards.
|
||||
--
|
||||
-- # Why assignments keep the text rather than pointing at a row here
|
||||
--
|
||||
-- The catalog is a rebuildable index (ARCH §6.12). What an image is keyworded
|
||||
-- with is authoritative in the sidecar and in XMP `dc:subject` (FR-CAT-13),
|
||||
-- and both of those carry a *string*. Rewriting the join to reference
|
||||
-- `keyword_terms(id)` would mean a catalog rebuilt from sidecars had to invent
|
||||
-- term rows before it could record a single assignment, and an integer that
|
||||
-- means nothing on the other device would sit where the durable fact belongs.
|
||||
-- It would also break `crate::query`, which matches `kw.keyword` directly and
|
||||
-- must keep hitting `keywords_term` on a 50k library (FR-CAT-6).
|
||||
--
|
||||
-- So the text is the fact and this table is the *identity*: it exists to give
|
||||
-- a rename and a deletion something a merge can key on, and to let a keyword
|
||||
-- exist in the vocabulary before any photograph carries it.
|
||||
CREATE TABLE keyword_terms (
|
||||
id INTEGER PRIMARY KEY,
|
||||
-- Device-independent identity, as `collections.uuid` is. The integer id is
|
||||
-- local and collides across devices.
|
||||
uuid TEXT NOT NULL UNIQUE,
|
||||
-- The word itself, and the value written into every assignment row.
|
||||
name TEXT NOT NULL,
|
||||
created INTEGER NOT NULL,
|
||||
-- Monotonic, bumped on every local edit. `crate::merge` compares these
|
||||
-- rather than timestamps, so a clock-skewed device cannot silently win.
|
||||
revision INTEGER NOT NULL DEFAULT 1,
|
||||
modified INTEGER NOT NULL,
|
||||
-- Tombstone, so a merge against a device that still holds the keyword does
|
||||
-- not resurrect it.
|
||||
deleted INTEGER NOT NULL DEFAULT 0
|
||||
);
|
||||
|
||||
-- Deliberately **not** UNIQUE.
|
||||
--
|
||||
-- Two devices that each type "Iceland" create two rows with two uuids, and
|
||||
-- both are correct until they meet. A unique constraint would abort the merge
|
||||
-- transaction at exactly that moment — the ordinary case, not a corner one.
|
||||
-- Uniqueness is instead reached by convergence: `crate::keywords::create`
|
||||
-- resolves an existing name locally, and `crate::keywords::fuse_duplicates`
|
||||
-- collapses a cross-device pair onto the lexicographically smaller uuid, which
|
||||
-- both devices compute identically without talking to each other.
|
||||
--
|
||||
-- Partial on `deleted = 0` because every lookup here is a live one: the
|
||||
-- vocabulary list, the resolve-by-name in `create`, and the fuse pass all
|
||||
-- exclude tombstones, and including them would grow the index with every
|
||||
-- keyword the library has ever had rather than with the ones it has.
|
||||
CREATE INDEX keyword_terms_name ON keyword_terms(name) WHERE deleted = 0;
|
||||
"#;
|
||||
|
||||
const V5: &str = r#"
|
||||
-- TRACES: FR-NC-6a | FR-CAT-9 | NFR-RES-4
|
||||
-- Offline availability: what is kept, why it is kept, and where it lives.
|
||||
@@ -671,6 +778,77 @@ mod tests {
|
||||
assert_eq!(bytes, 100, "the existing row is untouched");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_v5_catalog_keeps_its_keywords_and_gains_their_identities() {
|
||||
// TRACES: FR-CAT-5
|
||||
// The migration case that matters here: a library keyworded by an
|
||||
// import or an older build already has assignment rows, and they must
|
||||
// survive into the vocabulary rather than being left searchable but
|
||||
// invisible.
|
||||
let c = mem();
|
||||
for step in [V1, V2, V3, V4, V5] {
|
||||
c.execute_batch(step).unwrap();
|
||||
}
|
||||
c.pragma_update(None, "user_version", 5).unwrap();
|
||||
c.execute(
|
||||
"INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
c.execute(
|
||||
"INSERT INTO images(id, root_id, source_ref, added_at) VALUES (7, 1, 'IMG_7.CR3', 0)",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
c.execute(
|
||||
"INSERT INTO versions(id, image_id, uuid, name, is_default)
|
||||
VALUES (1, 7, 'v-7', 'Default', 1)",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
c.execute(
|
||||
"INSERT INTO keywords(version_id, keyword) VALUES (1, 'puffin')",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(migrate(&c).unwrap(), 5, "migrated from v5");
|
||||
assert_eq!(backfilled(&c, "keyword_terms"), 1);
|
||||
|
||||
let name: String = c
|
||||
.query_row("SELECT name FROM keyword_terms", [], |r| r.get(0))
|
||||
.unwrap();
|
||||
assert_eq!(name, "puffin");
|
||||
// The assignment is untouched — it is the durable fact, and the term
|
||||
// row is only its identity.
|
||||
let n: i64 = c
|
||||
.query_row("SELECT count(*) FROM keywords", [], |r| r.get(0))
|
||||
.unwrap();
|
||||
assert_eq!(n, 1);
|
||||
|
||||
// It runs on every open, so a second pass must find nothing to do.
|
||||
assert_eq!(backfilled(&c, "keyword_terms"), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_devices_may_both_hold_a_term_of_the_same_name() {
|
||||
// Deliberately not a unique index. Two devices each typing "Iceland"
|
||||
// is the ordinary case, and a constraint would abort the merge
|
||||
// transaction at exactly the moment they first sync.
|
||||
let c = mem();
|
||||
migrate(&c).unwrap();
|
||||
c.execute(
|
||||
"INSERT INTO keyword_terms(uuid, name, created, revision, modified)
|
||||
VALUES ('a', 'Iceland', 0, 1, 1), ('b', 'Iceland', 0, 1, 1)",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
let n: i64 = c
|
||||
.query_row("SELECT count(*) FROM keyword_terms", [], |r| r.get(0))
|
||||
.unwrap();
|
||||
assert_eq!(n, 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stems_ignore_directories_containing_dots() {
|
||||
assert_eq!(stem_of("2026.08/IMG_1.CR2"), "IMG_1");
|
||||
|
||||
Reference in New Issue
Block a user