Skip to content

Commit b57cd58

Browse files
fix(cleanup): pass array values as scalar binds, incompatible with fetch_types false (#6035)
1 parent c809845 commit b57cd58

4 files changed

Lines changed: 61 additions & 28 deletions

File tree

apps/sim/lib/execution/payloads/large-value-metadata.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -390,13 +390,16 @@ async function pruneStaleReferences(
390390
batchSize: number,
391391
dbClient: LargeValueMetadataClient
392392
): Promise<number> {
393+
// `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
394+
if (workspaceIds.length === 0) return 0
395+
393396
const rows = await dbClient.execute<{ count: number }>(sql`
394397
WITH deleted AS (
395398
DELETE FROM ${executionLargeValueReferences} AS ref
396399
WHERE ref.ctid IN (
397400
SELECT ref.ctid
398401
FROM ${executionLargeValueReferences} AS ref
399-
WHERE ref.workspace_id = ANY(${sql.param(workspaceIds)}::text[])
402+
WHERE ref.workspace_id IN ${workspaceIds}
400403
AND (
401404
(
402405
ref.source = 'execution_log'
@@ -431,13 +434,16 @@ async function pruneDeletedParentDependencies(
431434
batchSize: number,
432435
dbClient: LargeValueMetadataClient
433436
): Promise<number> {
437+
// `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
438+
if (workspaceIds.length === 0) return 0
439+
434440
const rows = await dbClient.execute<{ count: number }>(sql`
435441
WITH deleted AS (
436442
DELETE FROM ${executionLargeValueDependencies} AS dependency
437443
WHERE dependency.ctid IN (
438444
SELECT dependency.ctid
439445
FROM ${executionLargeValueDependencies} AS dependency
440-
WHERE dependency.workspace_id = ANY(${sql.param(workspaceIds)}::text[])
446+
WHERE dependency.workspace_id IN ${workspaceIds}
441447
AND (
442448
EXISTS (
443449
SELECT 1
@@ -466,13 +472,16 @@ async function pruneDeletedLargeValueTombstones(
466472
batchSize: number,
467473
dbClient: LargeValueMetadataClient
468474
): Promise<number> {
475+
// `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
476+
if (workspaceIds.length === 0) return 0
477+
469478
const rows = await dbClient.execute<{ count: number }>(sql`
470479
WITH deleted AS (
471480
DELETE FROM ${executionLargeValues} AS value
472481
WHERE value.ctid IN (
473482
SELECT value.ctid
474483
FROM ${executionLargeValues} AS value
475-
WHERE value.workspace_id = ANY(${sql.param(workspaceIds)}::text[])
484+
WHERE value.workspace_id IN ${workspaceIds}
476485
AND value.deleted_at IS NOT NULL
477486
AND value.deleted_at < ${deletedBefore}
478487
AND NOT EXISTS (

apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55
// Renders the real prune statements against the real drizzle dialect and
66
// schema. These are raw `sql` templates, so a rendering bug only surfaces at
77
// execution time — the global drizzle/schema mocks would hide it entirely.
8+
//
9+
// The shared client sets `fetch_types: false` (packages/db/db.ts), which skips
10+
// loading array type OIDs. That makes an array *parameter* unserializable —
11+
// postgres.js sends it in a form Postgres rejects with 22P02. Only scalar bind
12+
// params are safe here, so these tests assert no array parameter is emitted.
813
import { describe, expect, it, vi } from 'vitest'
914

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

24+
interface RenderedQuery {
25+
sql: string
26+
params: unknown[]
27+
}
28+
1929
/** Captures the raw statements `pruneLargeValueMetadata` issues, without a database. */
20-
async function renderPruneStatements(workspaceIds: string[]): Promise<string[]> {
30+
async function renderPruneStatements(workspaceIds: string[]): Promise<RenderedQuery[]> {
2131
const dialect = new PgDialect()
22-
const rendered: string[] = []
32+
const rendered: RenderedQuery[] = []
2333
const capturingClient = {
2434
execute: async (query: Parameters<PgDialect['sqlToQuery']>[0]) => {
25-
rendered.push(dialect.sqlToQuery(query).sql)
35+
rendered.push(dialect.sqlToQuery(query))
2636
return [{ count: 0 }]
2737
},
2838
}
@@ -39,25 +49,33 @@ async function renderPruneStatements(workspaceIds: string[]): Promise<string[]>
3949
}
4050

4151
describe('pruneLargeValueMetadata SQL', () => {
42-
it('binds workspace ids as one array parameter, not a value list', async () => {
43-
const statements = await renderPruneStatements(['ws-1', 'ws-2'])
44-
45-
expect(statements.length).toBeGreaterThan(0)
46-
for (const statement of statements) {
47-
// `ANY(($1, $2)::text[])` is what interpolating the raw JS array yields —
48-
// Postgres rejects it ("cannot cast type record to text[]"), and with a
49-
// single id `ANY(($1)::text[])` fails as 22P02 instead.
50-
expect(statement).not.toMatch(/ANY\(\(\$\d+/)
51-
expect(statement).toMatch(/ANY\(\$\d+::text\[\]\)/)
52-
}
53-
})
52+
for (const [label, ids] of [
53+
['multiple workspace ids', ['ws-1', 'ws-2']],
54+
['a single workspace id', ['ws-only']],
55+
] as const) {
56+
describe(label, () => {
57+
it('matches workspaces with an IN list of scalar params', async () => {
58+
const statements = await renderPruneStatements([...ids])
5459

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

58-
expect(statements.length).toBeGreaterThan(0)
59-
for (const statement of statements) {
60-
expect(statement).toMatch(/ANY\(\$\d+::text\[\]\)/)
61-
}
62-
})
70+
it('never binds an array as a parameter', async () => {
71+
const statements = await renderPruneStatements([...ids])
72+
73+
for (const { params } of statements) {
74+
for (const param of params) {
75+
expect(Array.isArray(param)).toBe(false)
76+
}
77+
}
78+
})
79+
})
80+
}
6381
})

apps/sim/lib/webhooks/polling/utils.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ vi.mock('drizzle-orm', () => {
1515
sqlCalls.push(node)
1616
return node
1717
}
18-
// Identity, so a `sql.param(arr)` value still shows up verbatim in `values`.
18+
// Identity, so an interpolated value still shows up verbatim in `values`.
1919
sql.param = (value: unknown) => value
20+
sql.join = (fragments: unknown[], separator: unknown) => ({ fragments, separator })
2021
return {
2122
sql,
2223
and: vi.fn(),
@@ -69,7 +70,7 @@ describe('updateWebhookProviderConfig (atomic jsonb merge)', () => {
6970

7071
expect(dbChainMockFns.update).toHaveBeenCalledTimes(1)
7172
expect(allInterpolatedValues()).toContain(JSON.stringify({ historyId: 'h1', nulled: null }))
72-
expect(allInterpolatedValues()).toContainEqual(['cleared'])
73+
expect(allInterpolatedValues()).toContain('cleared')
7374
})
7475

7576
it('uses merge only (no key-removal expression) when nothing is undefined', async () => {

apps/sim/lib/webhooks/polling/utils.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,12 @@ export async function updateWebhookProviderConfig(
165165

166166
const merged = sql`COALESCE(${webhook.providerConfig}::jsonb, '{}'::jsonb) || ${JSON.stringify(defined)}::jsonb`
167167
const nextConfig =
168-
removedKeys.length > 0 ? sql`(${merged}) - ${sql.param(removedKeys)}::text[]` : merged
168+
removedKeys.length > 0
169+
? sql`(${merged}) - ARRAY[${sql.join(
170+
removedKeys.map((key) => sql`${key}`),
171+
sql`, `
172+
)}]::text[]`
173+
: merged
169174

170175
await db
171176
.update(webhook)

0 commit comments

Comments
 (0)