Skip to content

Commit a0d9e4d

Browse files
authored
fix(tables): type-aware SQL casts for range filters on date columns (#4657)
* fix(tables): type-aware SQL casts for range filters on date columns * improvement(table): tighten filter-cast types & workspace guards - Drop redundant tableId/workspaceId from BulkUpdateData and BulkDeleteData; service uses table.id / table.workspaceId so column metadata and DB scope can't drift apart. - Add missing workspace-id guards to copilot user-table cases (insert_row, batch_insert_rows, update_row, batch_update_rows, rename); collapse duplicated rename check. - Add service-level integration tests that buildFilterClause/buildSortClause receive table.schema.columns from queryRows, updateRowsByFilter, deleteRowsByFilter. * improvement(table): cast jsonb date filters/sorts to timestamptz ::timestamp strips timezone offsets from ISO strings, making comparisons depend on the server TimeZone setting. ::timestamptz preserves the offset so chronological comparisons are correct regardless of server config. * improvement(table): correct JSDoc examples for required columns arg * improvement(table): validate range operator value types at SQL builder
1 parent 0c1167d commit a0d9e4d

11 files changed

Lines changed: 644 additions & 316 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Rou
6262
let firstJsonRow = true
6363
while (true) {
6464
const result = await queryRows(
65-
tableId,
66-
table.workspaceId,
65+
table,
6766
{ limit: EXPORT_BATCH_SIZE, offset, includeTotal: false },
6867
requestId
6968
)

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,14 @@ export const GET = withRouteHandler(
266266
eq(userTableRows.workspaceId, validated.workspaceId),
267267
]
268268

269+
const schema = table.schema as TableSchema
270+
269271
if (validated.filter) {
270-
const filterClause = buildFilterClause(validated.filter as Filter, USER_TABLE_ROWS_SQL_NAME)
272+
const filterClause = buildFilterClause(
273+
validated.filter as Filter,
274+
USER_TABLE_ROWS_SQL_NAME,
275+
schema.columns
276+
)
271277
if (filterClause) {
272278
baseConditions.push(filterClause)
273279
}
@@ -286,7 +292,6 @@ export const GET = withRouteHandler(
286292
.where(and(...baseConditions))
287293

288294
if (validated.sort) {
289-
const schema = table.schema as TableSchema
290295
const sortClause = buildSortClause(validated.sort, USER_TABLE_ROWS_SQL_NAME, schema.columns)
291296
if (sortClause) {
292297
query = query.orderBy(sortClause) as typeof query
@@ -388,14 +393,12 @@ export const PUT = withRouteHandler(
388393
}
389394

390395
const result = await updateRowsByFilter(
396+
table,
391397
{
392-
tableId,
393398
filter: validated.filter as Filter,
394399
data: validated.data as RowData,
395400
limit: validated.limit,
396-
workspaceId: validated.workspaceId,
397401
},
398-
table,
399402
requestId
400403
)
401404

@@ -503,11 +506,10 @@ export const DELETE = withRouteHandler(
503506
}
504507

505508
const result = await deleteRowsByFilter(
509+
table,
506510
{
507-
tableId,
508511
filter: validated.filter as Filter,
509512
limit: validated.limit,
510-
workspaceId: validated.workspaceId,
511513
},
512514
requestId
513515
)

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,14 @@ export const GET = withRouteHandler(async (request: NextRequest, context: TableR
158158
eq(userTableRows.workspaceId, validated.workspaceId),
159159
]
160160

161+
const schema = table.schema as TableSchema
162+
161163
if (validated.filter) {
162-
const filterClause = buildFilterClause(validated.filter as Filter, USER_TABLE_ROWS_SQL_NAME)
164+
const filterClause = buildFilterClause(
165+
validated.filter as Filter,
166+
USER_TABLE_ROWS_SQL_NAME,
167+
schema.columns
168+
)
163169
if (filterClause) {
164170
baseConditions.push(filterClause)
165171
}
@@ -177,7 +183,6 @@ export const GET = withRouteHandler(async (request: NextRequest, context: TableR
177183
.where(and(...baseConditions))
178184

179185
if (validated.sort) {
180-
const schema = table.schema as TableSchema
181186
const sortClause = buildSortClause(validated.sort, USER_TABLE_ROWS_SQL_NAME, schema.columns)
182187
if (sortClause) {
183188
query = query.orderBy(sortClause) as typeof query
@@ -378,14 +383,12 @@ export const PUT = withRouteHandler(async (request: NextRequest, context: TableR
378383
}
379384

380385
const result = await updateRowsByFilter(
386+
table,
381387
{
382-
tableId,
383388
filter: validated.filter as Filter,
384389
data: validated.data as RowData,
385390
limit: validated.limit,
386-
workspaceId: validated.workspaceId,
387391
},
388-
table,
389392
requestId
390393
)
391394

@@ -484,11 +487,10 @@ export const DELETE = withRouteHandler(
484487
}
485488

486489
const result = await deleteRowsByFilter(
490+
table,
487491
{
488-
tableId,
489492
filter: validated.filter as Filter,
490493
limit: validated.limit,
491-
workspaceId: validated.workspaceId,
492494
},
493495
requestId
494496
)

apps/sim/lib/copilot/tools/handlers/function-execute.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ async function resolveInputFiles(
6262
for (const tableId of inputTables) {
6363
if (typeof tableId !== 'string') continue
6464
const table = await getTableById(tableId)
65-
if (!table) {
65+
if (!table || table.workspaceId !== workspaceId) {
6666
logger.warn('Input table not found', { tableId })
6767
continue
6868
}
69-
const rows = await queryRows(tableId, workspaceId, {}, 'copilot-fn-exec')
69+
const rows = await queryRows(table, {}, 'copilot-fn-exec')
7070
if (!rows.rows?.length) continue
7171

7272
const allKeys = new Set<string>()

apps/sim/lib/copilot/tools/server/table/user-table.ts

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
367367
}
368368

369369
const table = await getTableById(args.tableId)
370-
if (!table) {
370+
if (!table || table.workspaceId !== workspaceId) {
371371
return { success: false, message: `Table not found: ${args.tableId}` }
372372
}
373373

@@ -418,7 +418,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
418418
}
419419

420420
const table = await getTableById(args.tableId)
421-
if (!table) {
421+
if (!table || table.workspaceId !== workspaceId) {
422422
return { success: false, message: `Table not found: ${args.tableId}` }
423423
}
424424

@@ -474,10 +474,14 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
474474
return { success: false, message: 'Workspace ID is required' }
475475
}
476476

477+
const table = await getTableById(args.tableId)
478+
if (!table || table.workspaceId !== workspaceId) {
479+
return { success: false, message: `Table not found: ${args.tableId}` }
480+
}
481+
477482
const requestId = generateId().slice(0, 8)
478483
const result = await queryRows(
479-
args.tableId,
480-
workspaceId,
484+
table,
481485
{
482486
filter: args.filter,
483487
sort: args.sort,
@@ -509,7 +513,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
509513
}
510514

511515
const table = await getTableById(args.tableId)
512-
if (!table) {
516+
if (!table || table.workspaceId !== workspaceId) {
513517
return { success: false, message: `Table not found: ${args.tableId}` }
514518
}
515519

@@ -569,21 +573,19 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
569573
}
570574

571575
const table = await getTableById(args.tableId)
572-
if (!table) {
576+
if (!table || table.workspaceId !== workspaceId) {
573577
return { success: false, message: `Table not found: ${args.tableId}` }
574578
}
575579

576580
const requestId = generateId().slice(0, 8)
577581
assertNotAborted()
578582
const result = await updateRowsByFilter(
583+
table,
579584
{
580-
tableId: args.tableId,
581585
filter: args.filter,
582586
data: args.data,
583587
limit: args.limit,
584-
workspaceId,
585588
},
586-
table,
587589
requestId
588590
)
589591

@@ -605,14 +607,18 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
605607
return { success: false, message: 'Workspace ID is required' }
606608
}
607609

610+
const table = await getTableById(args.tableId)
611+
if (!table || table.workspaceId !== workspaceId) {
612+
return { success: false, message: `Table not found: ${args.tableId}` }
613+
}
614+
608615
const requestId = generateId().slice(0, 8)
609616
assertNotAborted()
610617
const result = await deleteRowsByFilter(
618+
table,
611619
{
612-
tableId: args.tableId,
613620
filter: args.filter,
614621
limit: args.limit,
615-
workspaceId,
616622
},
617623
requestId
618624
)
@@ -664,7 +670,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
664670
}
665671

666672
const table = await getTableById(args.tableId)
667-
if (!table) {
673+
if (!table || table.workspaceId !== workspaceId) {
668674
return { success: false, message: `Table not found: ${args.tableId}` }
669675
}
670676

@@ -1089,12 +1095,9 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
10891095
}
10901096

10911097
const table = await getTableById(args.tableId)
1092-
if (!table) {
1098+
if (!table || table.workspaceId !== workspaceId) {
10931099
return { success: false, message: `Table not found: ${args.tableId}` }
10941100
}
1095-
if (table.workspaceId !== workspaceId) {
1096-
return { success: false, message: 'Table not found' }
1097-
}
10981101

10991102
const requestId = generateId().slice(0, 8)
11001103
assertNotAborted()

apps/sim/lib/copilot/tools/server/visualization/generate-visualization.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,11 @@ async function collectSandboxFiles(
118118
if (inputTables?.length) {
119119
for (const tableId of inputTables) {
120120
const table = await getTableById(tableId)
121-
if (!table) {
121+
if (!table || table.workspaceId !== workspaceId) {
122122
logger.warn('Sandbox input table not found', { tableId })
123123
continue
124124
}
125-
const { rows } = await queryRows(tableId, workspaceId, { limit: 10000 }, 'sandbox-input')
125+
const { rows } = await queryRows(table, { limit: 10000 }, 'sandbox-input')
126126
const schema = table.schema as { columns: Array<{ name: string; type?: string }> }
127127
const cols = schema.columns.map((c) => c.name)
128128
const typeComment = `# types: ${schema.columns.map((c) => `${c.name}=${c.type || 'string'}`).join(', ')}`
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Integration test asserting that `table.schema.columns` is forwarded to
5+
* `buildFilterClause` from each service function that filters rows. This
6+
* guards the contract that type-aware JSONB casts (numeric for numbers,
7+
* timestamp for dates) are always available at the SQL builder layer — the
8+
* latent bug that PR #4657 was originally fixing.
9+
*/
10+
import { dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing'
11+
import { sql } from 'drizzle-orm'
12+
import { beforeEach, describe, expect, it, vi } from 'vitest'
13+
import { buildFilterClause, buildSortClause } from '@/lib/table/sql'
14+
import type { ColumnDefinition, TableDefinition } from '@/lib/table/types'
15+
16+
vi.mock('@sim/db', () => dbChainMock)
17+
18+
vi.mock('@/lib/table/sql', () => ({
19+
buildFilterClause: vi.fn(() => sql`true`),
20+
buildSortClause: vi.fn(() => sql`true`),
21+
}))
22+
23+
vi.mock('@/lib/table/trigger', () => ({
24+
fireTableTrigger: vi.fn(),
25+
}))
26+
27+
vi.mock('@/lib/table/workflow-columns', () => ({
28+
assertValidSchema: vi.fn(),
29+
scheduleRunsForRows: vi.fn(),
30+
scheduleRunsForTable: vi.fn(),
31+
stripGroupDeps: vi.fn(),
32+
}))
33+
34+
vi.mock('@/lib/table/validation', () => ({
35+
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
36+
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
37+
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
38+
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
39+
getUniqueColumns: vi.fn(() => []),
40+
checkUniqueConstraintsDb: vi.fn(async () => ({ valid: true, errors: [] })),
41+
checkBatchUniqueConstraintsDb: vi.fn(async () => ({ valid: true, errors: [] })),
42+
}))
43+
44+
import { deleteRowsByFilter, queryRows, updateRowsByFilter } from '@/lib/table/service'
45+
46+
const COLUMNS: ColumnDefinition[] = [
47+
{ name: 'name', type: 'string' },
48+
{ name: 'birthDate', type: 'date' },
49+
{ name: 'score', type: 'number' },
50+
]
51+
52+
const TABLE: TableDefinition = {
53+
id: 'tbl-1',
54+
name: 'People',
55+
description: null,
56+
schema: { columns: COLUMNS },
57+
metadata: null,
58+
rowCount: 0,
59+
maxRows: 1000,
60+
workspaceId: 'ws-1',
61+
createdBy: 'user-1',
62+
archivedAt: null,
63+
createdAt: new Date('2024-01-01'),
64+
updatedAt: new Date('2024-01-01'),
65+
}
66+
67+
describe('service filter threading', () => {
68+
beforeEach(() => {
69+
vi.clearAllMocks()
70+
resetDbChainMock()
71+
})
72+
73+
it('queryRows forwards table.schema.columns to buildFilterClause', async () => {
74+
await queryRows(
75+
TABLE,
76+
{ filter: { birthDate: { $gte: '2024-01-01' } }, includeTotal: false },
77+
'req-1'
78+
).catch(() => {})
79+
80+
expect(buildFilterClause).toHaveBeenCalledTimes(1)
81+
expect(buildFilterClause).toHaveBeenCalledWith(
82+
{ birthDate: { $gte: '2024-01-01' } },
83+
expect.any(String),
84+
COLUMNS
85+
)
86+
})
87+
88+
it('queryRows forwards columns to buildSortClause as well', async () => {
89+
await queryRows(TABLE, { sort: { birthDate: 'asc' }, includeTotal: false }, 'req-1').catch(
90+
() => {}
91+
)
92+
93+
expect(buildSortClause).toHaveBeenCalledWith({ birthDate: 'asc' }, expect.any(String), COLUMNS)
94+
})
95+
96+
it('updateRowsByFilter forwards table.schema.columns to buildFilterClause', async () => {
97+
dbChainMockFns.where.mockResolvedValueOnce([])
98+
await updateRowsByFilter(
99+
TABLE,
100+
{ filter: { birthDate: { $lt: '2024-06-01' } }, data: { name: 'x' } },
101+
'req-1'
102+
)
103+
104+
expect(buildFilterClause).toHaveBeenCalledTimes(1)
105+
expect(buildFilterClause).toHaveBeenCalledWith(
106+
{ birthDate: { $lt: '2024-06-01' } },
107+
expect.any(String),
108+
COLUMNS
109+
)
110+
})
111+
112+
it('deleteRowsByFilter forwards table.schema.columns to buildFilterClause', async () => {
113+
dbChainMockFns.where.mockResolvedValueOnce([])
114+
await deleteRowsByFilter(TABLE, { filter: { score: { $gt: 90 } } }, 'req-1')
115+
116+
expect(buildFilterClause).toHaveBeenCalledTimes(1)
117+
expect(buildFilterClause).toHaveBeenCalledWith(
118+
{ score: { $gt: 90 } },
119+
expect.any(String),
120+
COLUMNS
121+
)
122+
})
123+
})

0 commit comments

Comments
 (0)