Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions apps/sim/app/api/table/[tableId]/export/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Rou
let firstJsonRow = true
while (true) {
const result = await queryRows(
tableId,
table.workspaceId,
table,
{ limit: EXPORT_BATCH_SIZE, offset, includeTotal: false },
requestId
)
Expand Down
16 changes: 9 additions & 7 deletions apps/sim/app/api/table/[tableId]/rows/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,14 @@ export const GET = withRouteHandler(
eq(userTableRows.workspaceId, validated.workspaceId),
]

const schema = table.schema as TableSchema

if (validated.filter) {
const filterClause = buildFilterClause(validated.filter as Filter, USER_TABLE_ROWS_SQL_NAME)
const filterClause = buildFilterClause(
validated.filter as Filter,
USER_TABLE_ROWS_SQL_NAME,
schema.columns
)
if (filterClause) {
baseConditions.push(filterClause)
}
Expand All @@ -286,7 +292,6 @@ export const GET = withRouteHandler(
.where(and(...baseConditions))

if (validated.sort) {
const schema = table.schema as TableSchema
const sortClause = buildSortClause(validated.sort, USER_TABLE_ROWS_SQL_NAME, schema.columns)
if (sortClause) {
query = query.orderBy(sortClause) as typeof query
Expand Down Expand Up @@ -388,14 +393,12 @@ export const PUT = withRouteHandler(
}

const result = await updateRowsByFilter(
table,
{
tableId,
filter: validated.filter as Filter,
data: validated.data as RowData,
limit: validated.limit,
workspaceId: validated.workspaceId,
},
table,
requestId
)

Expand Down Expand Up @@ -503,11 +506,10 @@ export const DELETE = withRouteHandler(
}

const result = await deleteRowsByFilter(
table,
{
tableId,
filter: validated.filter as Filter,
limit: validated.limit,
workspaceId: validated.workspaceId,
},
requestId
)
Expand Down
16 changes: 9 additions & 7 deletions apps/sim/app/api/v1/tables/[tableId]/rows/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,14 @@ export const GET = withRouteHandler(async (request: NextRequest, context: TableR
eq(userTableRows.workspaceId, validated.workspaceId),
]

const schema = table.schema as TableSchema

if (validated.filter) {
const filterClause = buildFilterClause(validated.filter as Filter, USER_TABLE_ROWS_SQL_NAME)
const filterClause = buildFilterClause(
validated.filter as Filter,
USER_TABLE_ROWS_SQL_NAME,
schema.columns
)
if (filterClause) {
baseConditions.push(filterClause)
}
Expand All @@ -177,7 +183,6 @@ export const GET = withRouteHandler(async (request: NextRequest, context: TableR
.where(and(...baseConditions))

if (validated.sort) {
const schema = table.schema as TableSchema
const sortClause = buildSortClause(validated.sort, USER_TABLE_ROWS_SQL_NAME, schema.columns)
if (sortClause) {
query = query.orderBy(sortClause) as typeof query
Expand Down Expand Up @@ -378,14 +383,12 @@ export const PUT = withRouteHandler(async (request: NextRequest, context: TableR
}

const result = await updateRowsByFilter(
table,
{
tableId,
filter: validated.filter as Filter,
data: validated.data as RowData,
limit: validated.limit,
workspaceId: validated.workspaceId,
},
table,
requestId
)

Expand Down Expand Up @@ -484,11 +487,10 @@ export const DELETE = withRouteHandler(
}

const result = await deleteRowsByFilter(
table,
{
tableId,
filter: validated.filter as Filter,
limit: validated.limit,
workspaceId: validated.workspaceId,
},
requestId
)
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/copilot/tools/handlers/function-execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ async function resolveInputFiles(
for (const tableId of inputTables) {
if (typeof tableId !== 'string') continue
const table = await getTableById(tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
logger.warn('Input table not found', { tableId })
continue
}
const rows = await queryRows(tableId, workspaceId, {}, 'copilot-fn-exec')
const rows = await queryRows(table, {}, 'copilot-fn-exec')
if (!rows.rows?.length) continue

const allKeys = new Set<string>()
Expand Down
35 changes: 19 additions & 16 deletions apps/sim/lib/copilot/tools/server/table/user-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
}

const table = await getTableById(args.tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}

Expand Down Expand Up @@ -418,7 +418,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
}

const table = await getTableById(args.tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}

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

const table = await getTableById(args.tableId)
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}

const requestId = generateId().slice(0, 8)
const result = await queryRows(
args.tableId,
workspaceId,
table,
{
filter: args.filter,
sort: args.sort,
Expand Down Expand Up @@ -509,7 +513,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
}

const table = await getTableById(args.tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}

Expand Down Expand Up @@ -569,21 +573,19 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
}

const table = await getTableById(args.tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}

const requestId = generateId().slice(0, 8)
assertNotAborted()
const result = await updateRowsByFilter(
table,
{
tableId: args.tableId,
filter: args.filter,
data: args.data,
limit: args.limit,
workspaceId,
},
table,
requestId
)

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

const table = await getTableById(args.tableId)
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}

const requestId = generateId().slice(0, 8)
assertNotAborted()
const result = await deleteRowsByFilter(
table,
{
tableId: args.tableId,
filter: args.filter,
limit: args.limit,
workspaceId,
},
requestId
)
Expand Down Expand Up @@ -664,7 +670,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
}

const table = await getTableById(args.tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}

Expand Down Expand Up @@ -1089,12 +1095,9 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
}

const table = await getTableById(args.tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
}
if (table.workspaceId !== workspaceId) {
return { success: false, message: 'Table not found' }
}

const requestId = generateId().slice(0, 8)
assertNotAborted()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ async function collectSandboxFiles(
if (inputTables?.length) {
for (const tableId of inputTables) {
const table = await getTableById(tableId)
if (!table) {
if (!table || table.workspaceId !== workspaceId) {
logger.warn('Sandbox input table not found', { tableId })
continue
}
const { rows } = await queryRows(tableId, workspaceId, { limit: 10000 }, 'sandbox-input')
const { rows } = await queryRows(table, { limit: 10000 }, 'sandbox-input')
const schema = table.schema as { columns: Array<{ name: string; type?: string }> }
const cols = schema.columns.map((c) => c.name)
const typeComment = `# types: ${schema.columns.map((c) => `${c.name}=${c.type || 'string'}`).join(', ')}`
Expand Down
123 changes: 123 additions & 0 deletions apps/sim/lib/table/__tests__/service-filter-threading.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/**
* @vitest-environment node
*
* Integration test asserting that `table.schema.columns` is forwarded to
* `buildFilterClause` from each service function that filters rows. This
* guards the contract that type-aware JSONB casts (numeric for numbers,
* timestamp for dates) are always available at the SQL builder layer — the
* latent bug that PR #4657 was originally fixing.
*/
import { dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing'
import { sql } from 'drizzle-orm'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { buildFilterClause, buildSortClause } from '@/lib/table/sql'
import type { ColumnDefinition, TableDefinition } from '@/lib/table/types'

vi.mock('@sim/db', () => dbChainMock)

vi.mock('@/lib/table/sql', () => ({
buildFilterClause: vi.fn(() => sql`true`),
buildSortClause: vi.fn(() => sql`true`),
}))

vi.mock('@/lib/table/trigger', () => ({
fireTableTrigger: vi.fn(),
}))

vi.mock('@/lib/table/workflow-columns', () => ({
assertValidSchema: vi.fn(),
scheduleRunsForRows: vi.fn(),
scheduleRunsForTable: vi.fn(),
stripGroupDeps: vi.fn(),
}))

vi.mock('@/lib/table/validation', () => ({
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
validateTableName: vi.fn(() => ({ valid: true, errors: [] })),
validateTableSchema: vi.fn(() => ({ valid: true, errors: [] })),
getUniqueColumns: vi.fn(() => []),
checkUniqueConstraintsDb: vi.fn(async () => ({ valid: true, errors: [] })),
checkBatchUniqueConstraintsDb: vi.fn(async () => ({ valid: true, errors: [] })),
}))

import { deleteRowsByFilter, queryRows, updateRowsByFilter } from '@/lib/table/service'

const COLUMNS: ColumnDefinition[] = [
{ name: 'name', type: 'string' },
{ name: 'birthDate', type: 'date' },
{ name: 'score', type: 'number' },
]

const TABLE: TableDefinition = {
id: 'tbl-1',
name: 'People',
description: null,
schema: { columns: COLUMNS },
metadata: null,
rowCount: 0,
maxRows: 1000,
workspaceId: 'ws-1',
createdBy: 'user-1',
archivedAt: null,
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-01'),
}

describe('service filter threading', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
})

it('queryRows forwards table.schema.columns to buildFilterClause', async () => {
await queryRows(
TABLE,
{ filter: { birthDate: { $gte: '2024-01-01' } }, includeTotal: false },
'req-1'
).catch(() => {})

expect(buildFilterClause).toHaveBeenCalledTimes(1)
expect(buildFilterClause).toHaveBeenCalledWith(
{ birthDate: { $gte: '2024-01-01' } },
expect.any(String),
COLUMNS
)
})

it('queryRows forwards columns to buildSortClause as well', async () => {
await queryRows(TABLE, { sort: { birthDate: 'asc' }, includeTotal: false }, 'req-1').catch(
() => {}
)

expect(buildSortClause).toHaveBeenCalledWith({ birthDate: 'asc' }, expect.any(String), COLUMNS)
})

it('updateRowsByFilter forwards table.schema.columns to buildFilterClause', async () => {
dbChainMockFns.where.mockResolvedValueOnce([])
await updateRowsByFilter(
TABLE,
{ filter: { birthDate: { $lt: '2024-06-01' } }, data: { name: 'x' } },
'req-1'
)

expect(buildFilterClause).toHaveBeenCalledTimes(1)
expect(buildFilterClause).toHaveBeenCalledWith(
{ birthDate: { $lt: '2024-06-01' } },
expect.any(String),
COLUMNS
)
})

it('deleteRowsByFilter forwards table.schema.columns to buildFilterClause', async () => {
dbChainMockFns.where.mockResolvedValueOnce([])
await deleteRowsByFilter(TABLE, { filter: { score: { $gt: 90 } } }, 'req-1')

expect(buildFilterClause).toHaveBeenCalledTimes(1)
expect(buildFilterClause).toHaveBeenCalledWith(
{ score: { $gt: 90 } },
expect.any(String),
COLUMNS
)
})
})
Loading
Loading