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
15 changes: 12 additions & 3 deletions apps/sim/lib/execution/payloads/large-value-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,16 @@ async function pruneStaleReferences(
batchSize: number,
dbClient: LargeValueMetadataClient
): Promise<number> {
// `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
if (workspaceIds.length === 0) return 0

const rows = await dbClient.execute<{ count: number }>(sql`
WITH deleted AS (
DELETE FROM ${executionLargeValueReferences} AS ref
WHERE ref.ctid IN (
SELECT ref.ctid
FROM ${executionLargeValueReferences} AS ref
WHERE ref.workspace_id = ANY(${sql.param(workspaceIds)}::text[])
WHERE ref.workspace_id IN ${workspaceIds}
AND (
(
ref.source = 'execution_log'
Expand Down Expand Up @@ -431,13 +434,16 @@ async function pruneDeletedParentDependencies(
batchSize: number,
dbClient: LargeValueMetadataClient
): Promise<number> {
// `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
if (workspaceIds.length === 0) return 0

const rows = await dbClient.execute<{ count: number }>(sql`
WITH deleted AS (
DELETE FROM ${executionLargeValueDependencies} AS dependency
WHERE dependency.ctid IN (
SELECT dependency.ctid
FROM ${executionLargeValueDependencies} AS dependency
WHERE dependency.workspace_id = ANY(${sql.param(workspaceIds)}::text[])
WHERE dependency.workspace_id IN ${workspaceIds}
AND (
EXISTS (
SELECT 1
Expand Down Expand Up @@ -466,13 +472,16 @@ async function pruneDeletedLargeValueTombstones(
batchSize: number,
dbClient: LargeValueMetadataClient
): Promise<number> {
// `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
if (workspaceIds.length === 0) return 0

const rows = await dbClient.execute<{ count: number }>(sql`
WITH deleted AS (
DELETE FROM ${executionLargeValues} AS value
WHERE value.ctid IN (
SELECT value.ctid
FROM ${executionLargeValues} AS value
WHERE value.workspace_id = ANY(${sql.param(workspaceIds)}::text[])
WHERE value.workspace_id IN ${workspaceIds}
AND value.deleted_at IS NOT NULL
AND value.deleted_at < ${deletedBefore}
AND NOT EXISTS (
Expand Down
62 changes: 40 additions & 22 deletions apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
// Renders the real prune statements against the real drizzle dialect and
// schema. These are raw `sql` templates, so a rendering bug only surfaces at
// execution time — the global drizzle/schema mocks would hide it entirely.
//
// The shared client sets `fetch_types: false` (packages/db/db.ts), which skips
// loading array type OIDs. That makes an array *parameter* unserializable —
// postgres.js sends it in a form Postgres rejects with 22P02. Only scalar bind
// params are safe here, so these tests assert no array parameter is emitted.
import { describe, expect, it, vi } from 'vitest'

vi.unmock('drizzle-orm')
Expand All @@ -16,13 +21,18 @@ process.env.DATABASE_URL ??= 'postgresql://user:pass@localhost:5432/test'
const { PgDialect } = await import('drizzle-orm/pg-core')
const { pruneLargeValueMetadata } = await import('@/lib/execution/payloads/large-value-metadata')

interface RenderedQuery {
sql: string
params: unknown[]
}

/** Captures the raw statements `pruneLargeValueMetadata` issues, without a database. */
async function renderPruneStatements(workspaceIds: string[]): Promise<string[]> {
async function renderPruneStatements(workspaceIds: string[]): Promise<RenderedQuery[]> {
const dialect = new PgDialect()
const rendered: string[] = []
const rendered: RenderedQuery[] = []
const capturingClient = {
execute: async (query: Parameters<PgDialect['sqlToQuery']>[0]) => {
rendered.push(dialect.sqlToQuery(query).sql)
rendered.push(dialect.sqlToQuery(query))
return [{ count: 0 }]
},
}
Expand All @@ -39,25 +49,33 @@ async function renderPruneStatements(workspaceIds: string[]): Promise<string[]>
}

describe('pruneLargeValueMetadata SQL', () => {
it('binds workspace ids as one array parameter, not a value list', async () => {
const statements = await renderPruneStatements(['ws-1', 'ws-2'])

expect(statements.length).toBeGreaterThan(0)
for (const statement of statements) {
// `ANY(($1, $2)::text[])` is what interpolating the raw JS array yields —
// Postgres rejects it ("cannot cast type record to text[]"), and with a
// single id `ANY(($1)::text[])` fails as 22P02 instead.
expect(statement).not.toMatch(/ANY\(\(\$\d+/)
expect(statement).toMatch(/ANY\(\$\d+::text\[\]\)/)
}
})
for (const [label, ids] of [
['multiple workspace ids', ['ws-1', 'ws-2']],
['a single workspace id', ['ws-only']],
] as const) {
describe(label, () => {
it('matches workspaces with an IN list of scalar params', async () => {
const statements = await renderPruneStatements([...ids])

it('renders identically for a single workspace id', async () => {
const statements = await renderPruneStatements(['ws-only'])
expect(statements.length).toBeGreaterThan(0)
for (const { sql: text } of statements) {
expect(text).toMatch(/workspace_id IN \(\$\d+/)
// `= IN (...)` is a syntax error Postgres only reports at execution.
expect(text).not.toMatch(/=\s*IN\s*\(/)
// `ANY(($1)::text[])` / `ANY(($1, $2)::text[])` — the row-constructor cast.
expect(text).not.toMatch(/ANY\(\(\$\d+/)
}
})

expect(statements.length).toBeGreaterThan(0)
for (const statement of statements) {
expect(statement).toMatch(/ANY\(\$\d+::text\[\]\)/)
}
})
it('never binds an array as a parameter', async () => {
const statements = await renderPruneStatements([...ids])

for (const { params } of statements) {
for (const param of params) {
expect(Array.isArray(param)).toBe(false)
}
}
})
})
}
})
5 changes: 3 additions & 2 deletions apps/sim/lib/webhooks/polling/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ vi.mock('drizzle-orm', () => {
sqlCalls.push(node)
return node
}
// Identity, so a `sql.param(arr)` value still shows up verbatim in `values`.
// Identity, so an interpolated value still shows up verbatim in `values`.
sql.param = (value: unknown) => value
sql.join = (fragments: unknown[], separator: unknown) => ({ fragments, separator })
return {
sql,
and: vi.fn(),
Expand Down Expand Up @@ -69,7 +70,7 @@ describe('updateWebhookProviderConfig (atomic jsonb merge)', () => {

expect(dbChainMockFns.update).toHaveBeenCalledTimes(1)
expect(allInterpolatedValues()).toContain(JSON.stringify({ historyId: 'h1', nulled: null }))
expect(allInterpolatedValues()).toContainEqual(['cleared'])
expect(allInterpolatedValues()).toContain('cleared')
})

it('uses merge only (no key-removal expression) when nothing is undefined', async () => {
Expand Down
7 changes: 6 additions & 1 deletion apps/sim/lib/webhooks/polling/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,12 @@ export async function updateWebhookProviderConfig(

const merged = sql`COALESCE(${webhook.providerConfig}::jsonb, '{}'::jsonb) || ${JSON.stringify(defined)}::jsonb`
const nextConfig =
removedKeys.length > 0 ? sql`(${merged}) - ${sql.param(removedKeys)}::text[]` : merged
removedKeys.length > 0
? sql`(${merged}) - ARRAY[${sql.join(
removedKeys.map((key) => sql`${key}`),
sql`, `
)}]::text[]`
: merged

await db
.update(webhook)
Expand Down
Loading