-
Notifications
You must be signed in to change notification settings - Fork 152
fix(db): batch D2 output callbacks to prevent duplicate key errors in joins #1114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… joins When D2's incremental join processes data, it can produce multiple outputs during a single graph.run() iteration - first a partial join result, then the full result plus a delete of the partial. Previously, each output callback had its own begin()/commit() cycle, causing the second insert for the same key to fail with DuplicateKeySyncError. This fix: - Accumulates all changes from output callbacks into a pendingChanges Map - Flushes accumulated changes in a single transaction after each graph.run() - Adds subscribedToAllCollections check to updateLiveQueryStatus() to ensure markReady() is only called after the graph has processed data - Properly types flushPendingChanges on SyncState to avoid type assertions Co-Authored-By: Claude Opus 4.5 <[email protected]>
🦋 Changeset detectedLatest commit: 5ad89bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: +43 B (+0.05%) Total Size: 90.2 kB
ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 3.47 kB ℹ️ View Unchanged
|
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
kevin-dp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the changes LGTM.
I left a comment to improve code quality because a helper function was inlined which makes the code more complex than it needs to be.
Restore the accumulateChanges helper function instead of inlining its logic. This improves code readability and maintainability by keeping the accumulation logic in a separate, testable function. Co-authored-by: Kevin <[email protected]>
|
🎉 This PR has been released! Thank you for your contribution! |
Summary
Fixes
isReady()returningtruewhiletoArray()returns[]. Also fixes a latent bug that was exposed by the timing fix.The Bug Report
Root Cause
markReady()was being called insubscribeToAllCollections()before the D2 graph had a chance to run and process data:The graph only runs in
maybeRunGraph(), which happens after subscriptions are set up. SoisReadybecametruewhile the data pipeline was still empty.The Fix
Add a guard to
updateLiveQueryStatus()so it only marks ready after subscriptions are set up AND the graph has had a chance to run:And remove the premature call from
subscribeToAllCollections(). The canonical place to mark ready is now aftergraph.run()inmaybeRunGraph().The Journey: Exposing a Latent Bug
When I made the timing fix, a test started failing: "should allow custom getKey with joins (1:1 relationships)"
Why did this test "pass" before?
The old code called
markReady()before the graph ran, which resolvedpreload()early. The duplicate key error was thrown after the promise resolved, so it was swallowed. The test appeared to pass, but was actually broken.What was the actual bug?
D2's incremental join produces multiple outputs during a single
graph.run():[key=1, undefined]with multiplicity +1[key=1, data]with multiplicity +1, AND delete of partial with multiplicity -1Previously, each output committed immediately. The first commit added key "1" to syncedData, and the second output's insert failed because the key already existed.
The fix: Batch all outputs within a single
graph.run()into one transaction. Changes for the same key are accumulated (+1, +1, -1 = +1) before committing.Key Invariants
markReady()only after graph processes data - thesubscribedToAllCollectionscheck ensures we don't mark ready before the graph runsgraph.run()share one transaction - prevents intermediate join states from causing duplicate key errorsVerification
Files Changed
collection-config-builder.tssubscribedToAllCollectionsguard toupdateLiveQueryStatus(), remove premature call fromsubscribeToAllCollections(), batch outputs inpendingChangesMaptypes.tsflushPendingChangestoSyncStatetypesync.tsvaluesEqualvariablelive-query-collection.test.ts🤖 Generated with Claude Code