fix(tables): read the remaining row cells by own key in bulk validation, replace dedupe, and the upsert probe

Author: waleedlatif1

apps/sim/lib/table/bulk-update-concurrency.test.ts

@@ -35,7 +35,8 @@ vi.mock('@/lib/table/sql', () => ({
 
 vi.mock('@/lib/table/trigger', () => tableTriggerMock)
 
-vi.mock('@/lib/table/validation', () => ({
+vi.mock('@/lib/table/validation', async (importOriginal) => ({
+  cellOf: (await importOriginal<typeof import('@/lib/table/validation')>()).cellOf,
   validateRowSize: hoisted.validateRowSize,
   coerceRowToSchema: hoisted.coerceRowToSchema,
   coerceRowValues: vi.fn(),

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

@@ -1126,6 +1126,74 @@ describe('table row writes against real PostgreSQL', () => {
       expect(result.affectedCount).toBe(2)
     })
 
+    it('does not count a required legacy column named after a prototype key as supplied', async () => {
+      const table = await createTable([
+        { name: 'constructor', type: 'string', required: true },
+        { id: 'note', name: 'note', type: 'string' },
+      ])
+      await seedRows(table.id, [
+        { id: `${table.id}-a`, data: { constructor: 'a', note: 'n' }, orderKey: 'a0' },
+      ])
+
+      const result = await updateRowsByFilter(
+        table,
+        {
+          filter: { note: 'n' },
+          data: { note: 'patched' },
+          limit: 10,
+          secretProvenance: undefined,
+          capabilityGovernedUserId: null,
+        },
+        'prototype-key'
+      )
+
+      expect(result.affectedCount).toBe(1)
+    })
+
+    it('replaces rows that leave a unique legacy column named after a prototype key empty', async () => {
+      const table = await createTable([
+        { name: 'constructor', type: 'string', unique: true },
+        { id: 'note', name: 'note', type: 'string' },
+      ])
+
+      await replaceTableRows(
+        {
+          tableId: table.id,
+          workspaceId,
+          rows: [{ note: 'a' }, { note: 'b' }],
+          secretProvenance: undefined,
+        },
+        table,
+        'prototype-key'
+      )
+
+      const [{ count }] = await control<{ count: number }[]>`SELECT count(*)::int AS count
+        FROM user_table_rows WHERE table_id = ${table.id}`
+      expect(count).toBe(2)
+    })
+
+    it('refuses an upsert missing its conflict target named after a prototype key', async () => {
+      const table = await createTable([
+        { name: 'constructor', type: 'string', unique: true },
+        { id: 'note', name: 'note', type: 'string' },
+      ])
+
+      await expect(
+        upsertRow(
+          {
+            tableId: table.id,
+            workspaceId,
+            data: { note: 'a' },
+            conflictTarget: 'constructor',
+            secretProvenance: undefined,
+            capabilityGovernedUserId: null,
+          },
+          table,
+          'prototype-key'
+        )
+      ).rejects.toThrow(/requires a value for the conflict target/)
+    })
+
     it('refuses a bulk update writing one value to rows of a column made unique since its snapshot', async () => {
       const table = await seededTable()
       await updateColumnConstraints(

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

@@ -110,6 +110,7 @@ import type {
   UpsertRowData,
 } from '@/lib/table/types'
 import {
+  cellOf,
   checkBatchUniqueConstraintsDb,
   checkUniqueConstraintsDb,
   coerceRowToSchema,
@@ -611,7 +612,7 @@ export async function replaceTableRowsWithTx(
         // Coerced rows are keyed by column id, not display name — reading
         // `row[col.name]` silently misses renamed columns and lets dupes through.
         const colId = getColumnId(col)
-        const value = row[colId]
+        const value = cellOf(row, colId)
         if (value === null || value === undefined) continue
         const normalized = uniqueValueKey(value, col)
         const map = seen.get(colId)!
@@ -756,7 +757,7 @@ function resolveUpsertTarget(
  * matches a stored `123`).
  */
 function upsertConflictProbe(target: ColumnDefinition, row: RowData): SQL {
-  const targetValue = row[getColumnId(target)]
+  const targetValue = cellOf(row, getColumnId(target))
   if (targetValue === undefined || targetValue === null) {
     // Surface the display name, not the internal id — v1 callers pass a name.
     throw new OrchestrationError(
@@ -2130,7 +2131,7 @@ function validateBulkUpdatePatch(
   policy: UncoercibleValuePolicy | undefined
 ): void {
   const suppliedColumns = table.schema.columns.filter((column) => {
-    const value = patch[getColumnId(column)]
+    const value = cellOf(patch, getColumnId(column))
     return value !== null && value !== undefined
   })
   if (suppliedColumns.length === 0) return

apps/sim/lib/table/update-runner.test.ts

@@ -31,13 +31,16 @@ vi.mock('@/lib/table/rows/ordering', () => ({
 }))
 vi.mock('@/lib/table/events', () => tableEventsMock)
 vi.mock('@/lib/table/sql', () => ({ buildFilterClause: mockBuildFilterClause }))
-vi.mock('@/lib/table/validation', async (importOriginal) => ({
-  uniqueColumnsInPatch: (await importOriginal<typeof import('@/lib/table/validation')>())
-    .uniqueColumnsInPatch,
-  validateRowSize: mockValidateRowSize,
-  coerceRowToSchema: mockCoerceRowToSchema,
-  coerceRowValues: mockCoerceRowValues,
-}))
+vi.mock('@/lib/table/validation', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@/lib/table/validation')>()
+  return {
+    cellOf: actual.cellOf,
+    uniqueColumnsInPatch: actual.uniqueColumnsInPatch,
+    validateRowSize: mockValidateRowSize,
+    coerceRowToSchema: mockCoerceRowToSchema,
+    coerceRowValues: mockCoerceRowValues,
+  }
+})
 vi.mock('@/lib/table/constants', () => ({
   ...tableConstantsMock,
   TABLE_LIMITS: { ...tableConstantsMock.TABLE_LIMITS, DELETE_PAGE_SIZE: 2, UPDATE_BATCH_SIZE: 100 },