Skip to content

fix: Rebuild the SQLite vector and keyword search index on every query - #6923

Open
MohammadHijjawi97 wants to merge 1 commit into
feast-dev:masterfrom
MohammadHijjawi97:fix/sqlite-search-index-per-query
Open

MohammadHijjawi97 wants to merge 1 commit into
feast-dev:masterfrom
MohammadHijjawi97:fix/sqlite-search-index-per-query

Conversation

@MohammadHijjawi97

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

The SQLite online store answers retrieve_online_documents and retrieve_online_documents_v2 (and so the feature server's /search) by copying the feature view's rows into a vec0 (vector) or FTS5 (keyword) table and querying that. The table lives in the online store database and is never cleared, so every search adds another copy of every row:

  • Vector search (vector_enabled: true) works only once. The next retrieve_online_documents_v2 call fails with sqlite3.OperationalError: UNIQUE constraint failed on vec_table primary key, and retrieve_online_documents with table vec_table already exists. Once a write commits the leftover rows, even the first search of a new process fails.
  • Keyword search (text_search_enabled: true) returns fewer documents on every call, because copies of the best matches fill top_k, and it keeps matching text that has since been overwritten.
  • The search also leaves its transaction open, so writes from any other connection (e.g. a feast materialize next to feast serve) fail with database is locked until the searching process writes something itself.

Repro on master, same data and query each time:

keyword search "sentence", top_k=3
  call 1: Title 0, Title 1, Title 2
  call 2: Title 0, Title 1
  call 3: Title 0
  after item 0 is rewritten without "sentence": still returns Title 0

vector search, top_k=3 (Python 3.10, sqlite-vec)
  call 1: doc 0, doc 1, doc 2
  call 2: OperationalError: UNIQUE constraint failed on vec_table primary key

This PR builds the index in the connection's temp schema, drops and refills it for every search, and commits it right away. Every call now returns the same top_k from the current rows, a rewritten document no longer matches its old text, and nothing is left in the online store file or locked. The vector index build is shared by both APIs.

Which issue(s) this PR fixes:

No existing issue.

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Misc

Added two tests to sdk/python/tests/unit/online_store/test_online_retrieval.py; both fail on master and pass with this change:

  • test_sqlite_get_online_documents_v2_search_is_repeatable repeats a keyword search and rewrites a document.
  • test_sqlite_get_online_documents_is_repeatable runs retrieve_online_documents_v2 and retrieve_online_documents twice each (Python 3.10 only, like the existing sqlite-vec test).

All sqlite tests in that file pass on Python 3.10 with SQLite 3.53 (vec0 KNN queries need SQLite >= 3.41 for LIMIT, as on ubuntu-latest), as does test_sqlite_get_online_documents when run directly. I did not run the Milvus tests (milvus-lite has no Windows build). ruff and mypy on sqlite.py are clean.

The SQLite online store answers retrieve_online_documents and
retrieve_online_documents_v2 by copying the feature view's rows into a
vec0 (vector) or FTS5 (keyword) table and querying that table. The table
lived in the online store database and was never cleared, so every
search added another copy of every row:

- Vector search only worked once. Later searches failed with "UNIQUE
  constraint failed on vec_table primary key" (v2) or "table vec_table
  already exists" (v1), and once a write committed the leftover rows,
  even the first search of a new process failed.
- Keyword search returned fewer documents on every call, as copies of
  the best matches filled top_k, and kept matching text that had since
  been overwritten.

The search also left its transaction open, so writes from other
processes failed with "database is locked".

Build the index in the connection's temp schema, drop and refill it for
every search, and commit it right away.

Signed-off-by: Mohammad Hijjawi <mohammad.hijjawi1997@gmail.com>
@MohammadHijjawi97
MohammadHijjawi97 requested a review from a team as a code owner October 1, 2026 15:03
@codecov-commenter

codecov-commenter commented Oct 1, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 57.14286% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.58%. Comparing base (81673d9) to head (a5cf5c0).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
sdk/python/feast/infra/online_stores/sqlite.py 57.14% 6 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6923   +/-   ##
=======================================
  Coverage   48.58%   48.58%           
=======================================
  Files         427      427           
  Lines       53787    53792    +5     
  Branches     7834     7834           
=======================================
+ Hits        26132    26136    +4     
- Misses      25788    25789    +1     
  Partials     1867     1867           
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 49.95% <57.14%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/infra/online_stores/sqlite.py 61.00% <57.14%> (+0.22%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81673d9...a5cf5c0. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants