Skip to content

Commit 41c8193

Browse files
fix(tables): coerce row values to column types on write instead of failing (#4761)
* fix(tables): coerce row values to column types on write instead of failing * fix(tables): persist coerced values in upsert match + bulk update, normalize Date to ISO * fix(tables): guard date coercion against out-of-range values
1 parent 59792c0 commit 41c8193

5 files changed

Lines changed: 244 additions & 18 deletions

File tree

apps/sim/lib/table/__tests__/service-filter-threading.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ vi.mock('@/lib/table/workflow-columns', () => ({
3434
vi.mock('@/lib/table/validation', () => ({
3535
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
3636
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
37+
coerceRowToSchema: vi.fn(() => ({ valid: true, errors: [] })),
38+
coerceRowValues: vi.fn(),
3739
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
3840
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
3941
getUniqueColumns: vi.fn(() => []),

apps/sim/lib/table/__tests__/update-row.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ vi.mock('@sim/db', () => dbChainMock)
2020
vi.mock('@/lib/table/validation', () => ({
2121
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
2222
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
23+
coerceRowToSchema: vi.fn(() => ({ valid: true, errors: [] })),
24+
coerceRowValues: vi.fn(),
2325
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
2426
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
2527
getUniqueColumns: vi.fn(() => []),

apps/sim/lib/table/__tests__/validation.test.ts

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { describe, expect, it } from 'vitest'
55
import { TABLE_LIMITS } from '../constants'
66
import {
77
type ColumnDefinition,
8+
coerceRowToSchema,
9+
coerceRowValues,
810
getUniqueColumns,
911
type TableSchema,
1012
validateColumnDefinition,
@@ -277,6 +279,127 @@ describe('Validation', () => {
277279
})
278280
})
279281

282+
describe('coerceRowToSchema', () => {
283+
const schema: TableSchema = {
284+
columns: [
285+
{ name: 'name', type: 'string', required: true },
286+
{ name: 'age', type: 'number' },
287+
{ name: 'founded', type: 'number', required: true },
288+
{ name: 'active', type: 'boolean' },
289+
{ name: 'created', type: 'date' },
290+
{ name: 'metadata', type: 'json' },
291+
],
292+
}
293+
294+
it('coerces a numeric string to a number in place', () => {
295+
const data = { name: 'Acme', founded: '1999' }
296+
const result = coerceRowToSchema(data, schema)
297+
expect(result.valid).toBe(true)
298+
expect(data.founded).toBe(1999)
299+
})
300+
301+
it('nulls an un-coercible value for an optional number column', () => {
302+
const data = { name: 'Acme', founded: 2000, age: 'unknown' }
303+
const result = coerceRowToSchema(data, schema)
304+
expect(result.valid).toBe(true)
305+
expect(data.age).toBeNull()
306+
})
307+
308+
it('rejects an un-coercible value for a required number column', () => {
309+
const data = { name: 'Acme', founded: 'unknown' }
310+
const result = coerceRowToSchema(data, schema)
311+
expect(result.valid).toBe(false)
312+
expect(result.errors[0]).toContain('founded must be number')
313+
expect(data.founded).toBe('unknown')
314+
})
315+
316+
it('coerces a number to a string for a string column', () => {
317+
const data = { name: 12345, founded: 2000 }
318+
const result = coerceRowToSchema(data, schema)
319+
expect(result.valid).toBe(true)
320+
expect(data.name).toBe('12345')
321+
})
322+
323+
it('coerces "true"/"false" strings to booleans', () => {
324+
const data = { name: 'Acme', founded: 2000, active: 'false' }
325+
const result = coerceRowToSchema(data, schema)
326+
expect(result.valid).toBe(true)
327+
expect(data.active).toBe(false)
328+
})
329+
330+
it('coerces an epoch number to an ISO date string', () => {
331+
const epoch = Date.parse('2024-01-15T00:00:00Z')
332+
const data = { name: 'Acme', founded: 2000, created: epoch }
333+
const result = coerceRowToSchema(data, schema)
334+
expect(result.valid).toBe(true)
335+
expect(data.created).toBe(new Date(epoch).toISOString())
336+
})
337+
338+
it('coerces a Date instance to an ISO date string', () => {
339+
const date = new Date('2024-01-15T00:00:00Z')
340+
const data = { name: 'Acme', founded: 2000, created: date }
341+
const result = coerceRowToSchema(data, schema)
342+
expect(result.valid).toBe(true)
343+
expect(data.created).toBe(date.toISOString())
344+
})
345+
346+
it('nulls an out-of-range epoch number for an optional date column without throwing', () => {
347+
const data = { name: 'Acme', founded: 2000, created: 1e20 }
348+
const result = coerceRowToSchema(data, schema)
349+
expect(result.valid).toBe(true)
350+
expect(data.created).toBeNull()
351+
})
352+
353+
it('nulls an invalid Date instance for an optional date column without throwing', () => {
354+
const data = { name: 'Acme', founded: 2000, created: new Date('not-a-date') }
355+
const result = coerceRowToSchema(data, schema)
356+
expect(result.valid).toBe(true)
357+
expect(data.created).toBeNull()
358+
})
359+
360+
it('leaves already-correct values untouched and passes through json', () => {
361+
const data = { name: 'Acme', founded: 2000, metadata: { k: 'v' } }
362+
const result = coerceRowToSchema(data, schema)
363+
expect(result.valid).toBe(true)
364+
expect(data).toEqual({ name: 'Acme', founded: 2000, metadata: { k: 'v' } })
365+
})
366+
367+
it('still rejects a missing required field', () => {
368+
const data = { name: 'Acme' }
369+
const result = coerceRowToSchema(data, schema)
370+
expect(result.valid).toBe(false)
371+
expect(result.errors).toContain('Missing required field: founded')
372+
})
373+
})
374+
375+
describe('coerceRowValues', () => {
376+
const schema: TableSchema = {
377+
columns: [
378+
{ name: 'name', type: 'string', required: true },
379+
{ name: 'founded', type: 'number', required: true },
380+
{ name: 'age', type: 'number' },
381+
],
382+
}
383+
384+
it('coerces a partial patch in place without flagging absent required fields', () => {
385+
const patch = { age: '42' }
386+
coerceRowValues(patch, schema)
387+
expect(patch.age).toBe(42)
388+
})
389+
390+
it('nulls an un-coercible optional value in a patch', () => {
391+
const patch: { age: unknown } = { age: 'nope' }
392+
coerceRowValues(patch as never, schema)
393+
expect(patch.age).toBeNull()
394+
})
395+
396+
it('leaves an un-coercible required value in place for downstream validation', () => {
397+
const patch: { founded: unknown } = { founded: 'nope' }
398+
coerceRowValues(patch as never, schema)
399+
expect(patch.founded).toBe('nope')
400+
})
401+
})
402+
280403
describe('getUniqueColumns', () => {
281404
it('should return only columns with unique=true', () => {
282405
const schema: TableSchema = {

apps/sim/lib/table/service.ts

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ import type {
6161
import {
6262
checkBatchUniqueConstraintsDb,
6363
checkUniqueConstraintsDb,
64+
coerceRowToSchema,
65+
coerceRowValues,
6466
getUniqueColumns,
65-
validateRowAgainstSchema,
6667
validateRowSize,
6768
validateTableName,
6869
validateTableSchema,
@@ -913,7 +914,7 @@ export async function insertRow(
913914
}
914915

915916
// Validate against schema
916-
const schemaValidation = validateRowAgainstSchema(data.data, table.schema)
917+
const schemaValidation = coerceRowToSchema(data.data, table.schema)
917918
if (!schemaValidation.valid) {
918919
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
919920
}
@@ -1060,7 +1061,7 @@ export async function batchInsertRowsWithTx(
10601061
throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`)
10611062
}
10621063

1063-
const schemaValidation = validateRowAgainstSchema(row, table.schema)
1064+
const schemaValidation = coerceRowToSchema(row, table.schema)
10641065
if (!schemaValidation.valid) {
10651066
throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`)
10661067
}
@@ -1201,7 +1202,7 @@ export async function replaceTableRowsWithTx(
12011202
throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`)
12021203
}
12031204

1204-
const schemaValidation = validateRowAgainstSchema(row, table.schema)
1205+
const schemaValidation = coerceRowToSchema(row, table.schema)
12051206
if (!schemaValidation.valid) {
12061207
throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`)
12071208
}
@@ -1331,22 +1332,24 @@ export async function upsertRow(
13311332
)
13321333
}
13331334

1334-
const targetValue = data.data[targetColumnName]
1335-
if (targetValue === undefined || targetValue === null) {
1336-
throw new Error(`Upsert requires a value for the conflict target column "${targetColumnName}"`)
1337-
}
1338-
13391335
// Validate row data
13401336
const sizeValidation = validateRowSize(data.data)
13411337
if (!sizeValidation.valid) {
13421338
throw new Error(sizeValidation.errors.join(', '))
13431339
}
13441340

1345-
const schemaValidation = validateRowAgainstSchema(data.data, schema)
1341+
const schemaValidation = coerceRowToSchema(data.data, schema)
13461342
if (!schemaValidation.valid) {
13471343
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
13481344
}
13491345

1346+
// Read the conflict-target value *after* coercion so `matchFilter` branches on
1347+
// the persisted type (e.g. a coerced `"123"` → `123` matches existing rows).
1348+
const targetValue = data.data[targetColumnName]
1349+
if (targetValue === undefined || targetValue === null) {
1350+
throw new Error(`Upsert requires a value for the conflict target column "${targetColumnName}"`)
1351+
}
1352+
13501353
// `data->` and `data->>` accept the JSON key as a parameterized text value;
13511354
// no need for `sql.raw` interpolation.
13521355
const matchFilter =
@@ -1957,7 +1960,7 @@ export async function updateRow(
19571960
}
19581961

19591962
// Validate against schema
1960-
const schemaValidation = validateRowAgainstSchema(mergedData, table.schema)
1963+
const schemaValidation = coerceRowToSchema(mergedData, table.schema)
19611964
if (!schemaValidation.valid) {
19621965
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
19631966
}
@@ -2167,6 +2170,12 @@ export async function updateRowsByFilter(
21672170
return { affectedCount: 0, affectedRowIds: [] }
21682171
}
21692172

2173+
// Coerce the patch itself in place — the write below persists `data.data`
2174+
// (as `patchJson`), so coercing only the per-row merged copies would be
2175+
// discarded. The merged validation in the loop still enforces required
2176+
// fields against the full row.
2177+
coerceRowValues(data.data, table.schema)
2178+
21702179
for (const row of matchingRows) {
21712180
const existingData = row.data as RowData
21722181
const mergedData = { ...existingData, ...data.data }
@@ -2176,7 +2185,7 @@ export async function updateRowsByFilter(
21762185
throw new Error(`Row ${row.id}: ${sizeValidation.errors.join(', ')}`)
21772186
}
21782187

2179-
const schemaValidation = validateRowAgainstSchema(mergedData, table.schema)
2188+
const schemaValidation = coerceRowToSchema(mergedData, table.schema)
21802189
if (!schemaValidation.valid) {
21812190
throw new Error(`Row ${row.id}: ${schemaValidation.errors.join(', ')}`)
21822191
}
@@ -2334,7 +2343,7 @@ export async function batchUpdateRows(
23342343
throw new Error(`Row ${update.rowId}: ${sizeValidation.errors.join(', ')}`)
23352344
}
23362345

2337-
const schemaValidation = validateRowAgainstSchema(merged, table.schema)
2346+
const schemaValidation = coerceRowToSchema(merged, table.schema)
23382347
if (!schemaValidation.valid) {
23392348
throw new Error(`Row ${update.rowId}: ${schemaValidation.errors.join(', ')}`)
23402349
}
@@ -3247,8 +3256,8 @@ export async function updateWorkflowGroup(
32473256

32483257
// Resolve the new leaf type for each remap so the column's declared type
32493258
// matches what the new mapping produces. Without this, a string→number
3250-
// remap would keep `type: 'string'` and validateRowAgainstSchema would
3251-
// reject every backfilled value.
3259+
// remap would keep `type: 'string'` and coerceRowToSchema would coerce
3260+
// every backfilled value toward the wrong type.
32523261
try {
32533262
const [
32543263
{ loadWorkflowFromNormalizedTables },

apps/sim/lib/table/validation.ts

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { userTableRows } from '@sim/db/schema'
77
import { and, eq, or, sql } from 'drizzle-orm'
88
import { NextResponse } from 'next/server'
99
import { COLUMN_TYPES, NAME_PATTERN, TABLE_LIMITS } from './constants'
10-
import type { ColumnDefinition, RowData, TableSchema, ValidationResult } from './types'
10+
import type { ColumnDefinition, JsonValue, RowData, TableSchema, ValidationResult } from './types'
1111

1212
export type { ColumnDefinition, TableSchema, ValidationResult }
1313

@@ -57,7 +57,7 @@ export async function validateRowData(
5757
}
5858
}
5959

60-
const schemaValidation = validateRowAgainstSchema(rowData, schema)
60+
const schemaValidation = coerceRowToSchema(rowData, schema)
6161
if (!schemaValidation.valid) {
6262
return {
6363
valid: false,
@@ -105,7 +105,7 @@ export async function validateBatchRows(
105105
continue
106106
}
107107

108-
const schemaValidation = validateRowAgainstSchema(rowData, schema)
108+
const schemaValidation = coerceRowToSchema(rowData, schema)
109109
if (!schemaValidation.valid) {
110110
errors.push({ row: i, errors: schemaValidation.errors })
111111
}
@@ -255,6 +255,96 @@ export function validateRowAgainstSchema(data: RowData, schema: TableSchema): Va
255255
return { valid: errors.length === 0, errors }
256256
}
257257

258+
/**
259+
* Attempts to coerce a non-null value to a column's declared type. Returns the
260+
* coerced value when the value already matches or can be converted without
261+
* ambiguity (e.g. the string `"1999"` to the number `1999`), and `ok: false`
262+
* when no safe conversion exists.
263+
*/
264+
function coerceValueToColumnType(
265+
value: JsonValue,
266+
type: ColumnDefinition['type']
267+
): { ok: true; value: JsonValue } | { ok: false } {
268+
switch (type) {
269+
case 'string':
270+
if (typeof value === 'string') return { ok: true, value }
271+
if (typeof value === 'number' || typeof value === 'boolean') {
272+
return { ok: true, value: String(value) }
273+
}
274+
return { ok: false }
275+
case 'number':
276+
if (typeof value === 'number') {
277+
return Number.isFinite(value) ? { ok: true, value } : { ok: false }
278+
}
279+
if (typeof value === 'string' && value.trim() !== '') {
280+
const parsed = Number(value)
281+
return Number.isFinite(parsed) ? { ok: true, value: parsed } : { ok: false }
282+
}
283+
return { ok: false }
284+
case 'boolean':
285+
if (typeof value === 'boolean') return { ok: true, value }
286+
if (typeof value === 'string') {
287+
const normalized = value.trim().toLowerCase()
288+
if (normalized === 'true') return { ok: true, value: true }
289+
if (normalized === 'false') return { ok: true, value: false }
290+
}
291+
return { ok: false }
292+
case 'date': {
293+
if (typeof value === 'string' && !Number.isNaN(Date.parse(value))) return { ok: true, value }
294+
// Date instances and epoch numbers may still be out of the representable
295+
// range (>±8.64e15ms) — guard `toISOString()`, which throws RangeError on
296+
// an Invalid Date, so an over-range value degrades to `{ ok: false }`
297+
// rather than crashing the write.
298+
const date =
299+
value instanceof Date ? value : typeof value === 'number' ? new Date(value) : null
300+
if (date && !Number.isNaN(date.getTime())) return { ok: true, value: date.toISOString() }
301+
return { ok: false }
302+
}
303+
default:
304+
return { ok: true, value }
305+
}
306+
}
307+
308+
/**
309+
* Coerces each present value in `data` toward its column's declared type **in
310+
* place**. Values that already match are untouched; unambiguous conversions
311+
* (e.g. `"1999"` → `1999`) are applied; values that cannot be coerced are set to
312+
* `null` when the column is optional, or left in place when required (so a
313+
* subsequent {@link validateRowAgainstSchema} reports them).
314+
*
315+
* Operates per-present-column, so it is safe on a partial patch (columns absent
316+
* from `data` are skipped — it never invents a missing-required-field error).
317+
*/
318+
export function coerceRowValues(data: RowData, schema: TableSchema): void {
319+
for (const column of schema.columns) {
320+
const value = data[column.name]
321+
if (value === null || value === undefined) continue
322+
323+
const coerced = coerceValueToColumnType(value, column.type)
324+
if (coerced.ok) {
325+
data[column.name] = coerced.value
326+
} else if (!column.required) {
327+
data[column.name] = null
328+
}
329+
}
330+
}
331+
332+
/**
333+
* Coerces a full row toward its schema **in place** (see {@link coerceRowValues})
334+
* then validates the result.
335+
*
336+
* This is the write-path entry point — callers that persist a complete row use
337+
* it instead of {@link validateRowAgainstSchema} so a single off-type field (a
338+
* tool returning `"unknown"` for a numeric column, say) nulls that one cell
339+
* rather than failing the entire row write. Callers persisting only a partial
340+
* patch should use {@link coerceRowValues} on the patch and validate the merged
341+
* row separately.
342+
*/
343+
export function coerceRowToSchema(data: RowData, schema: TableSchema): ValidationResult {
344+
coerceRowValues(data, schema)
345+
return validateRowAgainstSchema(data, schema)
346+
}
347+
258348
/** Validates row data size is within limits. */
259349
export function validateRowSize(data: RowData): ValidationResult {
260350
const size = JSON.stringify(data).length

0 commit comments

Comments
 (0)