From bccc14ffef3170dd2a3f6b6792738d8f20d6b1ca Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 12:01:36 +0200 Subject: [PATCH] Refuse an unversioned database that already has a files table (closes #11) A database at user_version 0 that already has a files table was made by something else: scan used to run its CREATE TABLE on it and fail with a raw SQLite error, and report and trees gave only a bare version mismatch. All three now refuse such a database with the schema-version error telling the operator to remove the file and rescan. scan creates the table and index and sets the version in one transaction, so a first scan stopped partway leaves an empty database the next scan sets up, never a files table at version 0. A genuinely empty database is unchanged. Model: opus-4-8 (implementation); opus-5-5 (rebase) --- README.md | 7 +++++- TODO.md | 4 +++ db.go | 59 ++++++++++++++++++++++++++++++++++++++++++--- db_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index afbf225..5859aed 100644 --- a/README.md +++ b/README.md @@ -329,7 +329,12 @@ All three subcommands operate on a single SQLite database file: `-shm` files beside it, which SQLite creates with the database file's permissions. - Schema (`PRAGMA user_version` is the schema version, currently 1; a - database with any other version is a fatal error): + database with any other version is a fatal error. `scan` creates + the schema and sets the version in one transaction, so a first scan + stopped while doing so leaves an empty database the next scan sets + up. A database at version 0 that already has a `files` table was + therefore not made by sfdupes; every subcommand refuses it with an + error telling the user to remove the file and rescan): ```sql CREATE TABLE files ( diff --git a/TODO.md b/TODO.md index c912a5c..ac052fc 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,10 @@ # Completed Steps +- `scan` creates the schema in one transaction; a version-0 database with a + `files` table is refused with a clear schema-version error (2026-10-04, + https://git.eeqj.de/sneak/sfdupes/issues/11) + - README documents install, Docker, a daily cron scan and how to read and check the reports (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/54) diff --git a/db.go b/db.go index 7633ac0..c2ec0a3 100644 --- a/db.go +++ b/db.go @@ -219,6 +219,12 @@ func openReportDatabase(ctx context.Context, } v, err := userVersion(ctx, db) + if err == nil && v == 0 { + // An empty database passes this check and fails the version + // check below. + err = checkUnversioned(ctx, db) + } + if err != nil { _ = db.Close() @@ -245,6 +251,11 @@ func initSchema(ctx context.Context, db *sql.DB) error { switch v { case 0: + err = checkUnversioned(ctx, db) + if err != nil { + return err + } + return createSchema(ctx, db) case schemaVersion: return nil @@ -254,25 +265,65 @@ func initSchema(ctx context.Context, db *sql.DB) error { } } +// checkUnversioned checks a database at user_version 0 before it is +// taken for an empty one. createSchema creates the files table and +// sets the version together, so a files table at version 0 was made by +// something else. Adopting it could corrupt unrelated data, so that is +// a schema-version error telling the operator to remove the file and +// rescan. +func checkUnversioned(ctx context.Context, db *sql.DB) error { + var name string + + err := db.QueryRowContext(ctx, + "SELECT name FROM sqlite_master "+ + "WHERE type = 'table' AND name = 'files'").Scan(&name) + + switch { + case err == nil: + return fmt.Errorf( + "has a files table but no schema version; "+ + "remove the file and rescan: %w", errSchemaVersion) + case errors.Is(err, sql.ErrNoRows): + return nil + default: + return fmt.Errorf("check for files table: %w", err) + } +} + // createSchema applies the schema to a fresh database and stamps the -// schema version. +// schema version in one transaction, so a creation stopped partway, by +// an interrupt or an error, leaves an empty database the next scan +// sets up, never a files table at version 0, which checkUnversioned +// refuses. func createSchema(ctx context.Context, db *sql.DB) error { - _, err := db.ExecContext(ctx, createTableSQL) + tx, err := db.BeginTx(ctx, nil) if err != nil { return fmt.Errorf("create schema: %w", err) } - _, err = db.ExecContext(ctx, createIndexSQL) + defer func() { _ = tx.Rollback() }() + + _, err = tx.ExecContext(ctx, createTableSQL) if err != nil { return fmt.Errorf("create schema: %w", err) } - _, err = db.ExecContext(ctx, + _, err = tx.ExecContext(ctx, createIndexSQL) + if err != nil { + return fmt.Errorf("create schema: %w", err) + } + + _, err = tx.ExecContext(ctx, "PRAGMA user_version = "+strconv.Itoa(schemaVersion)) if err != nil { return fmt.Errorf("set schema version: %w", err) } + err = tx.Commit() + if err != nil { + return fmt.Errorf("create schema: %w", err) + } + return nil } diff --git a/db_test.go b/db_test.go index 7bbb2b8..2fc6df5 100644 --- a/db_test.go +++ b/db_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "slices" + "strings" "testing" ) @@ -77,6 +78,76 @@ func TestOpenScanDatabaseCreates(t *testing.T) { } } +func TestOpenDatabaseUnversionedForeign(t *testing.T) { + t.Parallel() + + // A database that has a files table but user_version 0, written by + // some other tool. report, trees and scan must refuse it with the + // schema-version error, not adopt it and not emit a raw SQLite + // "table files already exists". + path := testDBPath(t) + + db, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + + _, err = db.ExecContext(t.Context(), "CREATE TABLE files (x INTEGER)") + if err != nil { + t.Fatal(err) + } + + _ = db.Close() + + _, err = openReportDatabase(t.Context(), path) + if !errors.Is(err, errSchemaVersion) || + !strings.Contains(err.Error(), "remove the file and rescan") { + t.Fatalf("report: err = %v, want errSchemaVersion telling the "+ + "operator to remove the file and rescan", err) + } + + _, err = openScanDatabase(t.Context(), path) + if !errors.Is(err, errSchemaVersion) || + !strings.Contains(err.Error(), "remove the file and rescan") { + t.Fatalf("scan: err = %v, want errSchemaVersion telling the "+ + "operator to remove the file and rescan", err) + } +} + +func TestSchemaCreationStoppedPartway(t *testing.T) { + t.Parallel() + + // A first scan stopped while creating the schema must leave a + // database the next scan accepts. max_page_count(2) leaves room for + // the files table but not its index, so schema creation fails right + // after CREATE TABLE, a point an interrupt could also stop it at. + path := testDBPath(t) + + db, err := openDB(path, scanParams+"&_pragma=max_page_count(2)") + if err != nil { + t.Fatal(err) + } + + err = initSchema(t.Context(), db) + _ = db.Close() + + if err == nil { + t.Fatal("initSchema with no room for the index succeeded") + } + + db, err = openScanDatabase(t.Context(), path) + if err != nil { + t.Fatalf("next scan: %v", err) + } + + defer func() { _ = db.Close() }() + + v, err := userVersion(t.Context(), db) + if err != nil || v != schemaVersion { + t.Fatalf("userVersion = %d, %v; want %d, nil", v, err, schemaVersion) + } +} + func TestOpenReportDatabaseMissing(t *testing.T) { t.Parallel()