Skip to content

Commit a0d0521

Browse files
DeusDataDeusData
committed
fix(store): keep the indexed checkout outside the digested tables
index_coverage_meta is one of the tables the graph digest pins column by column, so the new indexed_checkout_sha column made the digest reject every live store with a schema error: the live-schema compatibility test failed, and test-impact, which digests the graph it builds for HEAD, fell back to running every test for every change. Record the checkout in a table of its own instead: index_checkout (project primary key referencing projects ON DELETE CASCADE, indexed_checkout_sha NOT NULL). A checkout is source identity, which the graph digest deliberately does not bind, and the store already keeps other per-project data (doc_link_*, observed_*) outside the digested tables. index_coverage_meta keeps exactly the shape the digest expects, so the digest needs no change and no version bump, and a new table needs no migration of existing databases. The row is written and cleared in the same coverage transaction, so a failed replace keeps the previous identity, and it is removed with its project. The store API is unchanged: cbm_coverage_meta_t carries indexed_checkout_sha as before. Read-only opens of databases without the table report no identity, as before. store_graph_digest_indexed_checkout_identity_is_not_bound writes a store through the real coverage API with and without a checkout SHA and asserts that it still digests and that the SHA leaves the digest unchanged. Signed-off-by: Martin Vogel <[email protected]>
1 parent 9d0aca2 commit a0d0521

3 files changed

Lines changed: 143 additions & 62 deletions

File tree

‎src/store/store.c‎

Lines changed: 100 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -324,29 +324,22 @@ bool cbm_store_edges_schema_current(cbm_store_t *s) {
324324
return true;
325325
}
326326

327-
/* Best-effort probe for the BT-240 indexed_checkout_sha column. Returns true
328-
* only when the index_coverage_meta table exists AND exposes the column.
329-
* Writable opens run init_schema (which adds it), so false is expected only on
330-
* read-only legacy DBs opened via the query path — those must keep working and
331-
* simply report no indexed-checkout identity. */
332-
static bool coverage_meta_has_checkout_column(cbm_store_t *s) {
327+
/* Best-effort probe for the BT-240 index_checkout table. Writable opens run
328+
* init_schema (which creates it), so false is expected only on read-only
329+
* legacy DBs opened via the query path — those must keep working and simply
330+
* report no indexed-checkout identity. */
331+
static bool index_checkout_table_exists(cbm_store_t *s) {
333332
if (!s || !s->db) {
334333
return false;
335334
}
336335
sqlite3_stmt *stmt = NULL;
337-
if (sqlite3_prepare_v2(s->db, "PRAGMA table_info(index_coverage_meta);", CBM_NOT_FOUND, &stmt,
338-
NULL) != SQLITE_OK) {
336+
if (sqlite3_prepare_v2(s->db,
337+
"SELECT 1 FROM sqlite_master WHERE type = 'table' "
338+
"AND name = 'index_checkout';",
339+
CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) {
339340
return false;
340341
}
341-
bool has = false;
342-
int rc;
343-
while ((rc = sqlite3_step(stmt)) == SQLITE_ROW) {
344-
const char *name = (const char *)sqlite3_column_text(stmt, SKIP_ONE);
345-
if (name && strcmp(name, "indexed_checkout_sha") == 0) {
346-
has = true;
347-
break;
348-
}
349-
}
342+
bool has = sqlite3_step(stmt) == SQLITE_ROW;
350343
sqlite3_finalize(stmt);
351344
return has;
352345
}
@@ -490,25 +483,22 @@ static int init_schema(cbm_store_t *s) {
490483
" ignored_files_stored INTEGER NOT NULL DEFAULT 0,"
491484
" ignored_files_total INTEGER NOT NULL DEFAULT 0,"
492485
" coverage_version INTEGER NOT NULL DEFAULT 1,"
493-
" hash_records_complete INTEGER NOT NULL DEFAULT 0,"
494-
" indexed_checkout_sha TEXT"
486+
" hash_records_complete INTEGER NOT NULL DEFAULT 0"
487+
");"
488+
/* BT-240: the checkout an index was built from. A table of its own,
489+
* not an index_coverage_meta column: it is source identity, which the
490+
* pinned graph-digest tables deliberately do not bind, and a new
491+
* table needs no migration of existing databases. */
492+
"CREATE TABLE IF NOT EXISTS index_checkout ("
493+
" project TEXT PRIMARY KEY REFERENCES projects(name) ON DELETE CASCADE,"
494+
" indexed_checkout_sha TEXT NOT NULL"
495495
");";
496496

497497
int rc = exec_sql(s, ddl);
498498
if (rc != CBM_STORE_OK) {
499499
return rc;
500500
}
501501

502-
/* BT-240 migration: CREATE TABLE IF NOT EXISTS cannot add a column to an
503-
* existing legacy index_coverage_meta, so add it idempotently for any
504-
* writable DB that predates it. Read-only query opens skip init_schema
505-
* entirely and meta_get probes the column separately. */
506-
if (!coverage_meta_has_checkout_column(s) &&
507-
exec_sql(s, "ALTER TABLE index_coverage_meta ADD COLUMN indexed_checkout_sha TEXT;") !=
508-
CBM_STORE_OK) {
509-
return CBM_STORE_ERR;
510-
}
511-
512502
/* Schema-compat probe (#768): DBs created before the local_name_gen
513503
* discriminator still enforce UNIQUE(source_id,target_id,type) and lack
514504
* the column — the widened upsert in cbm_store_insert_edge can neither
@@ -3065,6 +3055,7 @@ int cbm_store_delete_project(cbm_store_t *s, const char *name) {
30653055
static const char *cleanup_sql[] = {
30663056
"DELETE FROM index_coverage WHERE project = ?1;",
30673057
"DELETE FROM index_coverage_meta WHERE project = ?1;",
3058+
"DELETE FROM index_checkout WHERE project = ?1;",
30683059
"DELETE FROM projects WHERE name = ?1 || '::missed';",
30693060
};
30703061
/* doc_link_unresolved exists only in databases published by a build that
@@ -4773,6 +4764,34 @@ static int cov_rebuild_shadow_graph(cbm_store_t *s, const char *project) {
47734764
return CBM_STORE_OK;
47744765
}
47754766

4767+
/* Record (or, with no sha, clear) the checkout a project's index was built
4768+
* from. Runs inside the caller's coverage transaction, so a failed replace
4769+
* keeps the previous identity. */
4770+
static int index_checkout_write(cbm_store_t *s, const char *project, const char *sha) {
4771+
bool record = sha && sha[0];
4772+
sqlite3_stmt *stmt = NULL;
4773+
if (sqlite3_prepare_v2(s->db,
4774+
record ? "INSERT INTO index_checkout (project, indexed_checkout_sha) "
4775+
"VALUES (?1, ?2) ON CONFLICT(project) DO UPDATE SET "
4776+
"indexed_checkout_sha=?2;"
4777+
: "DELETE FROM index_checkout WHERE project = ?1;",
4778+
CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) {
4779+
store_set_error_sqlite(s, "index checkout write prepare");
4780+
return CBM_STORE_ERR;
4781+
}
4782+
bind_text(stmt, SKIP_ONE, project);
4783+
if (record) {
4784+
bind_text(stmt, ST_COL_2, sha);
4785+
}
4786+
int rc = sqlite3_step(stmt);
4787+
sqlite3_finalize(stmt);
4788+
if (rc != SQLITE_DONE) {
4789+
store_set_error_sqlite(s, "index checkout write");
4790+
return CBM_STORE_ERR;
4791+
}
4792+
return CBM_STORE_OK;
4793+
}
4794+
47764795
int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project,
47774796
const cbm_coverage_row_t *rows, int count,
47784797
const cbm_coverage_meta_t *meta) {
@@ -4894,12 +4913,11 @@ int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project,
48944913
"INSERT INTO index_coverage_meta "
48954914
"(project, generation, index_mode, recorded_at, recording_status, "
48964915
" ignored_files_stored, ignored_files_total, coverage_version, "
4897-
" hash_records_complete, indexed_checkout_sha) "
4898-
"VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10) "
4916+
" hash_records_complete) "
4917+
"VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9) "
48994918
"ON CONFLICT(project) DO UPDATE SET generation=?2, index_mode=?3, "
49004919
"recorded_at=?4, recording_status=?5, ignored_files_stored=?6, "
4901-
"ignored_files_total=?7, coverage_version=?8, hash_records_complete=?9, "
4902-
"indexed_checkout_sha=?10;",
4920+
"ignored_files_total=?7, coverage_version=?8, hash_records_complete=?9;",
49034921
CBM_NOT_FOUND, &up_meta, NULL) != SQLITE_OK) {
49044922
store_set_error_sqlite(s, "coverage meta upsert prepare");
49054923
(void)exec_sql(s, "ROLLBACK;");
@@ -4914,18 +4932,17 @@ int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project,
49144932
sqlite3_bind_int(up_meta, 7, ignored_total);
49154933
sqlite3_bind_int(up_meta, 8, coverage_version);
49164934
sqlite3_bind_int(up_meta, 9, meta->hash_records_complete ? 1 : 0);
4917-
if (meta->indexed_checkout_sha && meta->indexed_checkout_sha[0]) {
4918-
bind_text(up_meta, 10, meta->indexed_checkout_sha);
4919-
} else {
4920-
sqlite3_bind_null(up_meta, 10);
4921-
}
49224935
int meta_rc = sqlite3_step(up_meta);
49234936
sqlite3_finalize(up_meta);
49244937
if (meta_rc != SQLITE_DONE) {
49254938
store_set_error_sqlite(s, "coverage meta upsert");
49264939
(void)exec_sql(s, "ROLLBACK;");
49274940
return CBM_STORE_ERR;
49284941
}
4942+
if (index_checkout_write(s, project, meta->indexed_checkout_sha) != CBM_STORE_OK) {
4943+
(void)exec_sql(s, "ROLLBACK;");
4944+
return CBM_STORE_ERR;
4945+
}
49294946
} else {
49304947
sqlite3_stmt *del_meta = NULL;
49314948
if (sqlite3_prepare_v2(s->db, "DELETE FROM index_coverage_meta WHERE project = ?1;",
@@ -4942,6 +4959,10 @@ int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project,
49424959
(void)exec_sql(s, "ROLLBACK;");
49434960
return CBM_STORE_ERR;
49444961
}
4962+
if (index_checkout_write(s, project, NULL) != CBM_STORE_OK) {
4963+
(void)exec_sql(s, "ROLLBACK;");
4964+
return CBM_STORE_ERR;
4965+
}
49454966
}
49464967

49474968
/* Rebuild the derived miss-graph view from the now-authoritative table
@@ -5076,6 +5097,41 @@ void cbm_store_coverage_meta_clear(cbm_coverage_meta_t *meta) {
50765097
memset(meta, 0, sizeof(*meta));
50775098
}
50785099

5100+
/* Fill out->indexed_checkout_sha from index_checkout. A legacy read-only DB
5101+
* (query opens skip init_schema) may lack the table: no identity, not an
5102+
* error. On failure out is cleared. */
5103+
static int index_checkout_read(cbm_store_t *s, const char *project, cbm_coverage_meta_t *out) {
5104+
if (!index_checkout_table_exists(s)) {
5105+
return CBM_STORE_OK;
5106+
}
5107+
sqlite3_stmt *stmt = NULL;
5108+
if (sqlite3_prepare_v2(s->db,
5109+
"SELECT indexed_checkout_sha FROM index_checkout WHERE project = ?1;",
5110+
CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) {
5111+
store_set_error_sqlite(s, "index checkout read prepare");
5112+
cbm_store_coverage_meta_clear(out);
5113+
return CBM_STORE_ERR;
5114+
}
5115+
bind_text(stmt, SKIP_ONE, project);
5116+
int rc = sqlite3_step(stmt);
5117+
if (rc == SQLITE_ROW) {
5118+
const char *sha = (const char *)sqlite3_column_text(stmt, 0);
5119+
if (sha && sha[0]) {
5120+
out->indexed_checkout_sha = cbm_mem_strdup(CBM_MEM_CLASS_STORE, sha);
5121+
rc = out->indexed_checkout_sha ? SQLITE_DONE : SQLITE_NOMEM;
5122+
} else {
5123+
rc = SQLITE_DONE;
5124+
}
5125+
}
5126+
sqlite3_finalize(stmt);
5127+
if (rc != SQLITE_DONE) {
5128+
store_set_error_sqlite(s, "index checkout read");
5129+
cbm_store_coverage_meta_clear(out);
5130+
return CBM_STORE_ERR;
5131+
}
5132+
return CBM_STORE_OK;
5133+
}
5134+
50795135
int cbm_store_coverage_meta_get(cbm_store_t *s, const char *project, cbm_coverage_meta_t *out) {
50805136
if (!out) {
50815137
return CBM_STORE_ERR;
@@ -5085,21 +5141,11 @@ int cbm_store_coverage_meta_get(cbm_store_t *s, const char *project, cbm_coverag
50855141
return CBM_STORE_ERR;
50865142
}
50875143
sqlite3_stmt *stmt = NULL;
5088-
/* Legacy read-only DBs (query opens skip init_schema) lack the
5089-
* indexed_checkout_sha column — probe and fall back so they still read and
5090-
* report no identity instead of erroring. */
5091-
static const char SQL_FULL[] =
5092-
"SELECT project, generation, index_mode, recorded_at, recording_status, "
5093-
"ignored_files_stored, ignored_files_total, coverage_version, "
5094-
"hash_records_complete, indexed_checkout_sha FROM index_coverage_meta "
5095-
"WHERE project = ?1;";
5096-
static const char SQL_LEGACY[] =
5097-
"SELECT project, generation, index_mode, recorded_at, recording_status, "
5098-
"ignored_files_stored, ignored_files_total, coverage_version, "
5099-
"hash_records_complete FROM index_coverage_meta WHERE project = ?1;";
5100-
bool has_checkout_column = coverage_meta_has_checkout_column(s);
5101-
if (sqlite3_prepare_v2(s->db, has_checkout_column ? SQL_FULL : SQL_LEGACY, CBM_NOT_FOUND, &stmt,
5102-
NULL) != SQLITE_OK) {
5144+
if (sqlite3_prepare_v2(s->db,
5145+
"SELECT project, generation, index_mode, recorded_at, recording_status, "
5146+
"ignored_files_stored, ignored_files_total, coverage_version, "
5147+
"hash_records_complete FROM index_coverage_meta WHERE project = ?1;",
5148+
CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) {
51035149
store_set_error_sqlite(s, "coverage meta get prepare");
51045150
return CBM_STORE_ERR;
51055151
}
@@ -5115,18 +5161,13 @@ int cbm_store_coverage_meta_get(cbm_store_t *s, const char *project, cbm_coverag
51155161
out->ignored_files_total = sqlite3_column_int(stmt, 6);
51165162
out->coverage_version = sqlite3_column_int(stmt, 7);
51175163
out->hash_records_complete = sqlite3_column_int(stmt, 8) != 0;
5118-
if (has_checkout_column) {
5119-
const char *sha = (const char *)sqlite3_column_text(stmt, 9);
5120-
out->indexed_checkout_sha =
5121-
sha && sha[0] ? cbm_mem_strdup(CBM_MEM_CLASS_STORE, sha) : NULL;
5122-
}
51235164
sqlite3_finalize(stmt);
51245165
if (!out->project || !out->generation || !out->index_mode || !out->recorded_at ||
51255166
!out->recording_status) {
51265167
cbm_store_coverage_meta_clear(out);
51275168
return CBM_STORE_ERR;
51285169
}
5129-
return CBM_STORE_OK;
5170+
return index_checkout_read(s, project, out);
51305171
}
51315172
sqlite3_finalize(stmt);
51325173
if (rc != SQLITE_DONE) {

‎src/store/store.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -791,9 +791,9 @@ typedef struct {
791791
int coverage_version;
792792
bool hash_records_complete;
793793
/* Git checkout identity frozen at the successful staged-generation
794-
* boundary (BT-240). NULL/"" when the run had no git context (non-git
795-
* repo) or the DB predates the column. Owned by the row: meta_get
796-
* allocates it, meta_clear frees it. */
794+
* boundary (BT-240), stored in the index_checkout table. NULL/"" when the
795+
* run had no git context (non-git repo) or the DB predates that table.
796+
* Owned by the row: meta_get allocates it, meta_clear frees it. */
797797
const char *indexed_checkout_sha;
798798
} cbm_coverage_meta_t;
799799

‎tests/test_store_graph_digest.c‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,45 @@ TEST(store_graph_digest_concurrent_isolation_with_held_callback_gates) {
594594
cbm_store_close(a);cbm_store_close(b);PASS();
595595
}
596596

597+
/* BT-240 (#1561): the indexed checkout is source identity, which the digest
598+
* never binds, so it lives outside the digested tables. A store written through
599+
* the real coverage API, with and without a checkout SHA, keeps passing the
600+
* live-schema check (test-impact falls back to running everything otherwise),
601+
* and recording or changing the SHA leaves the digest unchanged. */
602+
TEST(store_graph_digest_indexed_checkout_identity_is_not_bound) {
603+
cbm_store_t *s = cbm_store_open_memory();
604+
ASSERT_NOT_NULL(s);
605+
ASSERT_EQ(cbm_store_upsert_project(s, "p", "/live"), CBM_STORE_OK);
606+
cbm_coverage_meta_t meta = {.generation = "g",
607+
.index_mode = "full",
608+
.recorded_at = "t",
609+
.recording_status = "complete",
610+
.coverage_version = 1,
611+
.hash_records_complete = true};
612+
cbm_store_graph_digest_t none = {0}, first = {0}, second = {0};
613+
ASSERT_EQ(cbm_store_coverage_replace_ex(s, "p", NULL, 0, &meta), CBM_STORE_OK);
614+
bool none_ok = gd_p(s, &none);
615+
meta.indexed_checkout_sha = "0123456789abcdef0123456789abcdef01234567";
616+
ASSERT_EQ(cbm_store_coverage_replace_ex(s, "p", NULL, 0, &meta), CBM_STORE_OK);
617+
bool first_ok = gd_p(s, &first);
618+
meta.indexed_checkout_sha = "89abcdef0123456789abcdef0123456789abcdef";
619+
ASSERT_EQ(cbm_store_coverage_replace_ex(s, "p", NULL, 0, &meta), CBM_STORE_OK);
620+
bool second_ok = gd_p(s, &second);
621+
cbm_coverage_meta_t got = {0};
622+
int got_rc = cbm_store_coverage_meta_get(s, "p", &got);
623+
bool sha_kept = got_rc == CBM_STORE_OK && got.indexed_checkout_sha &&
624+
strcmp(got.indexed_checkout_sha, "89abcdef0123456789abcdef0123456789abcdef") == 0;
625+
cbm_store_coverage_meta_clear(&got);
626+
cbm_store_close(s);
627+
ASSERT_TRUE(none_ok);
628+
ASSERT_TRUE(first_ok);
629+
ASSERT_TRUE(second_ok);
630+
ASSERT_TRUE(sha_kept);
631+
ASSERT_TRUE(gd_equal(&none, &first));
632+
ASSERT_TRUE(gd_equal(&first, &second));
633+
PASS();
634+
}
635+
597636
SUITE(store_graph_digest) {
598637
RUN_TEST(store_graph_digest_independent_canonical_golden);
599638
RUN_TEST(store_graph_digest_logical_order_and_excluded_storage_are_stable);
@@ -607,4 +646,5 @@ SUITE(store_graph_digest) {
607646
RUN_TEST(store_graph_digest_cancellation_large_blob_never_publishes_prefix);
608647
RUN_TEST(store_graph_digest_sql_errors_cleanup_no_mutation_and_close_discard);
609648
RUN_TEST(store_graph_digest_concurrent_isolation_with_held_callback_gates);
649+
RUN_TEST(store_graph_digest_indexed_checkout_identity_is_not_bound);
610650
}

0 commit comments

Comments
 (0)