Skip to content

Commit 1c00cf5

Browse files
committed
fix(tables): key unique JSON values by canonical form so key order never hides a batch duplicate
1 parent fbc3db1 commit 1c00cf5

2 files changed

Lines changed: 51 additions & 17 deletions

File tree

‎apps/sim/lib/table/rows/row-writes.integration.ts‎

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { batchInsertRows, batchUpdateRows, insertRow, updateRow } from '@/lib/ta
2929
import { lockUniqueColumns, lockUniqueValues } from '@/lib/table/rows/unique-locks'
3030
import { getTableById } from '@/lib/table/service'
3131
import { getOrCreateTableSnapshot } from '@/lib/table/snapshot-cache'
32-
import type { ColumnDefinition, TableDefinition } from '@/lib/table/types'
32+
import type { ColumnDefinition, RowData, TableDefinition } from '@/lib/table/types'
3333

3434
const url = readTestDatabaseUrl()
3535
if (process.env.DATABASE_URL !== url) {
@@ -67,7 +67,7 @@ async function createTable(columns: ColumnDefinition[]): Promise<TableDefinition
6767

6868
async function seedRows(
6969
tableId: string,
70-
rows: Array<{ id: string; data: Record<string, string>; orderKey: string | null }>
70+
rows: Array<{ id: string; data: RowData; orderKey: string | null }>
7171
) {
7272
await db.insert(userTableRows).values(rows.map((row) => ({ ...row, tableId, workspaceId })))
7373
}
@@ -177,7 +177,10 @@ describe('table row writes against real PostgreSQL', () => {
177177
}
178178
}
179179

180-
async function storedCount(tableId: string, match: Record<string, string | number>) {
180+
async function storedCount(
181+
tableId: string,
182+
match: Record<string, string | number | Record<string, number>>
183+
) {
181184
const [{ count }] = await control<{ count: number }[]>`SELECT count(*)::int AS count
182185
FROM user_table_rows WHERE table_id = ${tableId} AND data @> ${control.json(match)}`
183186
return count
@@ -326,17 +329,21 @@ describe('table row writes against real PostgreSQL', () => {
326329
})
327330
await holding
328331

329-
const replace = importReplaceRows(
330-
table,
331-
[{ id: 'code', name: 'code', type: 'string', unique: true }],
332-
{ rows: [{ name: 'fresh', code: 'c-1' }], workspaceId },
333-
'unique-race'
334-
)
335-
expect(await waitForLockWaiters(table.id, { onOrderLock: 0, onValueLock: 1 })).toEqual({
336-
onOrderLock: 0,
337-
onValueLock: 1,
338-
})
339-
release()
332+
let replace: Promise<unknown> | undefined
333+
try {
334+
replace = importReplaceRows(
335+
table,
336+
[{ id: 'code', name: 'code', type: 'string', unique: true }],
337+
{ rows: [{ name: 'fresh', code: 'c-1' }], workspaceId },
338+
'unique-race'
339+
)
340+
expect(await waitForLockWaiters(table.id, { onOrderLock: 0, onValueLock: 1 })).toEqual({
341+
onOrderLock: 0,
342+
onValueLock: 1,
343+
})
344+
} finally {
345+
release()
346+
}
340347

341348
const results = await Promise.allSettled([writer, replace])
342349
expect(results.map((result) => result.status)).toEqual(['fulfilled', 'fulfilled'])
@@ -451,6 +458,31 @@ describe('table row writes against real PostgreSQL', () => {
451458
).rejects.toThrow(/must be unique/)
452459
expect(await storedCount(table.id, { email: 'dup@example.test' })).toBe(0)
453460
})
461+
462+
it('rejects a batch update that writes one JSON value to two rows in different key orders', async () => {
463+
const table = await createTable([{ id: 'meta', name: 'meta', type: 'json', unique: true }])
464+
await seedRows(table.id, [
465+
{ id: `${table.id}-a`, data: { meta: { n: 1 } }, orderKey: 'a0' },
466+
{ id: `${table.id}-b`, data: { meta: { n: 2 } }, orderKey: 'a1' },
467+
])
468+
469+
await expect(
470+
batchUpdateRows(
471+
{
472+
tableId: table.id,
473+
workspaceId,
474+
updates: [
475+
{ rowId: `${table.id}-a`, data: { meta: { x: 1, y: 2 } } },
476+
{ rowId: `${table.id}-b`, data: { meta: { y: 2, x: 1 } } },
477+
],
478+
capabilityGovernedUserId: null,
479+
},
480+
table,
481+
'unique-batch-update'
482+
)
483+
).rejects.toThrow(/must be unique/)
484+
expect(await storedCount(table.id, { meta: { x: 1, y: 2 } })).toBe(0)
485+
})
454486
})
455487

456488
describe.skipIf(!migrated)('rows_version', () => {

‎apps/sim/lib/table/validation.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { db } from '@sim/db'
66
import { userTableRows } from '@sim/db/schema'
77
import { and, eq, or, type SQL, sql } from 'drizzle-orm'
88
import { NextResponse } from 'next/server'
9+
import { canonicalJson } from '@/lib/api/cursor-binding'
910
import { getColumnId } from '@/lib/table/column-keys'
1011
import type { CoerceResult, TypeSpecificColumnKey } from '@/lib/table/column-types'
1112
import {
@@ -407,11 +408,12 @@ export function getUniqueColumns(schema: TableSchema): ColumnDefinition[] {
407408
}
408409

409410
/**
410-
* The key two unique-column values share exactly when the unique check treats them as equal.
411-
* In-batch duplicate detection and the unique-value locks both key on it.
411+
* The key two unique-column values share when the unique check treats them as equal. Object keys
412+
* are sorted, since the check compares JSONB, where key order carries no meaning. In-batch
413+
* duplicate detection and the unique-value locks both key on it.
412414
*/
413415
export function uniqueValueKey(value: JsonValue, column: ColumnDefinition): string {
414-
return JSON.stringify(columnValueForEquality(value, column))
416+
return canonicalJson(columnValueForEquality(value, column))
415417
}
416418

417419
/** Validates unique constraints against existing rows (in-memory version for batch validation within a batch). */

0 commit comments

Comments
 (0)