fix(db): encode Date binds in raw sql, document the fetch_types contract - #6040
fix(db): encode Date binds in raw sql, document the fetch_types contract#6040TheodoreSpeaks wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Documents the Test hardening: render tests assert scalar-only bind params (no arrays, no Reviewed by Cursor Bugbot for commit fb92941. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryFixes Date binds in raw drizzle
Confidence Score: 5/5Safe to merge; the Date-encoding fixes match the established column-encoder pattern and are covered by real-dialect render tests. Production paths bind timestamps through matching schema columns; docs and mock guards reduce recurrence of array/Date raw-sql failures under the app pool options; no remaining defect in the changed lines.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/execution/payloads/large-value-metadata.ts | Tombstone prune binds deletedBefore through executionLargeValues.deletedAt; empty workspace-id guards clarified as intentional no-ops. |
| apps/sim/app/api/v1/logs/filters.ts | Cursor comparisons encode cursorDate via workflowExecutionLogs.startedAt, matching the established timestamp-bind idiom. |
| packages/db/db.ts | Docstring corrected for bidirectional array type-map effects of fetch_types: false and safe raw-SQL patterns. |
| packages/db/scripts/migrate.ts | Documents why migration client must not copy app pool fetch_types: false (postgres-js tag array binds). |
| packages/testing/src/mocks/database.mock.ts | Mock sql.param throws on encoderless arrays/Dates so unit tests surface the same failure classes as production pools. |
| apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts | Render assertions require scalar binds only (no arrays, no Dates) via real PgDialect. |
| apps/sim/lib/webhooks/polling/config-sql.test.ts | New real-dialect test locks key-removal to scalar ARRAY elements, not a single array param. |
Reviews (1): Last reviewed commit: "fix(db): encode Date binds in raw sql, d..." | Re-trigger Greptile
Summary
Follow-up to #6035 and the fetch_types audit. Last night's staging cleanup run failed one layer deeper: statements 1–2 of
pruneLargeValueMetadatanow succeed, and statement 3 (tombstones) — which had never executed before because every prior run died earlier — failed withERR_INVALID_ARG_TYPE.Datebound into a raw drizzlesqltemplate has no encoder, so postgres-js'sunsafepath receives the Date object and dies inBuffer.byteLength. This is a Node-side failure, independent offetch_types(reproduced under both settings), andsql.param(date)without an encoder fails identically. The repo already had the correct idiom with a comment describing this exact failure (lib/global-work/summary.ts): bind through the matching column,sql.param(date, table.timestampColumn)deleted_at < ${sql.param(deletedBefore, executionLargeValues.deletedAt)})app/api/v1/logs/filters.ts— both${cursorDate}binds): any cursor-paginatedGET /api/v1/logsrequest throws. Reproduced end-to-end through a query builder against staging; zero prod occurrences in 3 days, so the path is apparently unexercised — but it's a live public-API bug dating to ~v0.4.8packages/db/db.tsfetch_typesdocstring: the old text ("builds array parsers only", "the one behavior this changes") is what misled fix(cleanup): bind array params as arrays, not expanded value lists #6010 into shipping broken — it now states both directions, all array types, the safe forms, and points at the pinning testpackages/db/scripts/migrate.tsdocumenting why it must not inheritfetch_types: false: script migrations bind JS arrays through postgres-js's own tag, and copying the app's pool options there fails every script migration and blocks startup on every deploy and fresh self-hosted installsql.param(array)andsql.param(date)now throw with pointers to the safe forms, turning every mockedsqlsite's tests into detectors for both classesprune-metadata-sql.test.tsnow asserts every bind is a scalar (no arrays, no Dates); added the missing scalar-params assertion tolive-paused-statuses-sql.test.ts; newconfig-sql.test.tsrenders the polling key-removal expression for real (its existing test passed against both previously-broken forms)Type of Change
Testing
pruneLargeValueMetadataend-to-end against local Postgres with the exact production client options (prepare: false, fetch_types: false): all 3 statements ran with real DELETEs, and a seeded 60-day-old tombstone was actually pruned (tombstonesDeleted: 1)tsc --noEmitclean; lint + full audit suite passChecklist