fix(core): chunk select_by_ids to stay under SQLite bound-parameter limit - #1057
Conversation
|
The production chunking change looks right, but the regression test does not currently prove chunking: modern SQLite builds allow far more than 1,100 bound parameters, so the pre-fix implementation can pass this test unchanged. It also inserts 1,100 rows into every backend/matrix job. Please make the test deterministic by lowering the chunk size in the test and asserting the number of |
…imit reindex --embeddings passes every entity id to find_by_ids at once, which built a single WHERE id IN (...) query binding the full list. SQLite caps bound parameters per statement (999 on builds older than 3.32.0), so large projects crashed with "too many SQL variables" at 0% before embedding started. Chunk select_by_ids into 500-id slices, one query per slice, and concatenate the results. Fixing the shared repository helper protects all bulk-id callers: vector reindex, project index maintenance, and note content batch reconciliation. Fixes #1045 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Signed-off-by: phernandez <paul@basicmachines.co>
c74ef52 to
28cdbfb
Compare
|
@codex review recheck |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
reindex --embeddingscrashed withsqlite3.OperationalError: too many SQL variablesat 0% on large projects, before any embedding work started.Root cause
Repository.select_by_ids()built oneWHERE primary_key IN (...)query for the entire id list.reindex_vectors()passes all entity ids at once, while older SQLite builds cap bound parameters at 999.Fix
Chunk
select_by_ids()into slices of 500 ids, leaving headroom for theproject_idbind, execute one query per slice, and concatenate the results. Return type and empty-list behavior are unchanged.Fixing the shared repository helper protects all bulk-id callers, including
indexing/project_index_maintenance.pyandindexing/note_content_batch_reconciliation.py.Test plan
SELECT_BY_IDS_CHUNK_SIZEto 2, queries five rows, and asserts the real repository path issues exactly three statements and returns every row. This proves both full chunks and the tail without depending on the SQLite build parameter limit.uv run pytest -q tests/repository/test_repository.py— 14 passed.just fast-check— passed.Fixes #1045
🤖 Generated with Claude Code