fix(store-sqlite): #229 chunks_fts 삭제를 전체 스캔에서 rowid 조회로 (V016)
`chunk_id` 는 `chunks_fts` 에서 UNINDEXED 다. 그런데 V002 이래 삭제 트리거가 그 컬럼으로 행을 찾았다 (`DELETE FROM chunks_fts WHERE chunk_id = old.chunk_id`). FTS5 는 UNINDEXED 컬럼에 색인을 만들지 않으니 이 조건을 만족할 색인이 없고, 삭제가 색인 전체 스캔으로 떨어진다. chunk 한 건 삭제가 O(색인 전체) 였다. `chunks` 에서 DELETE 가 나가는 모든 경로가 이 비용을 냈다 — sweep_deleted_files, reset --orphans-only, 그리고 파일이 수정될 때마다 도는 purge_orphan_at_workspace_path. 즉 정상적인 증분 재색인이 코퍼스가 커질수록 느려지는 형태였다. V016 이 chunks_fts 를 drop 후 재생성하면서 rowid 를 chunks.rowid 와 맞추고, 세 트리거의 행 지정을 rowid 로 바꾼다. FTS5 는 rowid 로 B-tree 조회를 하므로 O(log n) 이 된다. 컬럼 구성·tokenizer 는 그대로라 검색 경로는 한 줄도 안 바뀐다. 실제 KB 사본 (문서 28,427건 / chunk 600,808건) 에서 문서 200건 삭제: 현행 (chunk_id 로 DELETE) 1590.1초 V016 (rowid 로 DELETE) 2.0초 약 800배. 삭제 후 남은 chunks 와 chunks_fts 행 수가 양쪽 다 595,741 로 같다. 마이그레이션 자체는 60만 chunk 기준 32초이고 재색인은 필요 없다. 검색 결과는 불변이다. '한국'(15,977) / 'database'(1,067) / '서울 지하철'(230) / 'kebab'(3) 네 질의의 상위 20건을 chunk_id·bm25 점수·snippet 까지 해시로 비교했고 전후가 동일했다. 그래서 corpus_revision 을 올리지 않는다 — 어휘 검색 정렬이 `ORDER BY score, f.chunk_id` 라 rowid 와 무관하므로 미결 pagination cursor 를 무효화할 이유가 없다. tokenizer 가 바뀐 V009 와는 다른 경우다. 이슈가 제안한 external-content(`content='chunks'`) 는 택하지 않았다. V009 트리거가 색인하는 값이 `tokenized_korean_text || ' ' || text` 라 chunks 의 어느 컬럼과도 일치하지 않아, generated column 신설 + FTS 테이블에서 chunk_id/doc_id 제거 + 검색 경로의 rowid join 전환이 딸려온다. 삭제 비용은 rowid 정렬만으로 같은 복잡도로 내려가므로 본문 그림자 (chunks_fts_content, 실측 550 MB) 회수는 별 건으로 남긴다. rebuild_chunks_fts 도 같이 고쳤다. rowid 를 명시하지 않으면 FTS5 가 자기 번호를 매겨 정렬이 깨지고 그 뒤의 모든 삭제가 조용히 아무것도 안 하게 된다. 이 함수에는 별개의 잠복 결함도 있었다 — V009 가 색인하는 한국어 형태소 접두를 빠뜨리고 raw text 만 넣고 있어서, 재구축을 돌리면 2자 한국어 질의가 다음 재색인 때까지 안 맞았다. 트리거와 같은 CASE 로 맞췄다. 전제: chunks 는 chunk_id TEXT PRIMARY KEY 라 INTEGER PRIMARY KEY 가 없고, SQLite 의 VACUUM 은 그런 테이블의 rowid 를 다시 매길 수 있다. kebab 은 VACUUM 을 실행하지 않으며(코드베이스 전체에 없음) 사용자가 직접 돌렸다면 rebuild_chunks_fts 가 복구 경로다. external-content 도 같은 전제를 깔고 있어 이 위험은 선택지 간 차이가 아니다. design §5.5 verbatim block 을 rowid 트리거로 갱신하고 CI diff-check 를 V009 에서 V016 으로 재조준했다 (V007 → V009 때와 같은 방식). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017c9JwQq8ZkGvYjpKXMiDhF
This commit is contained in:
@@ -369,20 +369,20 @@ fn extract_design_5_5_fts_block() -> String {
|
||||
fts_slice[..last_end + "END;".len()].to_string()
|
||||
}
|
||||
|
||||
/// Extract the §5.5 verbatim block from the V009 migration (V009 replaces
|
||||
/// V007 's trigram tokenizer with unicode61 + CASE expression triggers for
|
||||
/// Korean morphological tokenization — V007 stays in place for historical
|
||||
/// cold-upgrade replay but V009 is now the source of truth),
|
||||
/// between the `── §5.5 verbatim block ──` anchor markers V009 carries.
|
||||
/// Extract the §5.5 verbatim block from the V016 migration (V016 repoints
|
||||
/// the sync triggers from `chunk_id` to `rowid` so deletes are a B-tree
|
||||
/// lookup instead of a full FTS5 scan — V009 stays in place for historical
|
||||
/// cold-upgrade replay but V016 is now the source of truth),
|
||||
/// between the `── §5.5 verbatim block ──` anchor markers V016 carries.
|
||||
fn extract_migration_5_5_verbatim_block() -> String {
|
||||
let migration = include_str!("../../../migrations/V009__fts_korean_morphological.sql");
|
||||
let migration = include_str!("../../../migrations/V016__fts_rowid_delete.sql");
|
||||
// The opening anchor line ends with `── §5.5 verbatim block ─...`.
|
||||
let open_marker = "§5.5 verbatim block";
|
||||
let close_marker = "End §5.5 verbatim block";
|
||||
|
||||
let open_idx = migration
|
||||
.find(open_marker)
|
||||
.expect("V009 must carry the `§5.5 verbatim block` opening anchor");
|
||||
.expect("V016 must carry the `§5.5 verbatim block` opening anchor");
|
||||
let after_open_line = open_idx
|
||||
+ migration[open_idx..]
|
||||
.find('\n')
|
||||
@@ -391,7 +391,7 @@ fn extract_migration_5_5_verbatim_block() -> String {
|
||||
|
||||
let close_idx = migration[after_open_line..]
|
||||
.find(close_marker)
|
||||
.expect("V009 must carry the `End §5.5 verbatim block` closing anchor")
|
||||
.expect("V016 must carry the `End §5.5 verbatim block` closing anchor")
|
||||
+ after_open_line;
|
||||
// Walk back from the close marker to the start of its comment line.
|
||||
let close_line_start = migration[..close_idx].rfind('\n').map_or(0, |n| n + 1);
|
||||
@@ -399,15 +399,14 @@ fn extract_migration_5_5_verbatim_block() -> String {
|
||||
migration[after_open_line..close_line_start].to_string()
|
||||
}
|
||||
|
||||
/// CI diff guard: the §5.5 block in `migrations/V009__fts_korean_morphological.sql`
|
||||
/// must match the design doc verbatim (whitespace-normalized). V009
|
||||
/// replaced V007 's trigram tokenizer with unicode61 + CASE expression
|
||||
/// triggers for Korean morphological tokenization (2026-05-28).
|
||||
/// V007 stays in place for historical replay of cold-upgrade paths
|
||||
/// but is no longer compared against the design doc — V009 is now
|
||||
/// the source of truth.
|
||||
/// CI diff guard: the §5.5 block in `migrations/V016__fts_rowid_delete.sql`
|
||||
/// must match the design doc verbatim (whitespace-normalized). V016
|
||||
/// repointed the sync triggers from `chunk_id` (UNINDEXED, so a full FTS5
|
||||
/// scan per delete) to `rowid` (2026-08-16, issue #229). V007 and V009 stay
|
||||
/// in place for historical replay of cold-upgrade paths but are no longer
|
||||
/// compared against the design doc — V016 is now the source of truth.
|
||||
#[test]
|
||||
fn fts_v009_matches_design_section_5_5_verbatim() {
|
||||
fn fts_v016_matches_design_section_5_5_verbatim() {
|
||||
let design = extract_design_5_5_fts_block();
|
||||
let migration_block = extract_migration_5_5_verbatim_block();
|
||||
|
||||
@@ -430,7 +429,7 @@ fn fts_v009_matches_design_section_5_5_verbatim() {
|
||||
let migration_n = normalize_ws(&migration_block);
|
||||
assert_eq!(
|
||||
design_n, migration_n,
|
||||
"V009__fts_korean_morphological.sql §5.5 block must match design doc §5.5 verbatim \
|
||||
"V016__fts_rowid_delete.sql §5.5 block must match design doc §5.5 verbatim \
|
||||
(whitespace-normalized). If you intentionally changed one, \
|
||||
update the other in the same commit."
|
||||
);
|
||||
@@ -656,3 +655,172 @@ fn fts_v009_english_whole_token_only() {
|
||||
"V009 unicode61: whole-token 'tokenizer' must hit"
|
||||
);
|
||||
}
|
||||
|
||||
// ── 8. V016 rowid-addressed deletes (issue #229) ──────────────────────
|
||||
|
||||
/// The shadow's rowid must equal the source row's rowid. Every other
|
||||
/// V016 property rests on this: the delete trigger addresses rows by
|
||||
/// rowid, so a drifted rowid makes deletes silently miss.
|
||||
#[test]
|
||||
fn fts_v016_shadow_rowid_mirrors_chunks_rowid() {
|
||||
let env = common::TestEnv::new();
|
||||
let store = SqliteStore::open(&env.config().storage).unwrap();
|
||||
store.run_migrations().unwrap();
|
||||
|
||||
let conn = raw_conn_no_fk(&env);
|
||||
for i in 0..5u8 {
|
||||
insert_chunk(
|
||||
&conn,
|
||||
&format!("{i:032}"),
|
||||
&"d".repeat(32),
|
||||
"[]",
|
||||
&format!("body {i}"),
|
||||
);
|
||||
}
|
||||
|
||||
let drifted: i64 = conn
|
||||
.query_row(
|
||||
"SELECT COUNT(*) FROM chunks c
|
||||
LEFT JOIN chunks_fts f ON f.rowid = c.rowid
|
||||
WHERE f.rowid IS NULL OR f.chunk_id != c.chunk_id",
|
||||
[],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.expect("join chunks to its shadow by rowid");
|
||||
assert_eq!(
|
||||
drifted, 0,
|
||||
"every chunks row must have a chunks_fts row at the same rowid carrying the same chunk_id"
|
||||
);
|
||||
}
|
||||
|
||||
/// Deleting one chunk must remove exactly that chunk's shadow row.
|
||||
/// Under the pre-V016 `WHERE chunk_id = ?` trigger this also passed —
|
||||
/// it is the correctness floor the rowid switch must not drop.
|
||||
#[test]
|
||||
fn fts_v016_delete_removes_only_the_deleted_row() {
|
||||
let env = common::TestEnv::new();
|
||||
let store = SqliteStore::open(&env.config().storage).unwrap();
|
||||
store.run_migrations().unwrap();
|
||||
|
||||
let conn = raw_conn_no_fk(&env);
|
||||
for i in 0..4u8 {
|
||||
insert_chunk(
|
||||
&conn,
|
||||
&format!("{i:032}"),
|
||||
&"d".repeat(32),
|
||||
"[]",
|
||||
&format!("alpha{i} shared"),
|
||||
);
|
||||
}
|
||||
assert_eq!(count(&conn, "chunks_fts"), 4);
|
||||
|
||||
conn.execute(
|
||||
"DELETE FROM chunks WHERE chunk_id = ?",
|
||||
rusqlite::params![format!("{:032}", 2u8)],
|
||||
)
|
||||
.expect("delete one chunk");
|
||||
|
||||
assert_eq!(count(&conn, "chunks"), 3);
|
||||
assert_eq!(count(&conn, "chunks_fts"), 3);
|
||||
assert_eq!(
|
||||
count_match(&conn, "alpha2"),
|
||||
0,
|
||||
"the deleted chunk must leave no shadow row behind"
|
||||
);
|
||||
for i in [0u8, 1, 3] {
|
||||
assert_eq!(
|
||||
count_match(&conn, &format!("alpha{i}")),
|
||||
1,
|
||||
"sibling chunk alpha{i} must survive its neighbour's delete"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The point of V016: the delete must be a rowid lookup, not a scan.
|
||||
/// SQLite reports a virtual table's accepted constraints in the plan's
|
||||
/// `INDEX n:str` field — FTS5 puts `=` there when it takes the rowid
|
||||
/// equality itself. Addressing by `chunk_id` leaves that field empty
|
||||
/// because `chunk_id` is UNINDEXED, and the delete degrades to a full
|
||||
/// index scan (measured at 1590s for 200 documents over 600k chunks,
|
||||
/// against 0.73s for this plan).
|
||||
#[test]
|
||||
fn fts_v016_delete_plan_uses_rowid_not_a_scan() {
|
||||
let env = common::TestEnv::new();
|
||||
let store = SqliteStore::open(&env.config().storage).unwrap();
|
||||
store.run_migrations().unwrap();
|
||||
|
||||
let conn = raw_conn_no_fk(&env);
|
||||
let plan_for = |sql: &str| -> String {
|
||||
let mut stmt = conn
|
||||
.prepare(&format!("EXPLAIN QUERY PLAN {sql}"))
|
||||
.expect("prepare plan");
|
||||
stmt.query_map([], |r| r.get::<_, String>(3))
|
||||
.expect("run plan")
|
||||
.collect::<rusqlite::Result<Vec<_>>>()
|
||||
.expect("collect plan")
|
||||
.join(" | ")
|
||||
};
|
||||
|
||||
let by_rowid = plan_for("DELETE FROM chunks_fts WHERE rowid = 1");
|
||||
let by_chunk_id = plan_for("DELETE FROM chunks_fts WHERE chunk_id = 'x'");
|
||||
|
||||
assert!(
|
||||
by_rowid.contains(":="),
|
||||
"rowid delete must hand FTS5 the equality constraint; got {by_rowid:?}"
|
||||
);
|
||||
assert!(
|
||||
!by_chunk_id.contains(":="),
|
||||
"chunk_id is UNINDEXED so FTS5 cannot take the constraint — if this ever \
|
||||
stops holding, the premise of issue #229 changed; got {by_chunk_id:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// `rebuild_chunks_fts` is the escape hatch for a drifted shadow, so it
|
||||
/// has to reproduce what the triggers write — both the rowid alignment
|
||||
/// (or every later delete becomes a no-op) and the Korean morpheme
|
||||
/// prefix (or 2-character Korean queries stop matching until the next
|
||||
/// re-ingest).
|
||||
#[test]
|
||||
fn fts_v016_rebuild_preserves_rowid_and_korean_morphemes() {
|
||||
let env = common::TestEnv::new();
|
||||
let store = SqliteStore::open(&env.config().storage).unwrap();
|
||||
store.run_migrations().unwrap();
|
||||
|
||||
let conn = raw_conn_no_fk(&env);
|
||||
let text = "한국의 수도는 서울이다";
|
||||
let tokenized = tokenize_korean_morphological(text);
|
||||
conn.execute(
|
||||
"INSERT INTO chunks (
|
||||
chunk_id, doc_id, text, heading_path_json, section_label,
|
||||
source_spans_json, token_estimate, chunker_version,
|
||||
policy_hash, block_ids_json, created_at, tokenized_korean_text
|
||||
) VALUES (?, ?, ?, '[]', NULL, '[]', 0, 'v1', 'h', '[]', '2024-01-01T00:00:00Z', ?)",
|
||||
rusqlite::params![&"k".repeat(32), &"d".repeat(32), text, tokenized],
|
||||
)
|
||||
.expect("insert chunk with tokenized_korean_text");
|
||||
|
||||
rebuild_chunks_fts(&conn).expect("rebuild");
|
||||
|
||||
let drifted: i64 = conn
|
||||
.query_row(
|
||||
"SELECT COUNT(*) FROM chunks c
|
||||
LEFT JOIN chunks_fts f ON f.rowid = c.rowid
|
||||
WHERE f.rowid IS NULL",
|
||||
[],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.expect("join after rebuild");
|
||||
assert_eq!(drifted, 0, "rebuild must keep the shadow rowid-aligned");
|
||||
assert!(
|
||||
count_match(&conn, "한국") >= 1,
|
||||
"rebuild must re-index the morpheme column, not just the raw text"
|
||||
);
|
||||
|
||||
// And the rebuilt rows must still be deletable through the trigger.
|
||||
conn.execute("DELETE FROM chunks", []).expect("delete all");
|
||||
assert_eq!(
|
||||
count(&conn, "chunks_fts"),
|
||||
0,
|
||||
"deletes after a rebuild must still find their shadow rows"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user