Fix SQLite variable limit in exports; extract shared-project creation flow - #453
Merged
Conversation
… flow - export:project, export:all-contacts, and export:vcard-all-contacts built unchunked `IN (...)` clauses from contact-ID lists that could exceed SQLite's 999-variable limit — both from the full member/contact set and from the `contactIds` selection filter. Bulk sub-table fetches now go through a chunked `fetchContactSubTables()` helper (chunks at 500, mirrors the pattern already used in the sync engine), and filterIds now filters in JS via a Set instead of building a SQL IN clause, since selection size is user-controlled and can't be bounded (fixes #433). - Deduplicated the export handlers' sub-table bulk-fetch/group and write-to-spreadsheet logic into `fetchContactSubTables()` + `writeExportRows()`, used by both export:project and export:all-contacts. - Extracted the ~40-line "create shared DB file, write project metadata, write local project record, compensating cleanup on failure" flow shared by projects:createShared, convertToShared, and regenerateShared into one `createSharedProjectFile()` helper. This also fixes a real bug: regenerateShared was missing the compensating cleanup (evict shared DB connection + unlink the file) that the other two handlers had, so a failure in its local DB write left an orphaned shared file on disk (fixes #442, items 1 and 2; item 3, sync-engine statement reuse, is left as explicitly low-priority per the issue text). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
#433 — Exports and shared-project conversion fail with >999 contacts (SQLite variable limit)
export:project,export:all-contacts, andexport:vcard-all-contactsbuilt unchunkedIN (...)clauses from contact-ID lists that could exceed SQLite's defaultSQLITE_LIMIT_VARIABLE_NUMBERof 999 — both from the full member/contact-ID set used for the bulk email/phone/link/handle fetches, and from thecontactIdsselection filter used for "export selected" flows.fetchContactSubTables()helper that chunksINclauses at 500 (mirroring the patternpushAppendOnlyalready uses in the sync engine), via a small genericchunkedIn()helper.contactIdsselection filter (which can be arbitrarily large — a user can select-all on a huge table) now filters in JS with aSetinstead of building a SQLINclause, since its size isn't bounded by anything the code controls.projects:convertToShared/regenerateShared's part of this issue (anUPDATE ... WHERE id IN (...)over member contact IDs) turned out to already be fixed as a side effect of the Sync: explicit replication state — per-project push tracking, full-scan pull, deletion tombstones #449 sync-replication-state redesign — it's now a single project-scopedDELETE FROM sync_pushed WHERE project_id = ?with no per-contactINclause at all.#442 — Extract duplicated flows: shared-project creation, export bulk-fetch, sync-engine statement reuse
Doing together with #433 since the issue calls out they touch the same lines.
projects:createShared,convertToShared, andregenerateSharedeach repeated the same ~40-line "create shared DB file → write project metadata → write local project record → compensating cleanup on failure" flow, and had drifted:regenerateSharedwas missing the compensating cleanup (evict the shared DB connection + unlink the file) that the other two had, so a failure in its local DB write left an orphaned shared file on disk. Extracted into onecreateSharedProjectFile()helper — the missing cleanup comes free.fetchContactSubTables()helper above roughly halves the duplicated per-sub-table fetch/group pattern (previously repeated 8 times across the two handlers). Also extracted the identical "write rows to a temp file, atomically rename into place" tail of both handlers intowriteExportRows().Closes #433
Closes #442
Test plan
npm run rebuild:node && npm test— full suite passes (519 tests)src/test/export.test.ts: exports 1200 contacts / 1200 project members without hitting "too many SQL variables" (would have thrown pre-fix); exports a filtered selection of >999 contact IDs correctly (excludes non-selected rows); sub-table data (emails/phones/handles) still round-trips correctly through the refactored fetch pathsrc/test/projects.test.tsforcreateShared/convertToShared/regenerateShared: happy-path behavior (payload returned,sync_pushedcleared,is_sharedflag set), and a regression test provingregenerateSharednow cleans up the shared file when the local DB write fails (previously missing — the mockedcloseSharedDb/unlinkSynccalls would not have happened on the old code)npx tsc --noEmitclean on bothtsconfig.node.jsonandtsconfig.web.jsonnpx eslint .clean (only pre-existing, unrelatedreact-hooks/exhaustive-depswarnings)npm run rebuild— restored Electron-targeted native binaries after testing🤖 Generated with Claude Code