From 47e9621faa8139574565968894d7cb9b43f16786 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 28 Jul 2026 17:40:38 -0700 Subject: [PATCH] fix(cleanup): pass array values as scalar binds, incompatible with fetch_types false --- .../payloads/large-value-metadata.ts | 15 ++++- .../payloads/prune-metadata-sql.test.ts | 62 ++++++++++++------- apps/sim/lib/webhooks/polling/utils.test.ts | 5 +- apps/sim/lib/webhooks/polling/utils.ts | 7 ++- 4 files changed, 61 insertions(+), 28 deletions(-) diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.ts b/apps/sim/lib/execution/payloads/large-value-metadata.ts index 6fedd8d7c63..95d4f43ea98 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.ts @@ -390,13 +390,16 @@ async function pruneStaleReferences( batchSize: number, dbClient: LargeValueMetadataClient ): Promise { + // `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' @@ -431,13 +434,16 @@ async function pruneDeletedParentDependencies( batchSize: number, dbClient: LargeValueMetadataClient ): Promise { + // `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 @@ -466,13 +472,16 @@ async function pruneDeletedLargeValueTombstones( batchSize: number, dbClient: LargeValueMetadataClient ): Promise { + // `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 ( diff --git a/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts b/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts index 0d5daa27e0a..0205ee7b460 100644 --- a/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts +++ b/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts @@ -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') @@ -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 { +async function renderPruneStatements(workspaceIds: string[]): Promise { const dialect = new PgDialect() - const rendered: string[] = [] + const rendered: RenderedQuery[] = [] const capturingClient = { execute: async (query: Parameters[0]) => { - rendered.push(dialect.sqlToQuery(query).sql) + rendered.push(dialect.sqlToQuery(query)) return [{ count: 0 }] }, } @@ -39,25 +49,33 @@ async function renderPruneStatements(workspaceIds: string[]): Promise } 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) + } + } + }) + }) + } }) diff --git a/apps/sim/lib/webhooks/polling/utils.test.ts b/apps/sim/lib/webhooks/polling/utils.test.ts index 7e988dcca11..ace42a66e9d 100644 --- a/apps/sim/lib/webhooks/polling/utils.test.ts +++ b/apps/sim/lib/webhooks/polling/utils.test.ts @@ -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(), @@ -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 () => { diff --git a/apps/sim/lib/webhooks/polling/utils.ts b/apps/sim/lib/webhooks/polling/utils.ts index 32a9c82a111..8064d8ab928 100644 --- a/apps/sim/lib/webhooks/polling/utils.ts +++ b/apps/sim/lib/webhooks/polling/utils.ts @@ -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)