Skip to content

Commit 77b815e

Browse files
kevin-dpclaude
kevin-dp
and
claude
authored
fix(db): re-accumulate pending changes by custom getKey to avoid DuplicateKeySyncError (#1290)
* Unit test to reproduce conflicting key * fix(db): re-accumulate pending changes by custom getKey to avoid DuplicateKeySyncError When a LEFT JOIN live query has a custom getKey and the right collection is populated after initial sync, the IVM retracts old rows and inserts new rows with different D2 internal keys that map to the same custom key. Because pendingChanges is keyed by D2 key, these end up as separate entries. If the INSERT is processed first, the sync layer throws DuplicateKeySyncError. Fix by re-accumulating pendingChanges by custom key in flushPendingChanges when this.config.getKey is set. This merges entries that the IVM sees as distinct D2 rows but the user considers the same logical row, producing a single UPDATE instead of a conflicting INSERT + DELETE. Closes #677 Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: add changeset for getKey collision fix Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: generalize changeset description Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 parent ac4ce67 commit 77b815e

3 files changed

Lines changed: 160 additions & 2 deletions

File tree

‎.changeset/fix-getkey-collision.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/db': patch
3+
---
4+
5+
fix: avoid DuplicateKeySyncError in join live queries when custom getKey only considers the identity of one of the joined collections

‎packages/db/src/query/live/collection-config-builder.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -728,8 +728,37 @@ export class CollectionConfigBuilder<
728728
if (pendingChanges.size === 0) {
729729
return
730730
}
731+
732+
let changesToApply = pendingChanges
733+
734+
// When a custom getKey is provided, multiple D2 internal keys may map
735+
// to the same user-visible key. Re-accumulate by custom key so that a
736+
// retract + insert for the same logical row merges into an UPDATE
737+
// instead of a separate DELETE and INSERT that can race.
738+
if (this.config.getKey) {
739+
const merged = new Map<unknown, Changes<TResult>>()
740+
for (const [, changes] of pendingChanges) {
741+
const customKey = this.config.getKey(changes.value)
742+
const existing = merged.get(customKey)
743+
if (existing) {
744+
existing.inserts += changes.inserts
745+
existing.deletes += changes.deletes
746+
// Keep the value from the insert side (the new value)
747+
if (changes.inserts > 0) {
748+
existing.value = changes.value
749+
if (changes.orderByIndex !== undefined) {
750+
existing.orderByIndex = changes.orderByIndex
751+
}
752+
}
753+
} else {
754+
merged.set(customKey, { ...changes })
755+
}
756+
}
757+
changesToApply = merged
758+
}
759+
731760
begin()
732-
pendingChanges.forEach(this.applyChanges.bind(this, config))
761+
changesToApply.forEach(this.applyChanges.bind(this, config))
733762
commit()
734763
pendingChanges = new Map()
735764
}

‎packages/db/tests/query/join.test.ts‎

Lines changed: 125 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import {
1111
or,
1212
} from '../../src/query/index.js'
1313
import { createCollection } from '../../src/collection/index.js'
14-
import { mockSyncCollectionOptions } from '../utils.js'
14+
import {
15+
mockSyncCollectionOptions,
16+
mockSyncCollectionOptionsNoInitialState,
17+
} from '../utils.js'
1518

1619
// Sample data types for join testing
1720
type User = {
@@ -1900,6 +1903,127 @@ function createJoinTests(autoIndex: `off` | `eager`): void {
19001903
{ leftId: `l4`, right: undefined },
19011904
])
19021905
})
1906+
1907+
// Regression test for https://github.com/TanStack/db/issues/677
1908+
// When a custom getKey is provided to a left join live query and the right
1909+
// collection is populated after initial sync, the system should not throw
1910+
// DuplicateKeySyncError. The old row (with undefined right side) should be
1911+
// deleted before the new row (with populated right side) is inserted.
1912+
test(`left join with custom getKey should not throw DuplicateKeySyncError when right collection is populated after initial sync (autoIndex: ${autoIndex})`, () => {
1913+
type Player = {
1914+
name: string
1915+
club_id: string
1916+
position: string
1917+
}
1918+
1919+
type Client = {
1920+
name: string
1921+
player: string
1922+
email: string
1923+
}
1924+
1925+
type Balance = {
1926+
name: string
1927+
client: string
1928+
amount: number
1929+
}
1930+
1931+
const samplePlayers: Array<Player> = [
1932+
{ name: `player1`, club_id: `club1`, position: `forward` },
1933+
{ name: `player2`, club_id: `club1`, position: `midfielder` },
1934+
{ name: `player3`, club_id: `club1`, position: `defender` },
1935+
]
1936+
1937+
const sampleClients: Array<Client> = [
1938+
{ name: `client1`, player: `player1`, email: `[email protected]` },
1939+
{ name: `client2`, player: `player2`, email: `[email protected]` },
1940+
{ name: `client3`, player: `player3`, email: `[email protected]` },
1941+
]
1942+
1943+
const sampleBalances: Array<Balance> = [
1944+
{ name: `balance1`, client: `client1`, amount: 1000 },
1945+
{ name: `balance2`, client: `client2`, amount: 2000 },
1946+
{ name: `balance3`, client: `client3`, amount: 1500 },
1947+
]
1948+
1949+
const playersCollection = createCollection(
1950+
mockSyncCollectionOptions<Player>({
1951+
id: `test-players-getkey-collision-${autoIndex}`,
1952+
getKey: (player) => player.name,
1953+
initialData: samplePlayers,
1954+
autoIndex,
1955+
}),
1956+
)
1957+
1958+
const clientsCollection = createCollection(
1959+
mockSyncCollectionOptionsNoInitialState<Client>({
1960+
id: `test-clients-getkey-collision-${autoIndex}`,
1961+
getKey: (client) => client.name,
1962+
autoIndex,
1963+
}),
1964+
)
1965+
1966+
const balancesCollection = createCollection(
1967+
mockSyncCollectionOptions<Balance>({
1968+
id: `test-balances-getkey-collision-${autoIndex}`,
1969+
getKey: (balance) => balance.name,
1970+
initialData: sampleBalances,
1971+
autoIndex,
1972+
}),
1973+
)
1974+
1975+
const chainedJoinQuery = createLiveQueryCollection({
1976+
startSync: true,
1977+
getKey: (r) => r.player_name,
1978+
query: (q) =>
1979+
q
1980+
.from({ player: playersCollection })
1981+
.join(
1982+
{ client: clientsCollection },
1983+
({ client, player }) => eq(client.player, player.name),
1984+
`left`,
1985+
)
1986+
.join(
1987+
{ balance: balancesCollection },
1988+
({ balance, client }) => eq(balance.client, client?.name),
1989+
`left`,
1990+
)
1991+
.select(({ player, client, balance }) => ({
1992+
player_name: player.name,
1993+
client_name: client?.name,
1994+
balance_amount: balance?.amount,
1995+
})),
1996+
})
1997+
1998+
// Initial state: 3 players, no clients, so left join gives undefined for client and balance
1999+
expect(chainedJoinQuery.toArray).toHaveLength(3)
2000+
expect(
2001+
chainedJoinQuery.toArray.every((r) => r.client_name === undefined),
2002+
).toBe(true)
2003+
expect(
2004+
chainedJoinQuery.toArray.every((r) => r.balance_amount === undefined),
2005+
).toBe(true)
2006+
2007+
// Populating the clients collection should not throw DuplicateKeySyncError.
2008+
// The IVM retracts old rows (key e.g. player1 with undefined client) and inserts
2009+
// new rows (key e.g. player1 with populated client). Since both map to the same
2010+
// custom getKey, deletes must be processed before inserts to avoid a collision.
2011+
clientsCollection.utils.begin()
2012+
sampleClients.forEach((client) => {
2013+
clientsCollection.utils.write({ type: `insert`, value: client })
2014+
})
2015+
clientsCollection.utils.commit()
2016+
clientsCollection.utils.markReady()
2017+
2018+
// Should still have 3 results, now with client and balance data populated
2019+
expect(chainedJoinQuery.toArray).toHaveLength(3)
2020+
expect(
2021+
chainedJoinQuery.toArray.every((r) => r.client_name !== undefined),
2022+
).toBe(true)
2023+
expect(
2024+
chainedJoinQuery.toArray.every((r) => r.balance_amount !== undefined),
2025+
).toBe(true)
2026+
})
19032027
}
19042028

19052029
describe(`Query JOIN Operations`, () => {

0 commit comments

Comments
 (0)