Repository navigation
Conversation
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 was referenced Oct 23, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A connection keeps an IVF column's trained state from the first time it reads it, so it misses another connection's
compute-centroidsorclear-centroids, and keeps one that was rolled back.ivf_is_trained()readivf_trained_Nfrom_infoonce per connection and kept it inivfTrainedCache, 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 owncompute-centroidswas rolled back, and failed every insert and query with "SQL logic error": with no centroids to list, the cell scan's SQL readscentroid_id IN (,-1). The state is now read from_infoeach 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_clearsandtest_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.