Skip to content

Commit fe6ea03

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(tables): coerce row values to column types on write instead of failing
1 parent 92fd17c commit fe6ea03

5 files changed

Lines changed: 161 additions & 13 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ 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: [] })),
3738
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
3839
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
3940
getUniqueColumns: vi.fn(() => []),

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ 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: [] })),
2324
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
2425
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
2526
getUniqueColumns: vi.fn(() => []),

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { describe, expect, it } from 'vitest'
55
import { TABLE_LIMITS } from '../constants'
66
import {
77
type ColumnDefinition,
8+
coerceRowToSchema,
89
getUniqueColumns,
910
type TableSchema,
1011
validateColumnDefinition,
@@ -277,6 +278,77 @@ describe('Validation', () => {
277278
})
278279
})
279280

281+
describe('coerceRowToSchema', () => {
282+
const schema: TableSchema = {
283+
columns: [
284+
{ name: 'name', type: 'string', required: true },
285+
{ name: 'age', type: 'number' },
286+
{ name: 'founded', type: 'number', required: true },
287+
{ name: 'active', type: 'boolean' },
288+
{ name: 'created', type: 'date' },
289+
{ name: 'metadata', type: 'json' },
290+
],
291+
}
292+
293+
it('coerces a numeric string to a number in place', () => {
294+
const data = { name: 'Acme', founded: '1999' }
295+
const result = coerceRowToSchema(data, schema)
296+
expect(result.valid).toBe(true)
297+
expect(data.founded).toBe(1999)
298+
})
299+
300+
it('nulls an un-coercible value for an optional number column', () => {
301+
const data = { name: 'Acme', founded: 2000, age: 'unknown' }
302+
const result = coerceRowToSchema(data, schema)
303+
expect(result.valid).toBe(true)
304+
expect(data.age).toBeNull()
305+
})
306+
307+
it('rejects an un-coercible value for a required number column', () => {
308+
const data = { name: 'Acme', founded: 'unknown' }
309+
const result = coerceRowToSchema(data, schema)
310+
expect(result.valid).toBe(false)
311+
expect(result.errors[0]).toContain('founded must be number')
312+
expect(data.founded).toBe('unknown')
313+
})
314+
315+
it('coerces a number to a string for a string column', () => {
316+
const data = { name: 12345, founded: 2000 }
317+
const result = coerceRowToSchema(data, schema)
318+
expect(result.valid).toBe(true)
319+
expect(data.name).toBe('12345')
320+
})
321+
322+
it('coerces "true"/"false" strings to booleans', () => {
323+
const data = { name: 'Acme', founded: 2000, active: 'false' }
324+
const result = coerceRowToSchema(data, schema)
325+
expect(result.valid).toBe(true)
326+
expect(data.active).toBe(false)
327+
})
328+
329+
it('coerces an epoch number to an ISO date string', () => {
330+
const epoch = Date.parse('2024-01-15T00:00:00Z')
331+
const data = { name: 'Acme', founded: 2000, created: epoch }
332+
const result = coerceRowToSchema(data, schema)
333+
expect(result.valid).toBe(true)
334+
expect(data.created).toBe(new Date(epoch).toISOString())
335+
})
336+
337+
it('leaves already-correct values untouched and passes through json', () => {
338+
const data = { name: 'Acme', founded: 2000, metadata: { k: 'v' } }
339+
const result = coerceRowToSchema(data, schema)
340+
expect(result.valid).toBe(true)
341+
expect(data).toEqual({ name: 'Acme', founded: 2000, metadata: { k: 'v' } })
342+
})
343+
344+
it('still rejects a missing required field', () => {
345+
const data = { name: 'Acme' }
346+
const result = coerceRowToSchema(data, schema)
347+
expect(result.valid).toBe(false)
348+
expect(result.errors).toContain('Missing required field: founded')
349+
})
350+
})
351+
280352
describe('getUniqueColumns', () => {
281353
it('should return only columns with unique=true', () => {
282354
const schema: TableSchema = {

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ import type {
6161
import {
6262
checkBatchUniqueConstraintsDb,
6363
checkUniqueConstraintsDb,
64+
coerceRowToSchema,
6465
getUniqueColumns,
65-
validateRowAgainstSchema,
6666
validateRowSize,
6767
validateTableName,
6868
validateTableSchema,
@@ -913,7 +913,7 @@ export async function insertRow(
913913
}
914914

915915
// Validate against schema
916-
const schemaValidation = validateRowAgainstSchema(data.data, table.schema)
916+
const schemaValidation = coerceRowToSchema(data.data, table.schema)
917917
if (!schemaValidation.valid) {
918918
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
919919
}
@@ -1060,7 +1060,7 @@ export async function batchInsertRowsWithTx(
10601060
throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`)
10611061
}
10621062

1063-
const schemaValidation = validateRowAgainstSchema(row, table.schema)
1063+
const schemaValidation = coerceRowToSchema(row, table.schema)
10641064
if (!schemaValidation.valid) {
10651065
throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`)
10661066
}
@@ -1201,7 +1201,7 @@ export async function replaceTableRowsWithTx(
12011201
throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`)
12021202
}
12031203

1204-
const schemaValidation = validateRowAgainstSchema(row, table.schema)
1204+
const schemaValidation = coerceRowToSchema(row, table.schema)
12051205
if (!schemaValidation.valid) {
12061206
throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`)
12071207
}
@@ -1342,7 +1342,7 @@ export async function upsertRow(
13421342
throw new Error(sizeValidation.errors.join(', '))
13431343
}
13441344

1345-
const schemaValidation = validateRowAgainstSchema(data.data, schema)
1345+
const schemaValidation = coerceRowToSchema(data.data, schema)
13461346
if (!schemaValidation.valid) {
13471347
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
13481348
}
@@ -1957,7 +1957,7 @@ export async function updateRow(
19571957
}
19581958

19591959
// Validate against schema
1960-
const schemaValidation = validateRowAgainstSchema(mergedData, table.schema)
1960+
const schemaValidation = coerceRowToSchema(mergedData, table.schema)
19611961
if (!schemaValidation.valid) {
19621962
throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`)
19631963
}
@@ -2176,7 +2176,7 @@ export async function updateRowsByFilter(
21762176
throw new Error(`Row ${row.id}: ${sizeValidation.errors.join(', ')}`)
21772177
}
21782178

2179-
const schemaValidation = validateRowAgainstSchema(mergedData, table.schema)
2179+
const schemaValidation = coerceRowToSchema(mergedData, table.schema)
21802180
if (!schemaValidation.valid) {
21812181
throw new Error(`Row ${row.id}: ${schemaValidation.errors.join(', ')}`)
21822182
}
@@ -2334,7 +2334,7 @@ export async function batchUpdateRows(
23342334
throw new Error(`Row ${update.rowId}: ${sizeValidation.errors.join(', ')}`)
23352335
}
23362336

2337-
const schemaValidation = validateRowAgainstSchema(merged, table.schema)
2337+
const schemaValidation = coerceRowToSchema(merged, table.schema)
23382338
if (!schemaValidation.valid) {
23392339
throw new Error(`Row ${update.rowId}: ${schemaValidation.errors.join(', ')}`)
23402340
}
@@ -3247,8 +3247,8 @@ export async function updateWorkflowGroup(
32473247

32483248
// Resolve the new leaf type for each remap so the column's declared type
32493249
// 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.
3250+
// remap would keep `type: 'string'` and coerceRowToSchema would coerce
3251+
// every backfilled value toward the wrong type.
32523252
try {
32533253
const [
32543254
{ loadWorkflowFromNormalizedTables },

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

Lines changed: 77 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,80 @@ 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 (value instanceof Date) return { ok: true, value }
294+
if (typeof value === 'string' && !Number.isNaN(Date.parse(value))) return { ok: true, value }
295+
if (typeof value === 'number' && Number.isFinite(value)) {
296+
return { ok: true, value: new Date(value).toISOString() }
297+
}
298+
return { ok: false }
299+
default:
300+
return { ok: true, value }
301+
}
302+
}
303+
304+
/**
305+
* Coerces each value in `data` toward its column's declared type **in place**,
306+
* then validates the result. Values that already match are untouched;
307+
* unambiguous conversions (e.g. `"1999"` → `1999`) are applied; values that
308+
* cannot be coerced are set to `null` when the column is optional, or left in
309+
* place to fail validation when the column is required.
310+
*
311+
* This is the write-path entry point — callers that persist rows use it instead
312+
* of {@link validateRowAgainstSchema} so a single off-type field (a tool
313+
* returning `"unknown"` for a numeric column, say) nulls that one cell rather
314+
* than failing the entire row write.
315+
*/
316+
export function coerceRowToSchema(data: RowData, schema: TableSchema): ValidationResult {
317+
for (const column of schema.columns) {
318+
const value = data[column.name]
319+
if (value === null || value === undefined) continue
320+
321+
const coerced = coerceValueToColumnType(value, column.type)
322+
if (coerced.ok) {
323+
data[column.name] = coerced.value
324+
} else if (!column.required) {
325+
data[column.name] = null
326+
}
327+
}
328+
329+
return validateRowAgainstSchema(data, schema)
330+
}
331+
258332
/** Validates row data size is within limits. */
259333
export function validateRowSize(data: RowData): ValidationResult {
260334
const size = JSON.stringify(data).length

0 commit comments

Comments
 (0)