diff --git a/apps/sim/app/api/table/[tableId]/export/route.ts b/apps/sim/app/api/table/[tableId]/export/route.ts index a2e59b42f83..8f9fa34b807 100644 --- a/apps/sim/app/api/table/[tableId]/export/route.ts +++ b/apps/sim/app/api/table/[tableId]/export/route.ts @@ -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 ) diff --git a/apps/sim/app/api/table/[tableId]/rows/route.ts b/apps/sim/app/api/table/[tableId]/rows/route.ts index 9b0076a127d..414ac57d41a 100644 --- a/apps/sim/app/api/table/[tableId]/rows/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/route.ts @@ -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) } @@ -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 @@ -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 ) @@ -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 ) diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts index d4d9c448837..e736a859eaa 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts @@ -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) } @@ -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 @@ -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 ) @@ -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 ) diff --git a/apps/sim/lib/copilot/tools/handlers/function-execute.ts b/apps/sim/lib/copilot/tools/handlers/function-execute.ts index 29f53924610..0968b3ffa86 100644 --- a/apps/sim/lib/copilot/tools/handlers/function-execute.ts +++ b/apps/sim/lib/copilot/tools/handlers/function-execute.ts @@ -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() diff --git a/apps/sim/lib/copilot/tools/server/table/user-table.ts b/apps/sim/lib/copilot/tools/server/table/user-table.ts index e6464e87afd..1baee52e9d0 100644 --- a/apps/sim/lib/copilot/tools/server/table/user-table.ts +++ b/apps/sim/lib/copilot/tools/server/table/user-table.ts @@ -367,7 +367,7 @@ export const userTableServerTool: BaseServerTool } const table = await getTableById(args.tableId) - if (!table) { + if (!table || table.workspaceId !== workspaceId) { return { success: false, message: `Table not found: ${args.tableId}` } } @@ -418,7 +418,7 @@ export const userTableServerTool: BaseServerTool } const table = await getTableById(args.tableId) - if (!table) { + if (!table || table.workspaceId !== workspaceId) { return { success: false, message: `Table not found: ${args.tableId}` } } @@ -474,10 +474,14 @@ export const userTableServerTool: BaseServerTool 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, @@ -509,7 +513,7 @@ export const userTableServerTool: BaseServerTool } const table = await getTableById(args.tableId) - if (!table) { + if (!table || table.workspaceId !== workspaceId) { return { success: false, message: `Table not found: ${args.tableId}` } } @@ -569,21 +573,19 @@ export const userTableServerTool: BaseServerTool } 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 ) @@ -605,14 +607,18 @@ export const userTableServerTool: BaseServerTool 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 ) @@ -664,7 +670,7 @@ export const userTableServerTool: BaseServerTool } const table = await getTableById(args.tableId) - if (!table) { + if (!table || table.workspaceId !== workspaceId) { return { success: false, message: `Table not found: ${args.tableId}` } } @@ -1089,12 +1095,9 @@ export const userTableServerTool: BaseServerTool } 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() diff --git a/apps/sim/lib/copilot/tools/server/visualization/generate-visualization.ts b/apps/sim/lib/copilot/tools/server/visualization/generate-visualization.ts index e642c244249..629622a1cf3 100644 --- a/apps/sim/lib/copilot/tools/server/visualization/generate-visualization.ts +++ b/apps/sim/lib/copilot/tools/server/visualization/generate-visualization.ts @@ -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(', ')}` diff --git a/apps/sim/lib/table/__tests__/service-filter-threading.test.ts b/apps/sim/lib/table/__tests__/service-filter-threading.test.ts new file mode 100644 index 00000000000..9174d05c6b6 --- /dev/null +++ b/apps/sim/lib/table/__tests__/service-filter-threading.test.ts @@ -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 + ) + }) +}) diff --git a/apps/sim/lib/table/__tests__/sql.test.ts b/apps/sim/lib/table/__tests__/sql.test.ts index 492e3e7fc8b..5258d91a06a 100644 --- a/apps/sim/lib/table/__tests__/sql.test.ts +++ b/apps/sim/lib/table/__tests__/sql.test.ts @@ -3,296 +3,393 @@ * * SQL Builder Unit Tests * - * Tests for the table SQL query builder utilities including filter and sort clause generation. + * Tests the table SQL query builder. Assertions inspect the generated SQL + * string so cast selection (numeric vs timestamptz) is verified end-to-end. + * + * Rendering: `drizzle-orm` is globally mocked in `vitest.setup.ts`. The mock + * represents tagged-template fragments as `{ strings, values }`, raw fragments + * as `{ rawSql }`, and joined fragments as `{ fragments, separator }`. The + * local `renderSql` helper walks that shape recursively so we can assert real + * substrings like `::timestamptz` against the generated SQL. */ import { describe, expect, it } from 'vitest' -import { buildFilterClause, buildSortClause } from '../sql' -import type { Filter } from '../types' +import { buildFilterClause, buildSortClause } from '@/lib/table/sql' +import type { ColumnDefinition, Filter, Sort } from '@/lib/table/types' + +type SqlNode = + | { strings: ArrayLike; values: unknown[] } + | { rawSql: string } + | { fragments: unknown[]; separator: unknown } + | string + | number + | boolean + | null + | undefined + +function isTemplateNode(n: unknown): n is { strings: ArrayLike; values: unknown[] } { + return ( + typeof n === 'object' && + n !== null && + 'strings' in n && + 'values' in n && + Array.isArray((n as { values: unknown[] }).values) + ) +} + +function isRawNode(n: unknown): n is { rawSql: string } { + return typeof n === 'object' && n !== null && 'rawSql' in n +} + +function isJoinNode(n: unknown): n is { fragments: unknown[]; separator: unknown } { + return ( + typeof n === 'object' && + n !== null && + 'fragments' in n && + Array.isArray((n as { fragments: unknown[] }).fragments) + ) +} + +/** Recursively render a mock SQL node into its generated SQL string. */ +function renderSql(node: SqlNode | unknown): string { + if (node == null) return String(node) + if (isRawNode(node)) return node.rawSql + if (isJoinNode(node)) { + const sep = isRawNode(node.separator) ? node.separator.rawSql : ', ' + return node.fragments.map(renderSql).join(sep) + } + if (isTemplateNode(node)) { + const parts: string[] = [] + for (let i = 0; i < node.strings.length; i++) { + parts.push(node.strings[i]) + if (i < node.values.length) { + parts.push(renderSql(node.values[i])) + } + } + return parts.join('') + } + if (typeof node === 'string') return `'${node}'` + return String(node) +} + +function render(node: unknown): string { + return renderSql(node) +} + +const TABLE = 'user_table_rows' +const NO_COLUMNS: ColumnDefinition[] = [] describe('SQL Builder', () => { describe('buildFilterClause', () => { - const tableName = 'user_table_rows' - - it('should return undefined for empty filter', () => { - const result = buildFilterClause({}, tableName) - expect(result).toBeUndefined() + it('returns undefined for empty filter', () => { + expect(buildFilterClause({}, TABLE, NO_COLUMNS)).toBeUndefined() }) - it('should handle simple equality filter', () => { - const filter: Filter = { name: 'John' } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles simple equality via JSONB containment', () => { + const out = render(buildFilterClause({ name: 'John' }, TABLE, NO_COLUMNS)) + expect(out).toContain('user_table_rows.data @>') + expect(out).toContain('"name":"John"') }) - it('should handle $eq operator', () => { - const filter: Filter = { status: { $eq: 'active' } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('emits ::numeric cast for $gt on a number column', () => { + const cols: ColumnDefinition[] = [{ name: 'age', type: 'number' }] + const out = render(buildFilterClause({ age: { $gt: 18 } }, TABLE, cols)) + expect(out).toContain(`(${TABLE}.data->>'age')::numeric > `) + expect(out).not.toContain('::timestamp') }) - it('should handle $ne operator', () => { - const filter: Filter = { status: { $ne: 'deleted' } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('falls back to ::numeric when column type is unknown', () => { + const out = render(buildFilterClause({ score: { $gte: 5 } }, TABLE, NO_COLUMNS)) + expect(out).toContain(`(${TABLE}.data->>'score')::numeric >= `) + expect(out).not.toContain('::timestamp') }) - it('should handle $gt operator', () => { - const filter: Filter = { age: { $gt: 18 } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles $eq operator', () => { + const out = render(buildFilterClause({ status: { $eq: 'active' } }, TABLE, NO_COLUMNS)) + expect(out).toContain('"status":"active"') }) - it('should handle $gte operator', () => { - const filter: Filter = { age: { $gte: 18 } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles $ne operator', () => { + const out = render(buildFilterClause({ status: { $ne: 'deleted' } }, TABLE, NO_COLUMNS)) + expect(out).toContain('NOT (') + expect(out).toContain('"status":"deleted"') }) - it('should handle $lt operator', () => { - const filter: Filter = { age: { $lt: 65 } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles $in with multiple values via OR of containments', () => { + const out = render( + buildFilterClause({ status: { $in: ['active', 'pending'] } }, TABLE, NO_COLUMNS) + ) + expect(out).toContain(' OR ') + expect(out).toContain('"status":"active"') + expect(out).toContain('"status":"pending"') }) - it('should handle $lte operator', () => { - const filter: Filter = { age: { $lte: 65 } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles $nin', () => { + const out = render( + buildFilterClause({ status: { $nin: ['deleted', 'archived'] } }, TABLE, NO_COLUMNS) + ) + expect(out).toContain('NOT (') + expect(out).toContain(' AND ') }) - it('should handle $in operator with single value', () => { - const filter: Filter = { status: { $in: ['active'] } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles $contains as ILIKE', () => { + const out = render(buildFilterClause({ name: { $contains: 'john' } }, TABLE, NO_COLUMNS)) + expect(out).toContain(`${TABLE}.data->>'name'`) + expect(out).toContain('ILIKE') }) - it('should handle $in operator with multiple values', () => { - const filter: Filter = { status: { $in: ['active', 'pending'] } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('joins multiple top-level conditions with AND', () => { + const out = render( + buildFilterClause({ status: 'active', age: { $gt: 18 } }, TABLE, NO_COLUMNS) + ) + expect(out).toContain(' AND ') }) - it('should handle $nin operator', () => { - const filter: Filter = { status: { $nin: ['deleted', 'archived'] } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles $or logical operator', () => { + const out = render( + buildFilterClause({ $or: [{ status: 'active' }, { status: 'pending' }] }, TABLE, NO_COLUMNS) + ) + expect(out).toContain(' OR ') }) - it('should handle $contains operator', () => { - const filter: Filter = { name: { $contains: 'john' } } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles $and logical operator', () => { + const out = render( + buildFilterClause({ $and: [{ status: 'active' }, { age: { $gt: 18 } }] }, TABLE, NO_COLUMNS) + ) + expect(out).toContain(' AND ') }) - it('should handle $or logical operator', () => { - const filter: Filter = { - $or: [{ status: 'active' }, { status: 'pending' }], - } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles nested $or and $and', () => { + const out = render( + buildFilterClause( + { $or: [{ $and: [{ status: 'active' }, { verified: true }] }, { role: 'admin' }] }, + TABLE, + NO_COLUMNS + ) + ) + expect(out).toContain(' OR ') + expect(out).toContain(' AND ') }) - it('should handle $and logical operator', () => { - const filter: Filter = { - $and: [{ status: 'active' }, { age: { $gt: 18 } }], - } - const result = buildFilterClause(filter, tableName) - + it('skips undefined values', () => { + const result = buildFilterClause({ name: undefined, status: 'active' }, TABLE, NO_COLUMNS) expect(result).toBeDefined() }) - it('should handle multiple conditions combined with AND', () => { - const filter: Filter = { - status: 'active', - age: { $gt: 18 }, - } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('handles boolean / null / numeric primitives', () => { + expect(render(buildFilterClause({ active: true }, TABLE, NO_COLUMNS))).toContain( + '"active":true' + ) + expect(render(buildFilterClause({ deleted_at: null }, TABLE, NO_COLUMNS))).toContain( + '"deleted_at":null' + ) + expect(render(buildFilterClause({ count: 42 }, TABLE, NO_COLUMNS))).toContain('"count":42') }) - it('should handle nested $or and $and', () => { - const filter: Filter = { - $or: [{ $and: [{ status: 'active' }, { verified: true }] }, { role: 'admin' }], - } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('throws on invalid field name', () => { + expect(() => buildFilterClause({ 'invalid-field': 'v' }, TABLE, NO_COLUMNS)).toThrow( + 'Invalid field name' + ) }) - it('should throw error for invalid field name', () => { - const filter: Filter = { 'invalid-field': 'value' } - - expect(() => buildFilterClause(filter, tableName)).toThrow('Invalid field name') + it('throws on invalid operator', () => { + const f = { name: { $invalid: 'value' } } as unknown as Filter + expect(() => buildFilterClause(f, TABLE, NO_COLUMNS)).toThrow('Invalid operator') }) + }) - it('should throw error for invalid operator', () => { - const filter = { name: { $invalid: 'value' } } as unknown as Filter - - expect(() => buildFilterClause(filter, tableName)).toThrow('Invalid operator') + describe('buildFilterClause > date column type', () => { + const dateCols: ColumnDefinition[] = [{ name: 'birthDate', type: 'date' }] + + it.each([ + ['$gt', '>'], + ['$gte', '>='], + ['$lt', '<'], + ['$lte', '<='], + ] as const)('emits ::timestamptz on both sides for %s on a date column', (operator, sqlOp) => { + const filter = { birthDate: { [operator]: '2024-01-01' } } as Filter + const out = render(buildFilterClause(filter, TABLE, dateCols)) + expect(out).toContain(`(${TABLE}.data->>'birthDate')::timestamptz ${sqlOp} `) + expect(out).toContain('::timestamptz') + expect(out).not.toContain('::numeric') + // RHS cast — without it Postgres would compare as text (lexicographic). + expect(out.match(/::timestamptz/g)?.length).toBe(2) + }) + + it('combined range ($gte + $lte) emits two ::timestamptz pairs', () => { + const out = render( + buildFilterClause( + { birthDate: { $gte: '2024-01-01', $lte: '2024-12-31' } }, + TABLE, + dateCols + ) + ) + expect(out.match(/::timestamptz/g)?.length).toBe(4) + expect(out).not.toContain('::numeric') + expect(out).toContain(' AND ') + }) + + it('propagates date cast through nested $and', () => { + const out = render( + buildFilterClause( + { $and: [{ birthDate: { $gte: '2024-01-01' } }, { birthDate: { $lt: '2025-01-01' } }] }, + TABLE, + dateCols + ) + ) + expect(out).toContain('::timestamptz') + expect(out).not.toContain('::numeric') + }) + + it('propagates date cast through nested $or', () => { + const out = render( + buildFilterClause( + { $or: [{ birthDate: { $lt: '2000-01-01' } }, { birthDate: { $gt: '2024-01-01' } }] }, + TABLE, + dateCols + ) + ) + expect(out).toContain('::timestamptz') + expect(out).not.toContain('::numeric') + expect(out).toContain(' OR ') + }) + + it('a number column in the same query keeps ::numeric (no cross-contamination)', () => { + const cols: ColumnDefinition[] = [ + { name: 'birthDate', type: 'date' }, + { name: 'age', type: 'number' }, + ] + const out = render( + buildFilterClause({ birthDate: { $gte: '2024-01-01' }, age: { $gt: 18 } }, TABLE, cols) + ) + expect(out).toContain('::timestamptz') + expect(out).toContain('::numeric') }) + }) - it('should skip undefined values', () => { - const filter: Filter = { name: undefined, status: 'active' } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + describe('buildFilterClause > range operator value type validation', () => { + it('throws when $gt on a number column receives a string', () => { + const cols: ColumnDefinition[] = [{ name: 'age', type: 'number' }] + expect(() => buildFilterClause({ age: { $gt: 'eighteen' } } as Filter, TABLE, cols)).toThrow( + /column "age" \(number\) requires a number, got string/ + ) }) - it('should handle boolean values', () => { - const filter: Filter = { active: true } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('throws when $gte on a date column receives a number', () => { + const cols: ColumnDefinition[] = [{ name: 'birthDate', type: 'date' }] + expect(() => + buildFilterClause({ birthDate: { $gte: 1704067200000 } } as Filter, TABLE, cols) + ).toThrow(/column "birthDate" \(date\) requires a date string, got number/) }) - it('should handle null values', () => { - const filter: Filter = { deleted_at: null } - const result = buildFilterClause(filter, tableName) - - expect(result).toBeDefined() + it('throws when $lt on an unknown column (numeric fallback) receives a string', () => { + expect(() => + buildFilterClause({ score: { $lt: 'high' } } as Filter, TABLE, NO_COLUMNS) + ).toThrow(/column "score" \(number\) requires a number, got string/) }) - it('should handle numeric values', () => { - const filter: Filter = { count: 42 } - const result = buildFilterClause(filter, tableName) + it('accepts valid number on number column', () => { + const cols: ColumnDefinition[] = [{ name: 'age', type: 'number' }] + expect(() => buildFilterClause({ age: { $gt: 18 } }, TABLE, cols)).not.toThrow() + }) - expect(result).toBeDefined() + it('accepts valid ISO string on date column', () => { + const cols: ColumnDefinition[] = [{ name: 'birthDate', type: 'date' }] + expect(() => + buildFilterClause({ birthDate: { $gte: '2024-01-01' } }, TABLE, cols) + ).not.toThrow() }) }) describe('buildSortClause', () => { - const tableName = 'user_table_rows' - - it('should return undefined for empty sort', () => { - const result = buildSortClause({}, tableName) - expect(result).toBeUndefined() + it('returns undefined for empty sort', () => { + expect(buildSortClause({}, TABLE, NO_COLUMNS)).toBeUndefined() }) - it('should handle single field ascending sort', () => { - const sort = { name: 'asc' as const } - const result = buildSortClause(sort, tableName) - - expect(result).toBeDefined() + it('sorts string columns as text (no cast)', () => { + const cols: ColumnDefinition[] = [{ name: 'name', type: 'string' }] + const out = render(buildSortClause({ name: 'asc' }, TABLE, cols)) + expect(out).toBe(`${TABLE}.data->>'name' ASC`) + expect(out).not.toContain('::') }) - it('should handle single field descending sort', () => { - const sort = { name: 'desc' as const } - const result = buildSortClause(sort, tableName) - - expect(result).toBeDefined() + it('sorts number columns with ::numeric NULLS LAST', () => { + const cols: ColumnDefinition[] = [{ name: 'salary', type: 'number' }] + const out = render(buildSortClause({ salary: 'desc' }, TABLE, cols)) + expect(out).toBe(`(${TABLE}.data->>'salary')::numeric DESC NULLS LAST`) }) - it('should handle multiple fields sort', () => { - const sort = { name: 'asc' as const, created_at: 'desc' as const } - const result = buildSortClause(sort, tableName) - - expect(result).toBeDefined() + it('sorts date columns with ::timestamptz NULLS LAST', () => { + const cols: ColumnDefinition[] = [{ name: 'birthDate', type: 'date' }] + const out = render(buildSortClause({ birthDate: 'asc' }, TABLE, cols)) + expect(out).toBe(`(${TABLE}.data->>'birthDate')::timestamptz ASC NULLS LAST`) }) - it('should handle createdAt field directly', () => { - const sort = { createdAt: 'desc' as const } - const result = buildSortClause(sort, tableName) - - expect(result).toBeDefined() + it('sorts createdAt / updatedAt as direct column refs', () => { + expect(render(buildSortClause({ createdAt: 'desc' }, TABLE, NO_COLUMNS))).toBe( + `${TABLE}.createdAt DESC` + ) + expect(render(buildSortClause({ updatedAt: 'asc' }, TABLE, NO_COLUMNS))).toBe( + `${TABLE}.updatedAt ASC` + ) }) - it('should handle updatedAt field directly', () => { - const sort = { updatedAt: 'asc' as const } - const result = buildSortClause(sort, tableName) - - expect(result).toBeDefined() + it('combines multiple sort fields with commas', () => { + const cols: ColumnDefinition[] = [ + { name: 'name', type: 'string' }, + { name: 'salary', type: 'number' }, + ] + const out = render(buildSortClause({ name: 'asc', salary: 'desc' }, TABLE, cols)) + expect(out).toBe( + `${TABLE}.data->>'name' ASC, (${TABLE}.data->>'salary')::numeric DESC NULLS LAST` + ) }) - it('should throw error for invalid field name', () => { - const sort = { 'invalid-field': 'asc' as const } - - expect(() => buildSortClause(sort, tableName)).toThrow('Invalid field name') + it('falls back to text sort for unknown column types', () => { + const sort: Sort = { unknownField: 'asc' } + const out = render(buildSortClause(sort, TABLE, NO_COLUMNS)) + expect(out).toBe(`${TABLE}.data->>'unknownField' ASC`) }) - it('should throw error for invalid direction', () => { - const sort = { name: 'invalid' as 'asc' | 'desc' } - - expect(() => buildSortClause(sort, tableName)).toThrow('Invalid sort direction') - }) - - it('should handle numeric column type for proper numeric sorting', () => { - const sort = { salary: 'desc' as const } - const columns = [{ name: 'salary', type: 'number' as const }] - const result = buildSortClause(sort, tableName, columns) - - expect(result).toBeDefined() - }) - - it('should handle date column type for chronological sorting', () => { - const sort = { birthDate: 'asc' as const } - const columns = [{ name: 'birthDate', type: 'date' as const }] - const result = buildSortClause(sort, tableName, columns) - - expect(result).toBeDefined() - }) - - it('should use text sorting for string columns', () => { - const sort = { name: 'asc' as const } - const columns = [{ name: 'name', type: 'string' as const }] - const result = buildSortClause(sort, tableName, columns) - - expect(result).toBeDefined() + it('throws on invalid field name', () => { + const sort: Sort = { 'invalid-field': 'asc' } + expect(() => buildSortClause(sort, TABLE, NO_COLUMNS)).toThrow('Invalid field name') }) - it('should fall back to text sorting when column type is unknown', () => { - const sort = { unknownField: 'asc' as const } - // No columns provided - const result = buildSortClause(sort, tableName) - - expect(result).toBeDefined() + it('throws on invalid direction', () => { + const sort = { name: 'invalid' as 'asc' | 'desc' } + expect(() => buildSortClause(sort, TABLE, NO_COLUMNS)).toThrow('Invalid sort direction') }) }) - describe('Field Name Validation', () => { - const tableName = 'user_table_rows' - - it('should accept valid field names', () => { - const validNames = ['name', 'user_id', '_private', 'Count123', 'a'] - - for (const name of validNames) { - const filter: Filter = { [name]: 'value' } - expect(() => buildFilterClause(filter, tableName)).not.toThrow() + describe('Field name validation', () => { + it('accepts valid identifiers', () => { + const valid = ['name', 'user_id', '_private', 'Count123', 'a'] + for (const name of valid) { + expect(() => buildFilterClause({ [name]: 'v' }, TABLE, NO_COLUMNS)).not.toThrow() } }) - it('should reject field names starting with number', () => { - const filter: Filter = { '123name': 'value' } - expect(() => buildFilterClause(filter, tableName)).toThrow('Invalid field name') + it('rejects identifiers starting with a digit', () => { + expect(() => buildFilterClause({ '123name': 'v' }, TABLE, NO_COLUMNS)).toThrow( + 'Invalid field name' + ) }) - it('should reject field names with special characters', () => { - const invalidNames = ['field-name', 'field.name', 'field name', 'field@name'] - - for (const name of invalidNames) { - const filter: Filter = { [name]: 'value' } - expect(() => buildFilterClause(filter, tableName)).toThrow('Invalid field name') + it('rejects identifiers with special characters', () => { + const invalid = ['field-name', 'field.name', 'field name', 'field@name'] + for (const name of invalid) { + expect(() => buildFilterClause({ [name]: 'v' }, TABLE, NO_COLUMNS)).toThrow( + 'Invalid field name' + ) } }) - it('should reject SQL injection attempts', () => { - const sqlInjectionAttempts = ["'; DROP TABLE users; --", 'name OR 1=1', 'name; DELETE FROM'] - - for (const attempt of sqlInjectionAttempts) { - const filter: Filter = { [attempt]: 'value' } - expect(() => buildFilterClause(filter, tableName)).toThrow('Invalid field name') + it('rejects SQL injection attempts in field names', () => { + const attempts = ["'; DROP TABLE users; --", 'name OR 1=1', 'name; DELETE FROM'] + for (const a of attempts) { + expect(() => buildFilterClause({ [a]: 'v' }, TABLE, NO_COLUMNS)).toThrow( + 'Invalid field name' + ) } }) }) diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index 972bb551026..693063bfdd4 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -1405,15 +1405,13 @@ export async function upsertRow( * (bounded only by the btree on `table_id`). Prefer equality on hot paths; set * `includeTotal: false` when the caller does not need the `COUNT(*)`. * - * @param tableId - Table ID to query - * @param workspaceId - Workspace ID for access control + * @param table - Table definition (provides id, workspaceId, and column schema for type-aware filter/sort casts) * @param options - Query options (filter, sort, limit, offset) * @param requestId - Request ID for logging * @returns Query result with rows and pagination info */ export async function queryRows( - tableId: string, - workspaceId: string, + table: TableDefinition, options: QueryOptions, requestId: string ): Promise { @@ -1426,16 +1424,17 @@ export async function queryRows( } = options const tableName = USER_TABLE_ROWS_SQL_NAME + const columns = table.schema.columns // Build WHERE clause const baseConditions = and( - eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, workspaceId) + eq(userTableRows.tableId, table.id), + eq(userTableRows.workspaceId, table.workspaceId) ) let whereClause = baseConditions if (filter && Object.keys(filter).length > 0) { - const filterClause = buildFilterClause(filter, tableName) + const filterClause = buildFilterClause(filter, tableName, columns) if (filterClause) { whereClause = and(baseConditions, filterClause) } @@ -1453,7 +1452,7 @@ export async function queryRows( // Build ORDER BY clause (default to position ASC for stable ordering) let orderByClause if (sort && Object.keys(sort).length > 0) { - orderByClause = buildSortClause(sort, tableName) + orderByClause = buildSortClause(sort, tableName, columns) } // Execute query @@ -1471,7 +1470,7 @@ export async function queryRows( const rows = await query.limit(limit).offset(offset) logger.info( - `[${requestId}] Queried ${rows.length} rows from table ${tableId} (total: ${totalCount})` + `[${requestId}] Queried ${rows.length} rows from table ${table.id} (total: ${totalCount})` ) return { @@ -1806,26 +1805,26 @@ export async function deleteRow( /** * Updates multiple rows matching a filter. * + * @param table - Table definition (provides column schema for type-aware filter casts) * @param data - Bulk update data - * @param table - Table definition * @param requestId - Request ID for logging * @returns Bulk operation result */ export async function updateRowsByFilter( - data: BulkUpdateData, table: TableDefinition, + data: BulkUpdateData, requestId: string ): Promise { const tableName = USER_TABLE_ROWS_SQL_NAME - const filterClause = buildFilterClause(data.filter, tableName) + const filterClause = buildFilterClause(data.filter, tableName, table.schema.columns) if (!filterClause) { throw new Error('Filter is required for bulk update') } const baseConditions = and( - eq(userTableRows.tableId, data.tableId), - eq(userTableRows.workspaceId, data.workspaceId) + eq(userTableRows.tableId, table.id), + eq(userTableRows.workspaceId, table.workspaceId) ) let query = db @@ -1873,7 +1872,7 @@ export async function updateRowsByFilter( const row = matchingRows[0] const mergedData = { ...(row.data as RowData), ...data.data } const uniqueValidation = await checkUniqueConstraintsDb( - data.tableId, + table.id, mergedData, table.schema, row.id @@ -1901,7 +1900,7 @@ export async function updateRowsByFilter( } }) - logger.info(`[${requestId}] Updated ${matchingRows.length} rows in table ${data.tableId}`) + logger.info(`[${requestId}] Updated ${matchingRows.length} rows in table ${table.id}`) const oldRows = new Map(matchingRows.map((r) => [r.id, r.data as RowData])) const updatedRows: TableRow[] = matchingRows.map((r) => ({ @@ -1913,7 +1912,7 @@ export async function updateRowsByFilter( updatedAt: now, })) void fireTableTrigger( - data.tableId, + table.id, table.name, 'update', updatedRows, @@ -2118,26 +2117,28 @@ async function recompactPositions(tableId: string, trx: DbTransaction, minDelete /** * Deletes multiple rows matching a filter. * + * @param table - Table definition (provides column schema for type-aware filter casts) * @param data - Bulk delete data * @param requestId - Request ID for logging * @returns Bulk operation result */ export async function deleteRowsByFilter( + table: TableDefinition, data: BulkDeleteData, requestId: string ): Promise { const tableName = USER_TABLE_ROWS_SQL_NAME // Build filter clause - const filterClause = buildFilterClause(data.filter, tableName) + const filterClause = buildFilterClause(data.filter, tableName, table.schema.columns) if (!filterClause) { throw new Error('Filter is required for bulk delete') } // Find matching rows const baseConditions = and( - eq(userTableRows.tableId, data.tableId), - eq(userTableRows.workspaceId, data.workspaceId) + eq(userTableRows.tableId, table.id), + eq(userTableRows.workspaceId, table.workspaceId) ) let query = db @@ -2167,8 +2168,8 @@ export async function deleteRowsByFilter( const batch = rowIds.slice(i, i + TABLE_LIMITS.DELETE_BATCH_SIZE) await trx.delete(userTableRows).where( and( - eq(userTableRows.tableId, data.tableId), - eq(userTableRows.workspaceId, data.workspaceId), + eq(userTableRows.tableId, table.id), + eq(userTableRows.workspaceId, table.workspaceId), sql`${userTableRows.id} = ANY(ARRAY[${sql.join( batch.map((id) => sql`${id}`), sql`, ` @@ -2177,10 +2178,10 @@ export async function deleteRowsByFilter( ) } - await recompactPositions(data.tableId, trx, minDeletedPos) + await recompactPositions(table.id, trx, minDeletedPos) }) - logger.info(`[${requestId}] Deleted ${matchingRows.length} rows from table ${data.tableId}`) + logger.info(`[${requestId}] Deleted ${matchingRows.length} rows from table ${table.id}`) return { affectedCount: matchingRows.length, diff --git a/apps/sim/lib/table/sql.ts b/apps/sim/lib/table/sql.ts index f854d2b5237..1b07407e998 100644 --- a/apps/sim/lib/table/sql.ts +++ b/apps/sim/lib/table/sql.ts @@ -21,6 +21,30 @@ export class TableQueryValidationError extends Error { } } +type ColumnType = ColumnDefinition['type'] +type ColumnTypeMap = ReadonlyMap + +/** + * Returns the Postgres cast needed to compare a JSONB text value of the given + * column type, or `null` when text comparison is correct. Single source of + * truth for both filter range operators and sort ordering — keeps the two + * paths from drifting apart. + */ +function jsonbCastForType(type: ColumnType | undefined): 'numeric' | 'timestamptz' | null { + switch (type) { + case 'number': + return 'numeric' + case 'date': + return 'timestamptz' + default: + return null + } +} + +function buildColumnTypeMap(columns: ColumnDefinition[]): ColumnTypeMap { + return new Map(columns.map((col) => [col.name, col.type])) +} + /** * Whitelist of allowed operators for query filtering. * Only these operators can be used in filter conditions. @@ -51,20 +75,42 @@ const ALLOWED_OPERATORS = new Set([ * * @param filter - Filter object with field conditions and logical operators * @param tableName - Table name for the query (e.g., 'user_table_rows') + * @param columns - Column definitions; drives type-aware JSONB casts (numeric for numbers, timestamptz for dates) * @returns SQL WHERE clause or undefined if no filter specified * @throws {TableQueryValidationError} if field name is invalid or operator is not allowed * * @example * // Simple equality - * buildFilterClause({ name: 'John' }, 'user_table_rows') + * buildFilterClause({ name: 'John' }, 'user_table_rows', [{ name: 'name', type: 'string' }]) * - * // Complex filter with operators - * buildFilterClause({ age: { $gte: 18 }, status: { $in: ['active', 'pending'] } }, 'user_table_rows') + * // Range on a date column — emits `::timestamptz` on both sides + * buildFilterClause( + * { birthDate: { $gte: '2024-01-01' } }, + * 'user_table_rows', + * [{ name: 'birthDate', type: 'date' }], + * ) * * // Logical operators - * buildFilterClause({ $or: [{ status: 'active' }, { verified: true }] }, 'user_table_rows') + * buildFilterClause( + * { $or: [{ status: 'active' }, { verified: true }] }, + * 'user_table_rows', + * [{ name: 'status', type: 'string' }, { name: 'verified', type: 'boolean' }], + * ) */ -export function buildFilterClause(filter: Filter, tableName: string): SQL | undefined { +export function buildFilterClause( + filter: Filter, + tableName: string, + columns: ColumnDefinition[] +): SQL | undefined { + const columnTypeMap = buildColumnTypeMap(columns) + return buildFilterClauseInternal(filter, tableName, columnTypeMap) +} + +function buildFilterClauseInternal( + filter: Filter, + tableName: string, + columnTypeMap: ColumnTypeMap +): SQL | undefined { const conditions: SQL[] = [] for (const [field, condition] of Object.entries(filter)) { @@ -75,7 +121,7 @@ export function buildFilterClause(filter: Filter, tableName: string): SQL | unde // This represents a case where the filter is a logical OR of multiple filters // e.g. { $or: [{ status: 'active' }, { status: 'pending' }] } if (field === '$or' && Array.isArray(condition)) { - const orClause = buildLogicalClause(condition as Filter[], tableName, 'OR') + const orClause = buildLogicalClause(condition as Filter[], tableName, 'OR', columnTypeMap) if (orClause) { conditions.push(orClause) } @@ -85,7 +131,7 @@ export function buildFilterClause(filter: Filter, tableName: string): SQL | unde // This represents a case where the filter is a logical AND of multiple filters // e.g. { $and: [{ status: 'active' }, { status: 'pending' }] } if (field === '$and' && Array.isArray(condition)) { - const andClause = buildLogicalClause(condition as Filter[], tableName, 'AND') + const andClause = buildLogicalClause(condition as Filter[], tableName, 'AND', columnTypeMap) if (andClause) { conditions.push(andClause) } @@ -103,7 +149,8 @@ export function buildFilterClause(filter: Filter, tableName: string): SQL | unde const fieldConditions = buildFieldCondition( tableName, field, - condition as JsonValue | ConditionOperators + condition as JsonValue | ConditionOperators, + columnTypeMap.get(field) ) conditions.push(...fieldConditions) } @@ -119,26 +166,33 @@ export function buildFilterClause(filter: Filter, tableName: string): SQL | unde * * @param sort - Sort object with field names and directions * @param tableName - Table name for the query (e.g., 'user_table_rows') - * @param columns - Optional column definitions for type-aware sorting + * @param columns - Column definitions; drives type-aware casts (numeric for numbers, timestamptz for dates) * @returns SQL ORDER BY clause or undefined if no sort specified * @throws {TableQueryValidationError} if field name or sort direction is invalid * * @example - * buildSortClause({ name: 'asc', age: 'desc' }, 'user_table_rows') - * // Returns: ORDER BY data->>'name' ASC, data->>'age' DESC + * buildSortClause( + * { name: 'asc' }, + * 'user_table_rows', + * [{ name: 'name', type: 'string' }], + * ) + * // Returns: ORDER BY user_table_rows.data->>'name' ASC * * @example - * // With column types for proper numeric sorting - * buildSortClause({ salary: 'desc' }, 'user_table_rows', [{ name: 'salary', type: 'number' }]) - * // Returns: ORDER BY (data->>'salary')::numeric DESC NULLS LAST + * buildSortClause( + * { salary: 'desc' }, + * 'user_table_rows', + * [{ name: 'salary', type: 'number' }], + * ) + * // Returns: ORDER BY (user_table_rows.data->>'salary')::numeric DESC NULLS LAST */ export function buildSortClause( sort: Sort, tableName: string, - columns?: ColumnDefinition[] + columns: ColumnDefinition[] ): SQL | undefined { const clauses: SQL[] = [] - const columnTypeMap = new Map(columns?.map((col) => [col.name, col.type])) + const columnTypeMap = buildColumnTypeMap(columns) for (const [field, direction] of Object.entries(sort)) { validateFieldName(field) @@ -189,6 +243,31 @@ function validateOperator(operator: string): void { } } +/** + * Validates that a range-operator value matches its column's expected JS type + * before it reaches Postgres. Surfaces an actionable, column-named error at the + * SQL builder layer instead of a generic `invalid input syntax for type numeric` + * from the database. + */ +function validateComparisonValue( + field: string, + columnType: ColumnType | undefined, + cast: 'numeric' | 'timestamptz', + value: number | string +): void { + if (cast === 'numeric' && typeof value !== 'number') { + const label = columnType ?? 'number' + throw new TableQueryValidationError( + `Range operator on column "${field}" (${label}) requires a number, got ${typeof value}` + ) + } + if (cast === 'timestamptz' && typeof value !== 'string') { + throw new TableQueryValidationError( + `Range operator on column "${field}" (date) requires a date string, got ${typeof value}` + ) + } +} + /** * Builds SQL conditions for a single field based on the provided condition. * @@ -208,7 +287,8 @@ function validateOperator(operator: string): void { function buildFieldCondition( tableName: string, field: string, - condition: JsonValue | ConditionOperators + condition: JsonValue | ConditionOperators, + columnType: ColumnType | undefined ): SQL[] { validateFieldName(field) @@ -231,19 +311,27 @@ function buildFieldCondition( break case '$gt': - conditions.push(buildComparisonClause(tableName, field, '>', value as number)) + conditions.push( + buildComparisonClause(tableName, field, '>', value as number | string, columnType) + ) break case '$gte': - conditions.push(buildComparisonClause(tableName, field, '>=', value as number)) + conditions.push( + buildComparisonClause(tableName, field, '>=', value as number | string, columnType) + ) break case '$lt': - conditions.push(buildComparisonClause(tableName, field, '<', value as number)) + conditions.push( + buildComparisonClause(tableName, field, '<', value as number | string, columnType) + ) break case '$lte': - conditions.push(buildComparisonClause(tableName, field, '<=', value as number)) + conditions.push( + buildComparisonClause(tableName, field, '<=', value as number | string, columnType) + ) break case '$in': @@ -312,11 +400,12 @@ function buildFieldCondition( function buildLogicalClause( subFilters: Filter[], tableName: string, - operator: 'OR' | 'AND' + operator: 'OR' | 'AND', + columnTypeMap: ColumnTypeMap ): SQL | undefined { const clauses: SQL[] = [] for (const subFilter of subFilters) { - const clause = buildFilterClause(subFilter, tableName) + const clause = buildFilterClauseInternal(subFilter, tableName, columnTypeMap) if (clause) { clauses.push(clause) } @@ -334,15 +423,36 @@ function buildContainmentClause(tableName: string, field: string, value: JsonVal return sql`${sql.raw(`${tableName}.data`)} @> ${jsonObj}::jsonb` } -/** Builds numeric comparison: `(data->>'field')::numeric value` (cannot use GIN index) */ +/** + * Builds a typed range comparison against a JSONB cell. + * + * `number` columns cast both sides to `numeric`; `date` columns cast both sides + * to `timestamptz` so date strings compare chronologically and timezone offsets + * in ISO strings (e.g. `2024-01-01T00:00:00Z`) are preserved rather than + * silently stripped (which would make results depend on the server's TimeZone + * setting). Unknown/other types + * fall back to `numeric` (legacy default — preserves behavior for ad-hoc fields + * with no schema entry). The right-hand value is cast explicitly because + * drizzle parameterizes it as `text`; without the cast, Postgres would compare + * `text text` and silently produce lexicographic results. + * + * Cannot use the GIN index — falls back to a sequential scan over the table's + * rows (bounded by the btree prefix on `table_id`). + */ function buildComparisonClause( tableName: string, field: string, operator: '>' | '>=' | '<' | '<=', - value: number + value: number | string, + columnType: ColumnType | undefined ): SQL { const escapedField = field.replace(/'/g, "''") - return sql`(${sql.raw(`${tableName}.data->>'${escapedField}'`)})::numeric ${sql.raw(operator)} ${value}` + const cast = jsonbCastForType(columnType) ?? 'numeric' + validateComparisonValue(field, columnType, cast, value) + const cell = sql.raw(`(${tableName}.data->>'${escapedField}')::${cast}`) + return cast === 'timestamptz' + ? sql`${cell} ${sql.raw(operator)} ${value}::timestamptz` + : sql`${cell} ${sql.raw(operator)} ${value}` } /** Escapes LIKE/ILIKE wildcard characters so they match literally */ @@ -370,7 +480,7 @@ function buildSortFieldClause( tableName: string, field: string, direction: 'asc' | 'desc', - columnType?: string + columnType: ColumnType | undefined ): SQL { const escapedField = field.replace(/'/g, "''") const directionSql = direction.toUpperCase() @@ -380,18 +490,13 @@ function buildSortFieldClause( } const jsonbExtract = `${tableName}.data->>'${escapedField}'` + const cast = jsonbCastForType(columnType) - // Cast to appropriate type for correct sorting - if (columnType === 'number') { - // Cast to numeric, with NULLS LAST to handle null/invalid values - return sql.raw(`(${jsonbExtract})::numeric ${directionSql} NULLS LAST`) - } - - if (columnType === 'date') { - // Cast to timestamp for chronological sorting - return sql.raw(`(${jsonbExtract})::timestamp ${directionSql} NULLS LAST`) + if (cast === null) { + // Sort as text (string, boolean, json, or unknown types) + return sql.raw(`${jsonbExtract} ${directionSql}`) } - // Default: sort as text (for string, boolean, json, or unknown types) - return sql.raw(`${jsonbExtract} ${directionSql}`) + // NULLS LAST so rows with null/invalid values sort to the bottom regardless of direction + return sql.raw(`(${jsonbExtract})::${cast} ${directionSql} NULLS LAST`) } diff --git a/apps/sim/lib/table/types.ts b/apps/sim/lib/table/types.ts index 5d6b90d8413..52e81b73f77 100644 --- a/apps/sim/lib/table/types.ts +++ b/apps/sim/lib/table/types.ts @@ -163,10 +163,10 @@ export interface TableRow { export interface ConditionOperators { $eq?: ColumnValue $ne?: ColumnValue - $gt?: number - $gte?: number - $lt?: number - $lte?: number + $gt?: number | string + $gte?: number | string + $lt?: number | string + $lte?: number | string $in?: ColumnValue[] $nin?: ColumnValue[] $contains?: string @@ -319,11 +319,9 @@ export interface UpdateRowData { } export interface BulkUpdateData { - tableId: string filter: Filter data: RowData limit?: number - workspaceId: string } export interface BatchUpdateByIdData { @@ -339,10 +337,8 @@ export interface BatchUpdateByIdData { } export interface BulkDeleteData { - tableId: string filter: Filter limit?: number - workspaceId: string } export interface BulkDeleteByIdsData {