Skip to content
Open
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
6 changes: 4 additions & 2 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,10 @@ 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 +288,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 @@ -509,6 +510,7 @@ export const DELETE = withRouteHandler(
limit: validated.limit,
workspaceId: validated.workspaceId,
},
table,
requestId
)

Expand Down
6 changes: 4 additions & 2 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,10 @@ 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 +179,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 @@ -490,6 +491,7 @@ export const DELETE = withRouteHandler(
limit: validated.limit,
workspaceId: validated.workspaceId,
},
table,
requestId
)

Expand Down
12 changes: 12 additions & 0 deletions apps/sim/lib/copilot/tools/server/table/user-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,11 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
return { success: false, message: 'Workspace ID is required' }
}

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

const requestId = generateId().slice(0, 8)
const result = await queryRows(
args.tableId,
Expand All @@ -483,6 +488,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
sort: args.sort,
limit: args.limit,
offset: args.offset,
columns: table.schema.columns,
},
requestId
)
Expand Down Expand Up @@ -605,6 +611,11 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
return { success: false, message: 'Workspace ID is required' }
}

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

const requestId = generateId().slice(0, 8)
assertNotAborted()
const result = await deleteRowsByFilter(
Expand All @@ -614,6 +625,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
limit: args.limit,
workspaceId,
},
table,
requestId
)

Expand Down
96 changes: 96 additions & 0 deletions apps/sim/lib/table/__tests__/sql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,29 @@
*
* Tests for the table SQL query builder utilities including filter and sort clause generation.
*/
import type { SQL } from 'drizzle-orm'
import { describe, expect, it } from 'vitest'
import { buildFilterClause, buildSortClause } from '../sql'
import type { Filter } from '../types'

/**
* Serializes a drizzle SQL object to a flat string for assertion.
* Works with both real SQL instances (queryChunks) and the test-environment mock (strings/values).
*/
function getRawSqlString(sqlObj: SQL): string {
const obj = sqlObj as unknown as Record<string, unknown>
if (Array.isArray(obj['queryChunks'])) {
return (obj['queryChunks'] as unknown[])
.map((chunk) => {
if (typeof chunk === 'string') return chunk
const raw = chunk as { value?: string[] }
return raw.value?.join('') ?? ''
})
.join('')
}
return JSON.stringify(sqlObj)
}

Comment on lines +17 to +30
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 getRawSqlString relies on undocumented drizzle internals

The helper reads queryChunks from the drizzle SQL object, which is not part of drizzle-orm's public API. The inner shape ({ value?: string[] }) also doesn't match the actual exported StringChunk / Param types in drizzle — a minor drizzle version bump could silently change the internal structure, causing these assertions to always pass (fallback to JSON.stringify) without actually testing the cast.

describe('SQL Builder', () => {
describe('buildFilterClause', () => {
const tableName = 'user_table_rows'
Expand Down Expand Up @@ -171,6 +190,83 @@ describe('SQL Builder', () => {

expect(result).toBeDefined()
})

describe('date column type', () => {
const dateColumns = [{ name: 'birthDate', type: 'date' as const }]

it('should use ::timestamp cast for $gt with date column type', () => {
const filter: Filter = { birthDate: { $gt: '2024-01-01' } }
const result = buildFilterClause(filter, tableName, dateColumns)

expect(result).toBeDefined()
const raw = getRawSqlString(result!)
expect(raw).toContain('::timestamp')
expect(raw).not.toContain('::numeric')
})

it('should use ::timestamp cast for $gte with date column type', () => {
const filter: Filter = { birthDate: { $gte: '2024-01-01' } }
const result = buildFilterClause(filter, tableName, dateColumns)

expect(result).toBeDefined()
const raw = getRawSqlString(result!)
expect(raw).toContain('::timestamp')
expect(raw).not.toContain('::numeric')
})

it('should use ::timestamp cast for $lt with date column type', () => {
const filter: Filter = { birthDate: { $lt: '2024-12-31' } }
const result = buildFilterClause(filter, tableName, dateColumns)

expect(result).toBeDefined()
const raw = getRawSqlString(result!)
expect(raw).toContain('::timestamp')
expect(raw).not.toContain('::numeric')
})

it('should use ::timestamp cast for $lte with date column type', () => {
const filter: Filter = { birthDate: { $lte: '2024-12-31' } }
const result = buildFilterClause(filter, tableName, dateColumns)

expect(result).toBeDefined()
const raw = getRawSqlString(result!)
expect(raw).toContain('::timestamp')
expect(raw).not.toContain('::numeric')
})

it('should use ::timestamp cast for date range ($gte + $lte)', () => {
const filter: Filter = { birthDate: { $gte: '2024-01-01', $lte: '2024-12-31' } }
const result = buildFilterClause(filter, tableName, dateColumns)

expect(result).toBeDefined()
const raw = getRawSqlString(result!)
expect(raw).toContain('::timestamp')
expect(raw).not.toContain('::numeric')
})

it('should use ::timestamp cast inside $and with date column type', () => {
const filter: Filter = {
$and: [{ birthDate: { $gte: '2024-01-01' } }, { birthDate: { $lte: '2024-12-31' } }],
}
const result = buildFilterClause(filter, tableName, dateColumns)

expect(result).toBeDefined()
const raw = getRawSqlString(result!)
expect(raw).toContain('::timestamp')
expect(raw).not.toContain('::numeric')
})

it('should still use ::numeric cast for $gt on a number column', () => {
const numberColumns = [{ name: 'age', type: 'number' as const }]
const filter: Filter = { age: { $gt: 18 } }
const result = buildFilterClause(filter, tableName, numberColumns)

expect(result).toBeDefined()
const raw = getRawSqlString(result!)
expect(raw).toContain('::numeric')
expect(raw).not.toContain('::timestamp')
})
})
})

describe('buildSortClause', () => {
Expand Down
10 changes: 6 additions & 4 deletions apps/sim/lib/table/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1435,7 +1435,7 @@ export async function queryRows(

let whereClause = baseConditions
if (filter && Object.keys(filter).length > 0) {
const filterClause = buildFilterClause(filter, tableName)
const filterClause = buildFilterClause(filter, tableName, options.columns)
Comment thread
cursor[bot] marked this conversation as resolved.
if (filterClause) {
whereClause = and(baseConditions, filterClause)
}
Expand All @@ -1453,7 +1453,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, options.columns)
}

// Execute query
Expand Down Expand Up @@ -1818,7 +1818,7 @@ export async function updateRowsByFilter(
): Promise<BulkOperationResult> {
const tableName = USER_TABLE_ROWS_SQL_NAME

const filterClause = buildFilterClause(data.filter, tableName)
const filterClause = buildFilterClause(data.filter, tableName, (table.schema as TableSchema).columns)
if (!filterClause) {
throw new Error('Filter is required for bulk update')
}
Expand Down Expand Up @@ -2119,17 +2119,19 @@ async function recompactPositions(tableId: string, trx: DbTransaction, minDelete
* Deletes multiple rows matching a filter.
*
* @param data - Bulk delete data
* @param table - Table definition used to emit correct SQL casts in filter expressions
* @param requestId - Request ID for logging
* @returns Bulk operation result
*/
export async function deleteRowsByFilter(
data: BulkDeleteData,
table: TableDefinition,
requestId: string
): Promise<BulkOperationResult> {
const tableName = USER_TABLE_ROWS_SQL_NAME

// Build filter clause
const filterClause = buildFilterClause(data.filter, tableName)
const filterClause = buildFilterClause(data.filter, tableName, (table.schema as TableSchema).columns)
if (!filterClause) {
throw new Error('Filter is required for bulk delete')
}
Expand Down
45 changes: 31 additions & 14 deletions apps/sim/lib/table/sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,13 @@ const ALLOWED_OPERATORS = new Set([
* // Logical operators
* buildFilterClause({ $or: [{ status: 'active' }, { verified: true }] }, 'user_table_rows')
*/
export function buildFilterClause(filter: Filter, tableName: string): SQL | undefined {
export function buildFilterClause(
filter: Filter,
tableName: string,
columns?: ColumnDefinition[]
): SQL | undefined {
Comment thread
cursor[bot] marked this conversation as resolved.
const conditions: SQL[] = []
const columnTypeMap = new Map(columns?.map((col) => [col.name, col.type]))

for (const [field, condition] of Object.entries(filter)) {
if (condition === undefined) {
Expand All @@ -75,7 +80,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', columns)
if (orClause) {
conditions.push(orClause)
}
Expand All @@ -85,7 +90,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', columns)
if (andClause) {
conditions.push(andClause)
}
Expand All @@ -103,7 +108,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)
}
Expand Down Expand Up @@ -208,7 +214,8 @@ function validateOperator(operator: string): void {
function buildFieldCondition(
tableName: string,
field: string,
condition: JsonValue | ConditionOperators
condition: JsonValue | ConditionOperators,
columnType?: string
): SQL[] {
validateFieldName(field)

Expand All @@ -231,19 +238,19 @@ 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':
Expand Down Expand Up @@ -312,11 +319,12 @@ function buildFieldCondition(
function buildLogicalClause(
subFilters: Filter[],
tableName: string,
operator: 'OR' | 'AND'
operator: 'OR' | 'AND',
columns?: ColumnDefinition[]
): SQL | undefined {
const clauses: SQL[] = []
for (const subFilter of subFilters) {
const clause = buildFilterClause(subFilter, tableName)
const clause = buildFilterClause(subFilter, tableName, columns)
if (clause) {
clauses.push(clause)
}
Expand All @@ -334,15 +342,24 @@ function buildContainmentClause(tableName: string, field: string, value: JsonVal
return sql`${sql.raw(`${tableName}.data`)} @> ${jsonObj}::jsonb`
}

/** Builds numeric comparison: `(data->>'field')::numeric <op> value` (cannot use GIN index) */
/**
* Builds a range comparison: `(data->>'field')::<type> <op> value` (cannot use GIN index).
* Uses `::timestamp` when `columnType === 'date'`, otherwise `::numeric`.
*/
function buildComparisonClause(
tableName: string,
field: string,
operator: '>' | '>=' | '<' | '<=',
value: number
value: number | string,
columnType?: string
): SQL {
const escapedField = field.replace(/'/g, "''")
return sql`(${sql.raw(`${tableName}.data->>'${escapedField}'`)})::numeric ${sql.raw(operator)} ${value}`
const extract = sql.raw(`${tableName}.data->>'${escapedField}'`)
const isDate = columnType === 'date'
if (isDate) {
return sql`(${extract})::timestamp ${sql.raw(operator)} ${value}::timestamp`
}
return sql`(${extract})::numeric ${sql.raw(operator)} ${value}`
}

/** Escapes LIKE/ILIKE wildcard characters so they match literally */
Expand Down
Loading