Skip to content

Read the IVF trained state from _info at each use - #351

Open
MayCXC wants to merge 1 commit into
asg017:mainfrom
MayCXC:ivf-trained-state
Open

MayCXC wants to merge 1 commit into
asg017:mainfrom
MayCXC:ivf-trained-state

Conversation

@MayCXC

@MayCXC MayCXC commented Oct 23, 2026 •

Copy link
Copy Markdown

A connection keeps an IVF column's trained state from the first time it reads it, so it misses another connection's compute-centroids or clear-centroids, and keeps one that was rolled back.

-- connection A, on a database file
create virtual table t using vec0(v float[4] indexed by ivf(nlist=2, nprobe=2));
-- 20 inserts
select rowid from t where v match :q and k = 3;   -- 3 rows
-- connection B
insert into t(t) values ('compute-centroids');
-- connection A
select rowid from t where v match :q and k = 3;   -- no rows

ivf_is_trained() read ivf_trained_N from _info once per connection and kept it in ivfTrainedCache, which the commands also set as they ran. A connection that had read "untrained" went on scanning only the unassigned cells after another connection moved every row into trained ones, and found nothing. One that had read "trained" went on looking for centroids after another connection cleared them, or after its own compute-centroids was rolled back, and failed every insert and query with "SQL logic error": with no centroids to list, the cell scan's SQL reads centroid_id IN (,-1). The state is now read from _info each time, so it is always that of the transaction reading it. The read costs an insert about a microsecond (20,000 inserts of 64 dimensions: 15.2 to 16.7 µs each untrained, 13.0 to 13.6 µs trained), and KNN query times are unchanged.

Tests in test-ivf.py: test_ivf_knn_after_another_connection_trains, test_ivf_insert_after_another_connection_clears and test_ivf_trained_state_after_rollback. All three fail on main (no rows, then "SQL logic error" twice) and pass here. The Python suite passes (with -DSQLITE_VEC_EXPERIMENTAL_IVF_ENABLE=1, 304 passed, 4 skipped; without it, 239 passed, 69 skipped), under ASan and UBSan as well, and both builds' warnings are main's.

This merges cleanly with every open PR that touches the IVF code except #313, which rewrites the comment on the struct member that follows the removed ivfTrainedCache; the conflict is that one line, and its resolution keeps #313's.

Origin. Found in the IVF port to photostructure/sqlite-vec, photostructure#12 (65817f4), which lists it among the port's differences from upstream; the change and its tests are written here for asg017's main. No open or closed PR or issue here makes the same change.

ivf_is_trained() kept the trained state per connection once it had read
it, and compute-centroids, set-centroid and clear-centroids set that
cache as they ran. Another connection's command, or a rolled-back one,
left it wrong: a connection that had queried before another trained the
index scanned only the emptied unassigned cells and found no rows, and
one that had trained it, then saw another connection clear it or rolled
its own training back, failed every insert and query with "SQL logic
error". The state is now read from _info each time, so it is always
that of the transaction reading it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant