Skip to content

Commit 60b80ec

Browse files
waleedlatif1claude
andauthored
improvement(tables): race-free row-count trigger + scoped tx timeouts (#4289)
* improvement(tables): race-free row-count trigger + scoped tx timeouts * fix(tables): close upsert race + serialize replaceTableRows Two concurrency bugs flagged by review: 1. `upsertRow` insert path: removing FOR UPDATE broke serialization between the initial existing-row SELECT and the INSERT. Two concurrent upserts on the same conflict target could both find no match, then both insert, producing a duplicate that bypasses the app-level unique check. Fixed by re-checking for the matching row *after* acquiring the per-table advisory lock; if a racing tx already inserted, switch to UPDATE. 2. `replaceTableRows`: under READ COMMITTED each tx's DELETE uses its own MVCC snapshot, so two concurrent replaces could each DELETE only the rows visible at their start, then both INSERT, leaving the table with the union of both row sets. Fixed by acquiring the per-table advisory lock at the start of the tx to serialize replaces against each other and against auto-position inserts. Also updated a stale docstring on `upsertRow` that still referenced the removed FOR UPDATE. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(tables): serialize explicit-position inserts with advisory lock The `(table_id, position)` index is non-unique. Concurrent explicit- position inserts at the same slot can both observe the slot empty, both skip the shift, then each INSERT — producing duplicate `(table_id, position)` rows. Take the per-table advisory lock in the explicit-position branches of `insertRow` and `batchInsertRows` (the auto-position branches already do this). Updated the test that asserted the explicit path skipped the lock, and added coverage for `batchInsertRows` with explicit positions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(tables): dedupe upsert UPDATE path + extract nextAutoPosition Two pure cleanups on the round-1 changes: 1. Extract `nextAutoPosition(trx, tableId)` — the `SELECT coalesce(max( position), -1) + 1` pattern was repeated in three call sites (`insertRow` auto branch, `batchInsertRows` auto branch, `upsertRow` insert branch). One helper, same behavior. 2. Consolidate `upsertRow` update path. The initial-SELECT match and the post-lock re-select match previously had two literal duplicates of the same UPDATE + return block. Resolve `matchedRowId` first, then run one UPDATE branch. Lock is still only acquired when we don't find a match on the first pass. No behavior change. 98/98 table unit tests unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent af4be77 commit 60b80ec

11 files changed

Lines changed: 15990 additions & 168 deletions

File tree

apps/sim/app/api/table/[tableId]/rows/route.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ const QueryRowsSchema = z.object({
6666
.min(0, 'Offset must be 0 or greater')
6767
.optional()
6868
.default(0),
69+
includeTotal: z
70+
.preprocess(
71+
(val) => (val === null || val === undefined || val === '' ? undefined : val === 'true'),
72+
z.boolean().optional()
73+
)
74+
.default(true),
6975
})
7076

7177
const nonEmptyFilter = z
@@ -328,6 +334,7 @@ export const GET = withRouteHandler(
328334
const sortParam = searchParams.get('sort')
329335
const limit = searchParams.get('limit')
330336
const offset = searchParams.get('offset')
337+
const includeTotalParam = searchParams.get('includeTotal')
331338

332339
let filter: Record<string, unknown> | undefined
333340
let sort: Sort | undefined
@@ -349,6 +356,7 @@ export const GET = withRouteHandler(
349356
sort,
350357
limit,
351358
offset,
359+
includeTotal: includeTotalParam,
352360
})
353361

354362
const accessResult = await checkAccess(tableId, authResult.userId, 'read')
@@ -398,17 +406,19 @@ export const GET = withRouteHandler(
398406
query = query.orderBy(userTableRows.position) as typeof query
399407
}
400408

401-
const countQuery = db
402-
.select({ count: sql<number>`count(*)` })
403-
.from(userTableRows)
404-
.where(and(...baseConditions))
405-
406-
const [{ count: totalCount }] = await countQuery
409+
let totalCount: number | null = null
410+
if (validated.includeTotal) {
411+
const [{ count }] = await db
412+
.select({ count: sql<number>`count(*)` })
413+
.from(userTableRows)
414+
.where(and(...baseConditions))
415+
totalCount = Number(count)
416+
}
407417

408418
const rows = await query.limit(validated.limit).offset(validated.offset)
409419

410420
logger.info(
411-
`[${requestId}] Queried ${rows.length} rows from table ${tableId} (total: ${totalCount})`
421+
`[${requestId}] Queried ${rows.length} rows from table ${tableId} (total: ${totalCount ?? 'n/a'})`
412422
)
413423

414424
return NextResponse.json({
@@ -424,7 +434,7 @@ export const GET = withRouteHandler(
424434
r.updatedAt instanceof Date ? r.updatedAt.toISOString() : String(r.updatedAt),
425435
})),
426436
rowCount: rows.length,
427-
totalCount: Number(totalCount),
437+
totalCount,
428438
limit: validated.limit,
429439
offset: validated.offset,
430440
},

apps/sim/app/api/v1/tables/[tableId]/rows/route.ts

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ const QueryRowsSchema = z.object({
7171
.optional()
7272
)
7373
.default(0),
74+
includeTotal: z
75+
.preprocess(
76+
(val) => (val === null || val === undefined || val === '' ? undefined : val === 'true'),
77+
z.boolean().optional()
78+
)
79+
.default(true),
7480
})
7581

7682
const nonEmptyFilter = z
@@ -219,6 +225,7 @@ export const GET = withRouteHandler(
219225
sort,
220226
limit: searchParams.get('limit'),
221227
offset: searchParams.get('offset'),
228+
includeTotal: searchParams.get('includeTotal'),
222229
})
223230

224231
const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId)
@@ -268,16 +275,37 @@ export const GET = withRouteHandler(
268275
query = query.orderBy(userTableRows.position) as typeof query
269276
}
270277

271-
const countQuery = db
272-
.select({ count: sql<number>`count(*)` })
273-
.from(userTableRows)
274-
.where(and(...baseConditions))
278+
const rowsPromise = query.limit(validated.limit).offset(validated.offset)
279+
280+
let totalCount: number | null = null
281+
if (validated.includeTotal) {
282+
const countQuery = db
283+
.select({ count: sql<number>`count(*)` })
284+
.from(userTableRows)
285+
.where(and(...baseConditions))
286+
const [countResult, rows] = await Promise.all([countQuery, rowsPromise])
287+
totalCount = Number(countResult[0].count)
288+
return NextResponse.json({
289+
success: true,
290+
data: {
291+
rows: rows.map((r) => ({
292+
id: r.id,
293+
data: r.data,
294+
position: r.position,
295+
createdAt:
296+
r.createdAt instanceof Date ? r.createdAt.toISOString() : String(r.createdAt),
297+
updatedAt:
298+
r.updatedAt instanceof Date ? r.updatedAt.toISOString() : String(r.updatedAt),
299+
})),
300+
rowCount: rows.length,
301+
totalCount,
302+
limit: validated.limit,
303+
offset: validated.offset,
304+
},
305+
})
306+
}
275307

276-
const [countResult, rows] = await Promise.all([
277-
countQuery,
278-
query.limit(validated.limit).offset(validated.offset),
279-
])
280-
const totalCount = countResult[0].count
308+
const rows = await rowsPromise
281309

282310
return NextResponse.json({
283311
success: true,
@@ -292,7 +320,7 @@ export const GET = withRouteHandler(
292320
r.updatedAt instanceof Date ? r.updatedAt.toISOString() : String(r.updatedAt),
293321
})),
294322
rowCount: rows.length,
295-
totalCount: Number(totalCount),
323+
totalCount,
296324
limit: validated.limit,
297325
offset: validated.offset,
298326
},

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-data.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export function useTableData({
3434
offset: 0,
3535
filter: queryOptions.filter,
3636
sort: queryOptions.sort,
37+
includeTotal: false,
3738
enabled: Boolean(workspaceId && tableId),
3839
})
3940

apps/sim/hooks/queries/tables.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,14 @@ interface TableRowsParams {
3838
offset: number
3939
filter?: Filter | null
4040
sort?: Sort | null
41+
/** When `false`, skip the server-side `COUNT(*)` and receive `totalCount: null`. */
42+
includeTotal?: boolean
4143
}
4244

4345
interface TableRowsResponse {
4446
rows: TableRow[]
45-
totalCount: number
47+
/** `null` when the request opted out of the count via `includeTotal: false`. */
48+
totalCount: number | null
4649
}
4750

4851
interface RowMutationContext {
@@ -64,12 +67,14 @@ function createRowsParamsKey({
6467
offset,
6568
filter,
6669
sort,
70+
includeTotal,
6771
}: Omit<TableRowsParams, 'workspaceId' | 'tableId'>): string {
6872
return JSON.stringify({
6973
limit,
7074
offset,
7175
filter: filter ?? null,
7276
sort: sort ?? null,
77+
includeTotal: includeTotal ?? true,
7378
})
7479
}
7580

@@ -98,6 +103,7 @@ async function fetchTableRows({
98103
offset,
99104
filter,
100105
sort,
106+
includeTotal,
101107
signal,
102108
}: TableRowsParams & { signal?: AbortSignal }): Promise<TableRowsResponse> {
103109
const searchParams = new URLSearchParams({
@@ -114,22 +120,26 @@ async function fetchTableRows({
114120
searchParams.set('sort', JSON.stringify(sort))
115121
}
116122

123+
if (includeTotal === false) {
124+
searchParams.set('includeTotal', 'false')
125+
}
126+
117127
const res = await fetch(`/api/table/${tableId}/rows?${searchParams}`, { signal })
118128
if (!res.ok) {
119129
const error = await res.json().catch(() => ({}))
120130
throw new Error(error.error || 'Failed to fetch rows')
121131
}
122132

123133
const json: {
124-
data?: { rows: TableRow[]; totalCount: number }
134+
data?: { rows: TableRow[]; totalCount: number | null }
125135
rows?: TableRow[]
126-
totalCount?: number
136+
totalCount?: number | null
127137
} = await res.json()
128138

129139
const data = json.data || json
130140
return {
131141
rows: (data.rows || []) as TableRow[],
132-
totalCount: data.totalCount || 0,
142+
totalCount: data.totalCount ?? null,
133143
}
134144
}
135145

@@ -209,9 +219,10 @@ export function useTableRows({
209219
offset,
210220
filter,
211221
sort,
222+
includeTotal,
212223
enabled = true,
213224
}: TableRowsParams & { enabled?: boolean }) {
214-
const paramsKey = createRowsParamsKey({ limit, offset, filter, sort })
225+
const paramsKey = createRowsParamsKey({ limit, offset, filter, sort, includeTotal })
215226

216227
return useQuery({
217228
queryKey: tableKeys.rows(tableId, paramsKey),
@@ -223,6 +234,7 @@ export function useTableRows({
223234
offset,
224235
filter,
225236
sort,
237+
includeTotal,
226238
signal,
227239
}),
228240
enabled: Boolean(workspaceId && tableId) && enabled,
@@ -393,7 +405,11 @@ export function useCreateTableRow({ workspaceId, tableId }: RowMutationContext)
393405
r.position >= row.position ? { ...r, position: r.position + 1 } : r
394406
)
395407
const rows: TableRow[] = [...shifted, row].sort((a, b) => a.position - b.position)
396-
return { ...old, rows, totalCount: old.totalCount + 1 }
408+
return {
409+
...old,
410+
rows,
411+
totalCount: old.totalCount === null ? null : old.totalCount + 1,
412+
}
397413
}
398414
)
399415
},

0 commit comments

Comments
 (0)