Skip to content

fix(cleanup): bind array params as arrays, not expanded value lists - #6010

Open
TheodoreSpeaks wants to merge 1 commit into
stagingfrom
fix/prune-workspace-ids-array
Open

fix(cleanup): bind array params as arrays, not expanded value lists#6010
TheodoreSpeaks wants to merge 1 commit into
stagingfrom
fix/prune-workspace-ids-array

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to fix(cleanup): compare live-paused statuses with IN, not ANY over a row constructor #6004, which fixed the status arrays but missed the same pattern elsewhere. Prod is still logging Failed to prune metadata: ref.workspace_id = ANY(($1)::text[])22P02 Array value must start with "{"
  • Drizzle expands an interpolated JS array into a value list, so ANY(${arr}::text[]) casts a row constructor (or, with one element, a bare scalar) to text[] — Postgres rejects both
  • Fixed the 3 workspaceIds sites in the prune queries with sql.param(...), which binds the whole array as a single parameter (ANY($1::text[])) and is correct at every size, including empty
  • Swept the codebase for the pattern and found one more real instance: lib/webhooks/polling/utils.ts built - ${removedKeys}::text[] for the jsonb key-removal operator, so any polling config update that drops a key would fail the same way. Fixed identically
  • The script-migrations hits use postgres-js's own sql tag, which binds arrays natively — not affected. The mcp/discover IN ${workspaceIds} is guarded against empty upstream — left alone

Type of Change

  • Bug fix

Testing

  • New prune-metadata-sql.test.ts renders the real prune statements (unmocking drizzle + schema, capturing via a fake client) and asserts ANY($1::text[]) — verified it fails on the old form and passes on the fix, for both single and multi id
  • Ran the fixed prune query against staging Postgres with 1 workspace id and with an empty array: both execute (the old form errors)
  • Added sql.param to the shared testing mock and the polling test's local drizzle mock; 126 tests across payloads, webhooks/polling, cleanup pass
  • tsc --noEmit clean; bun run lint and the full audit suite pass

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 28, 2026 6:34pm

Request Review

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches production cleanup DELETEs and webhook config updates; fixes incorrect SQL binding but changes query parameter shape on paths that were failing in prod.

Overview
Fixes production failures where Drizzle interpolated JS arrays into SQL as value lists instead of a single Postgres array parameter, breaking ANY(...::text[]) and jsonb - text[] expressions (e.g. Failed to prune metadata / 22P02).

Large-value metadata cleanup: All three prune DELETE paths now use sql.param(workspaceIds) so workspace filters render as ANY($n::text[]), including single-id and empty chunks.

Webhook polling config: Key removal in updateWebhookProviderConfig wraps removedKeys with sql.param(...) so jsonb - $n::text[] binds correctly when polling state drops undefined keys.

Tests / mocks: Adds prune-metadata-sql.test.ts (real Drizzle dialect, unmocked) to assert the prune statements never emit ANY(($1 and always use ANY($n::text[]). Extends shared and local drizzle mocks with sql.param so existing tests still see array values.

Reviewed by Cursor Bugbot for commit cf599d0. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes Drizzle SQL array binding so Postgres receives a single text[] parameter instead of an expanded value list.

  • Wraps workspaceIds with sql.param(...) in three large-value metadata prune queries (ANY($1::text[]))
  • Same fix for removedKeys in webhook provider-config JSONB key removal
  • Adds rendering tests for prune SQL; extends drizzle mocks with sql.param

Confidence Score: 5/5

Safe to merge; the change correctly binds arrays as single Postgres parameters and is covered by targeted SQL rendering tests.

The four production call sites and mocks implement the standard drizzle escape hatch for array parameters, matching the failure mode described in production (22P02 / record-to-text[] cast), with no remaining incorrect expansion on the changed paths.

Important Files Changed

Filename Overview
apps/sim/lib/execution/payloads/large-value-metadata.ts Binds workspace ID arrays via sql.param in three prune DELETE statements so Postgres accepts text[] casts at any array size.
apps/sim/lib/webhooks/polling/utils.ts Binds removedKeys as a single array param for jsonb - key removal, avoiding the same cast failure on config updates.
apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts Unmocks drizzle/schema and asserts rendered prune SQL uses ANY($n::text[]) for single and multi workspace ids.
packages/testing/src/mocks/database.mock.ts Adds sql.param to the shared createMockSql helper so tests that use param binding do not break.
apps/sim/lib/webhooks/polling/utils.test.ts Local drizzle mock exposes sql.param as identity so removedKeys still appear in captured values.

Reviews (1): Last reviewed commit: "fix(cleanup): bind array params as array..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant