Repository navigation
Conversation
xBestIndex hands a KNN query's `rowid in (...)` constraint to xFilter and tells SQLite to omit it, but vec0Filter_knn_diskann() read only the MATCH and k constraints from idxStr, so a filtered query on a DiskANN column returned the unfiltered k nearest rows. A rowid filter now ranks the rows it names by their exact distances, read from _vectorsNN, which holds the buffered rows as well as the graph's, and skips a rowid or id the table does not have; the graph search and the buffer scan run only without one.
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 KNN query on a DiskANN column ignores a
rowid in (...)filter and returns the unfiltered k nearest rows.vec0BestIndex()passes the filter to xFilter as ansqlite3_vtab_in()list and setsomit, so SQLite does not check it again, butvec0Filter_knn_diskann()reads only the MATCH and k entries of idxStr. A rowid filter now ranks the rows it names by their exact distances, read from_vectorsNN, which holds the buffered rows as well as the graph's, and keeps the k nearest. A rowid or id the table does not have is skipped, as SQL'sindoes (a flat column returns no rows at all when a text-id filter names one). The graph search and the buffer scan run only without a filter, since the graph search reaches only the rows near the query, which the filter may not name.Tests in test-diskann.py:
test_diskann_knn_rowid_incompares rowids and distances with a flat column built from the same vectors, for filters naming graph rows, buffered rows, a duplicated rowid and a missing one, on a table with and without a buffer;test_diskann_knn_id_in_text_primary_keyfilters a text primary key with an id the table does not have. All three fail on main and pass here. The Python suite passes (242 passed, 66 skipped), under ASan and UBSan as well, and the build's warnings are main's.The distance half of the same gap is #316, which applies
distanceconstraints in every backend. The two merge cleanly, and together a distance constraint applies to the k rows the filter ranked, as it does to the graph's, sorowid in (...) and distance > xcan return fewer than k of the named rows where a flat column returns k. This also merges cleanly with every open PR that touches the DiskANN code, #311, #313, #342, #344, #345, #346 and #347 included.Origin. Found in the DiskANN port to photostructure/sqlite-vec, photostructure#11 (cd53413), whose
diskann_knn()takes the rowid filter and distance constraints of the rescore index's KNN; the change and its tests are written here for asg017's main. Skipping an id the table does not have is the rule mceachen's photostructure/sqlite-vec@07eb685 gives that fork's flat filter, and mceachen's photostructure/sqlite-vec@1eb6348 applies a one-valuerowid = ?orrowid in (?)there before the k, which a DiskANN column here still applies after it, as a flat column does. No open or closed PR or issue here makes any of these changes.