fix(status): paginate col.get() to stay under SQLite variable limit by eldar702 · Pull Request #1016 · MemPalace/mempalace

eldar702 · 2026-04-18T19:25:04Z

Summary

mempalace status crashes on any palace with more than ~32 766 drawers with:

chromadb.errors.InternalError: Error executing plan:
  Internal error: error returned from database:
  (code: 1) too many SQL variables

Root cause: miner.py::status() calls col.get(limit=total) unbounded. Chroma issues one SQL statement whose host-parameter count grows with the limit, and once total > 32 766 (SQLite's default SQLITE_MAX_VARIABLE_NUMBER) SQLite rejects it.

Change

Paginate col.get() in 10 000-drawer batches and aggregate metadatas. No behavior change for smaller palaces.

-    # Count by wing and room
+    # Count by wing and room. Paginate col.get() to stay under SQLite's
+    # SQLITE_MAX_VARIABLE_NUMBER (default 32766). A single col.get(limit=total)
+    # raises `too many SQL variables` once a palace exceeds that many drawers.
     total = col.count()
-    r = col.get(limit=total, include=["metadatas"]) if total else {"metadatas": []}
-    metas = r["metadatas"]
+    metas: list = []
+    if total:
+        BATCH = 10000
+        for offset in range(0, total, BATCH):
+            r = col.get(limit=BATCH, offset=offset, include=["metadatas"])
+            metas.extend(r.get("metadatas", []) or [])

One file touched, 9 added / 3 removed.

Reproducer

On any palace with > 32 766 drawers:

mempalace status    # crashes with the traceback above

Minimal programmatic repro:

from mempalace.config import MempalaceConfig
from mempalace.palace import get_collection
col = get_collection(MempalaceConfig().palace_path)
col.get(limit=col.count(), include=["metadatas"])
# → InternalError: too many SQL variables

Exact inflection point:

limit=32766  → ok (32766 rows)
limit=33000  → FAIL: too many SQL variables

Test plan

mempalace status succeeds on a 43 852-drawer palace after the patch (produced the normal Wing/Room breakdown)
mempalace status succeeds on an empty palace
No change in output format (verified by diffing against a < 32 K drawer palace before / after)
Existing test suite (pytest) — not run locally; maintainers please verify in CI

Why 10 000

SQLITE_MAX_VARIABLE_NUMBER is 32 766 on most SQLite ≥ 3.32 builds; some older distributions ship it as 999. 10 000 is comfortably below both and large enough that aggregation overhead stays negligible (palace would need > 100 K drawers before the per-batch overhead mattered).

`mempalace status` currently calls `col.get(limit=total, include=["metadatas"])` with no pagination. Chroma's backend issues a SQL statement whose host-parameter count scales with the limit. Once `total > 32766` (SQLite's default `SQLITE_MAX_VARIABLE_NUMBER`) the call raises: chromadb.errors.InternalError: Error executing plan: Internal error: error returned from database: (code: 1) too many SQL variables Any palace larger than ~32 K drawers can no longer run `mempalace status`. Fix: paginate the .get() in 10 K batches and aggregate metadatas. No behavior change for smaller palaces. Fixes MemPalace#1015

bensig · 2026-04-19T05:41:27Z

Code review

Found 1 issue:

Duplicate of already-approved #851 with narrower scope. PR fix(status): paginate metadata fetch to support large palaces #851 (OPEN, approved by @bensig on 2026-04-15) paginates the same col.get() in the same status() function and additionally fixes #850 (silent truncation at 10k drawers, where len(metas) displayed 10000 regardless of true count). This PR retains len(metas) in the header rather than switching to col.count() and materializes the full metadata list instead of aggregating per-batch, so it resolves mempalace status crashes on palaces >~32K drawers (SQLite variable limit) #1015's crash but leaves mempalace status silently truncates at 10,000 drawers (incorrect count displayed) #850's silent-truncation symptom unaddressed. Before merging for v3.3.2, consider landing fix(status): paginate metadata fetch to support large palaces #851 instead (or rebasing this onto fix(status): paginate metadata fetch to support large palaces #851's approach).

mempalace/mempalace/miner.py

Lines 850 to 861 in 492b316

    
           # Count by wing and room. Paginate col.get() to stay under SQLite's 
        
           # SQLITE_MAX_VARIABLE_NUMBER (default 32766). A single col.get(limit=total) 
        
           # raises `too many SQL variables` once a palace exceeds that many drawers. 
        
           total = col.count() 
        
           metas: list = [] 
        
           if total: 
        
               BATCH = 10000 
        
               for offset in range(0, total, BATCH): 
        
                   r = col.get(limit=BATCH, offset=offset, include=["metadatas"]) 
        
                   metas.extend(r.get("metadatas", []) or []) 
        
           wing_rooms = defaultdict(lambda: defaultdict(int))

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

jphein · 2026-04-19T16:07:36Z

For context — I filed #1036 this morning with the same pagination approach, not realising your PR already existed. Sorry for the dup. Yours is the narrower fix and landed first; the logic looks right to me. The fork has been running a semantically identical paginated col.get() since 2026-04-10 on a 152,794-drawer palace with no issues. Happy to see this merge.

jphein · 2026-04-19T16:10:36Z

Correcting my earlier comment — I missed @bensig's note that #851 is the canonical approved fix (2026-04-15, also handles #850). Yours and mine are both narrower duplicates of it. #851 is the path forward.

jfcampbl · 2026-04-21T17:23:31Z

Confirming the reproducer and fix on a large palace.

Setup: mempalace v3.3.2, pipx install, chromadb 1.5.8, Python 3.13, macOS, palace with 114,232 drawers across 11 wings (5 project + 6 diary).

Crash trace:

File ".../mempalace/miner.py", line 852, in status
    r = col.get(limit=total, include=["metadatas"]) if total else {"metadatas": []}
  File ".../mempalace/backends/chroma.py", line 341, in get
    raw = self._collection.get(**kwargs)
  File ".../chromadb/api/models/Collection.py", line 161, in get
    get_results = self._client._get(...)
  File ".../chromadb/api/rust.py", line 440, in _get
    rust_response = self.bindings.get(...)
chromadb.errors.InternalError: Error executing plan: Internal error: error returned from database: (code: 1) too many SQL variables

After applying this PR: mempalace status enumerates all 11 wings in 5.6s, total reported as 114,232. No truncation, no crash.

The offset-pagination pattern in this PR matches what cli.py:309-319 and repair.py:240-246 already use internally, so this just closes a gap left by PR #707 (which removed the 10k sampling cap but swapped it for an unbatched col.get(limit=total)).

Also resolves #1073 (filed today), #950, #802.

+1 for merge.

sha2fiddy · 2026-04-21T18:34:12Z

Ran into this exact bug last week on a ~465k-drawer palace and filed #1073 before I spotted your PR. Same root cause: miner.py:852, col.get(limit=total) overflowing the 32,766-variable cap. Patch reads correctly to me — pagination is non-overlapping, order-preserving, and the if total: guard handles the empty-palace case. Happy to actually run it against our big palace if a second data point would help.

One thing before you merge though. There's another call site with the same pattern that this PR leaves alone, mempalace/closet_llm.py:224:

total = drawers_col.count()                                  # line 219
...
all_data = drawers_col.get(limit=total, include=["documents", "metadatas"])

Reproduces identically via python -m mempalace.closet_llm on anything past 32k drawers. Rest of the col.get(limit=...) sites in the repo (cli.py:315, repair.py:246, palace_graph.py:59, exporter.py:68) are already paginated, so closet_llm is the only straggler.

If it's easy to drop the same 10k chunking into closet_llm.py on this branch, I'll close #1073 once you merge. If you'd rather keep this PR tight, I can open a one-liner against closet_llm after yours lands. No strong preference either way.

bensig · 2026-04-22T00:38:38Z

Superseded by #851 which was merged. #851 uses per-batch aggregation and fixes both #1015 and #850 silent truncation. Thank you for the contribution!

Mirror the pagination pattern PR MemPalace#851 landed in miner.py:status(). A single drawers_col.get(limit=total, ...) on palaces larger than SQLite's SQLITE_MAX_VARIABLE_NUMBER (32766) crashes inside chromadb. Fetch drawers in batch_size=5000 chunks, stepping offset until the collection is drained. by_source aggregation semantics are preserved exactly — grouping, wing filter, meta capture all unchanged. Closes MemPalace#1073. Related: MemPalace#802, MemPalace#850, MemPalace#1016.

eldar702 requested review from bensig, igorls and milla-jovovich as code owners April 18, 2026 19:25

eldar702 force-pushed the fix/status-sqlite-variable-limit branch from 92f98f9 to 492b316 Compare April 18, 2026 19:30

eldar702 changed the base branch from main to develop April 18, 2026 19:31

This was referenced Apr 19, 2026

status crashes with 'too many SQL variables' at large drawer counts (123k+) #950

Closed

mempalace status crashes on palaces >~32K drawers (SQLite variable limit) #1015

Closed

mempalace status crashes with "too many SQL variables" on large palaces #802

Open

jphein mentioned this pull request Apr 19, 2026

perf: paginate miner.status() — fixes SQLITE_MAX_VARIABLE_NUMBER crash past ~32K drawers #1036

Closed

2 tasks

jfcampbl mentioned this pull request Apr 21, 2026

mempalace status CLI fails on palaces with >32k drawers: too many SQL variables (miner.py:852) #1073

Closed

This was referenced Apr 21, 2026

fix(status): paginate metadata fetch to support large palaces #851

Merged

bug: existing 0.6.x palaces need mempalace repair after #1010 — filtered query(where=...) fails with 'Error finding id' until HNSW index is rebuilt #1035

Open

potterdigital mentioned this pull request Apr 21, 2026

fix(search): hint at mempalace repair when filtered query hits HNSW index mismatch (#1035) #1081

Open

4 tasks

bensig closed this Apr 22, 2026

jphein mentioned this pull request Apr 22, 2026

v3.3.2 release defect: mempalace-mcp entry point missing from pyproject.toml despite plugin.json requiring it — MCP server fails to start on fresh install #1093

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(status): paginate col.get() to stay under SQLite variable limit#1016

fix(status): paginate col.get() to stay under SQLite variable limit#1016
eldar702 wants to merge 1 commit intoMemPalace:developfrom
eldar702:fix/status-sqlite-variable-limit

eldar702 commented Apr 18, 2026

Uh oh!

bensig commented Apr 19, 2026

Uh oh!

jphein commented Apr 19, 2026

Uh oh!

jphein commented Apr 19, 2026

Uh oh!

jfcampbl commented Apr 21, 2026

Uh oh!

sha2fiddy commented Apr 21, 2026 •

edited

Loading

Uh oh!

bensig commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

eldar702 commented Apr 18, 2026

Summary

Change

Reproducer

Test plan

Why 10 000

Uh oh!

bensig commented Apr 19, 2026

Code review

Uh oh!

jphein commented Apr 19, 2026

Uh oh!

jphein commented Apr 19, 2026

Uh oh!

jfcampbl commented Apr 21, 2026

Uh oh!

sha2fiddy commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bensig commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sha2fiddy commented Apr 21, 2026 •

edited

Loading