Skip to content

Commit 6f48eb6

Browse files
fix(tables): guard import batches in-transaction and split paste by verb
- Re-assert the insert lock inside each import batch's insert transaction, under the same advisory lock updateTableLocks takes. Reversing the earlier "let the file finish" call: an admin can lift the lock to clean up a partial import, so honouring it beats letting the rest of the file land - Gate paste per verb. Overwriting existing rows is an update and extending past the last row is a full-row insert, so a paste-append still works on an append-only table while an overwrite explains itself. Refuses the whole paste rather than applying half of it - Space opens the same row editor as double-click, so it now follows the update lock instead of filling in a form that 423s on save
1 parent 0f13ee2 commit 6f48eb6

4 files changed

Lines changed: 48 additions & 12 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,8 @@ export function TableGrid({
572572
canEditCellRef.current = canEditCell
573573
const canManualAddRowRef = useRef(canManualAddRow)
574574
canManualAddRowRef.current = canManualAddRow
575+
const canInsertFullRowRef = useRef(canInsertFullRow)
576+
canInsertFullRowRef.current = canInsertFullRow
575577
// Read by the closure-free double-click handler to tell "locked" apart from
576578
// "no write permission" — only the former gets the explanation modal.
577579
const updateLockedRef = useRef(locks?.updateLocked)
@@ -2346,6 +2348,12 @@ export function TableGrid({
23462348
if (e.key === ' ' && !e.shiftKey) {
23472349
if (!canEditRef.current) return
23482350
e.preventDefault()
2351+
// Space opens the same row editor as double-click, so it follows the
2352+
// update lock too — otherwise the form fills in and only 423s on save.
2353+
if (updateLockedRef.current) {
2354+
onBlockedActionRef.current('edit-cell')
2355+
return
2356+
}
23492357
const row = currentRows[anchor.rowIndex]
23502358
if (row) {
23512359
onOpenRowModalRef.current(row)
@@ -2840,10 +2848,7 @@ export function TableGrid({
28402848
const handlePaste = (e: ClipboardEvent) => {
28412849
const tag = (e.target as HTMLElement).tagName
28422850
if (tag === 'INPUT' || tag === 'TEXTAREA') return
2843-
// Paste overwrites the selected cells (an edit). Under an update lock we
2844-
// block it wholesale — the append-into-new-rows variant is a rarer path
2845-
// the user can still reach via the Add row button.
2846-
if (!canEditCellRef.current) return
2851+
if (!canEditRef.current) return
28472852

28482853
const currentAnchor = selectionAnchorRef.current
28492854
if (!currentAnchor || editingCellRef.current) return
@@ -2899,6 +2904,20 @@ export function TableGrid({
28992904
}
29002905
}
29012906

2907+
// A paste can do two different things at once: overwrite existing rows
2908+
// (an update) and extend past the last row (a full-row insert). Gate each
2909+
// against its own lock, and refuse the whole paste rather than applying
2910+
// half of it — so a full-row paste-append still works on an append-only
2911+
// table, while an overwrite on that same table explains itself.
2912+
if (updateBatch.length > 0 && !canEditCellRef.current) {
2913+
onBlockedActionRef.current('edit-cell')
2914+
return
2915+
}
2916+
if (createBatchRows.length > 0 && !canInsertFullRowRef.current) {
2917+
onBlockedActionRef.current('add-row')
2918+
return
2919+
}
2920+
29022921
if (updateBatch.length > 0) {
29032922
batchUpdateRef.current({ updates: updateBatch })
29042923
pushUndoRef.current({

apps/sim/lib/table/import-data.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing'
1313
import { CSV_MAX_BATCH_SIZE } from '@/lib/table/import'
1414
import { assertRowDelete, assertRowInsert, assertSchemaMutable } from '@/lib/table/mutation-locks'
1515
import { nKeysBetween } from '@/lib/table/order-key'
16-
import { acquireRowOrderLock } from '@/lib/table/rows/ordering'
16+
import {
17+
acquireRowOrderLock,
18+
guardBatch,
19+
type MutationRevalidator,
20+
} from '@/lib/table/rows/ordering'
1721
import { batchInsertRowsWithTx, replaceTableRowsWithTx } from '@/lib/table/rows/service'
1822
import { addTableColumnsWithTx, auditTableColumnsAdded } from '@/lib/table/service'
1923
import type {
@@ -61,7 +65,9 @@ export interface BulkImportBatch {
6165
export async function bulkInsertImportBatch(
6266
data: BulkImportBatch,
6367
table: TableDefinition,
64-
requestId: string
68+
requestId: string,
69+
/** Re-asserts the insert lock inside the write transaction. See {@link guardBatch}. */
70+
revalidate?: MutationRevalidator
6571
): Promise<{ inserted: number; lastOrderKey: string | null }> {
6672
assertRowInsert(table)
6773

@@ -107,7 +113,10 @@ export async function bulkInsertImportBatch(
107113
...(data.userId ? { createdBy: data.userId } : {}),
108114
}))
109115

110-
await db.insert(userTableRows).values(rowsToInsert)
116+
await db.transaction(async (trx) => {
117+
await guardBatch(trx, data.tableId, revalidate)
118+
await trx.insert(userTableRows).values(rowsToInsert)
119+
})
111120
logger.info(`[${requestId}] Bulk-imported ${rowsToInsert.length} rows into table ${data.tableId}`)
112121
return {
113122
inserted: rowsToInsert.length,

apps/sim/lib/table/import-runner.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
} from '@/lib/table/import-data'
3030
import { markJobFailed, markJobReady, updateJobProgress } from '@/lib/table/jobs/service'
3131
import { assertRowDelete, assertRowInsert } from '@/lib/table/mutation-locks'
32+
import type { DbTransaction } from '@/lib/table/planner'
3233
import { nextImportStartOrderKey, nextImportStartPosition } from '@/lib/table/rows/ordering'
3334
import { getTableById } from '@/lib/table/service'
3435
import { deleteFile, downloadFileStream, headObject } from '@/lib/uploads/core/storage-service'
@@ -103,12 +104,18 @@ export async function runTableImport(payload: TableImportPayload): Promise<void>
103104
// fails up front instead of after `deleteAllTableRows` has already wiped it.
104105
// (The sync replace path gets this for free from `replaceTableRowsWithTx`,
105106
// which asserts both in one place; this path deletes and inserts separately.)
106-
// Checked once, unlike the delete/update runners which re-check per page: a
107-
// lock landing mid-import should let the file finish, since a half-imported
108-
// table has to be cleaned up with the very deletes the lock now forbids.
109107
assertRowInsert(table)
110108
if (mode === 'replace') assertRowDelete(table)
111109

110+
// Re-asserted inside every batch's insert transaction, under the same
111+
// advisory lock `updateTableLocks` writes with, so enabling the insert lock
112+
// mid-import stops it at the next batch instead of letting the rest of the
113+
// file through. Rows already committed stay — as with an explicit cancel.
114+
const revalidateInsert = async (trx: DbTransaction) => {
115+
const fresh = await getTableById(tableId, { tx: trx, includeArchived: true })
116+
if (fresh) assertRowInsert(fresh)
117+
}
118+
112119
// Total byte size for the progress estimate — a cheap HEAD, no download. May be null on
113120
// the local dev provider, in which case the bar stays indeterminate (rows still show).
114121
const totalBytes = (await headObject(fileKey, 'workspace'))?.size ?? 0
@@ -244,7 +251,8 @@ export async function runTableImport(payload: TableImportPayload): Promise<void>
244251
afterOrderKey: lastOrderKey,
245252
},
246253
{ ...table, schema },
247-
requestId
254+
requestId,
255+
revalidateInsert
248256
)
249257
notifyTableRowUsage({
250258
workspaceId,

apps/sim/lib/table/rows/ordering.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ export type MutationRevalidator = (trx: DbTransaction) => Promise<void>
409409
* waits for this batch to finish. Without it, the caller's proof would only
410410
* describe the lock state at some earlier point in the run.
411411
*/
412-
async function guardBatch(
412+
export async function guardBatch(
413413
trx: DbTransaction,
414414
tableId: string,
415415
revalidate: MutationRevalidator | undefined

0 commit comments

Comments
 (0)