fix(postgres-js): don't override Date serializer, only the parser (#5789)#5976
Open
momomuchu wants to merge 1 commit into
Open
fix(postgres-js): don't override Date serializer, only the parser (#5789)#5976momomuchu wants to merge 1 commit into
momomuchu wants to merge 1 commit into
Conversation
…izzle-team#5789) drizzle(client) neutralized both the inbound parsers AND the outbound serializers for date/timestamp OIDs (1184/1082/1083/1114/1182/1185/1115/1231). Only the parser override was intended (see porsager/postgres#761): it lets Drizzle parse raw text from Postgres itself. The serializer override was unintentional collateral damage: it made postgres-js's Bind() step receive a raw JS `Date` instance instead of a wire-safe string for any `${jsDate}` parameter passed through a raw `sql` template, which throws `TypeError [ERR_INVALID_ARG_TYPE]` from `Buffer.byteLength(date)`. The fix drops the `client.options.serializers[type] = transparentParser` assignment from the date-OID loop, leaving only the parser override. The `json`/`jsonb` serializer overrides just below (unrelated OIDs 114/3802) are untouched. Alternative considered and rejected: keep a serializer override but replace the passthrough with a real serializer (`x => x instanceof Date ? x.toISOString() : x`). Rejected because it duplicates postgres-js's own default Date serializer logic for no benefit, dropping the override entirely lets postgres-js's already-correct default serializer do the job, which is the narrower and more maintainable change (no drift risk if postgres-js changes its own serializer implementation). Adds a source-level regression test (drizzle-orm/tests/postgres-js/driver.test.ts) that exercises the real driver.ts `construct()` against an unconnected postgres-js client (postgres-js is lazy, so no live DB is needed) and asserts the outbound serializer for OID 1184 still matches postgres-js's own default and produces a wire-safe string, while the inbound parser is still neutralized. Also adds an integration-level regression test (integration-tests/tests/pg/postgres-js.test.ts) reproducing the exact reported scenario end-to-end (raw `sql` insert with a Date param against a timestamptz column); this test requires a live Postgres instance and was not executed locally. Closes drizzle-team#5789
8eae44f to
ef7c793
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
drizzle()for the postgres-js driver overrides postgres.js's outbound timestamp/date serializers alongside its inbound parsers, when only the parsers were meant to be overridden (see porsager/postgres#761). This breaks every${jsDate}parameter passed through a rawsqltemplate, throwingTypeError [ERR_INVALID_ARG_TYPE]from postgres.js'sBind()step.Closes #5789
Root cause
src/postgres-js/driver.ts, inconstruct():postgres.js's default serializer for OID 1184 is
x => (x instanceof Date ? x : new Date(x)).toISOString(). After this override the passthrough returns the rawDateunchanged; postgres.js'sBind()then callsBuffer.byteLength(date), which throws.Fix
Drop the serializer override from the date-OID loop; keep only the parser override (the documented intent per porsager/postgres#761). The unrelated json/jsonb serializer overrides (OIDs 114/3802) are untouched.
Alternative considered and rejected: keep a serializer override but make it "actually serialize" (
x => x instanceof Date ? x.toISOString() : x). Rejected, it duplicates postgres.js's own default Date serializer and risks drifting from its implementation. Dropping the override and falling back to postgres.js's default is narrower.Tests
tests/postgres-js/driver.test.ts(new): source-level unit test against the realconstruct()using an unconnected postgres.js client (no live DB, postgres.js only opens a socket on first query). Asserts the outbound serializer for OID 1184 still matches postgres.js's default and produces a wire-safe string, while the inbound parser stays neutralized. RED (before): 1 failed / 1 passed → GREEN (after): 2 passed.integration-tests/tests/pg/postgres-js.test.ts(new case,insert via raw sql with a Date param does not throw (#5789)): end-to-end reproduction against atimestamptzcolumn. This test requires a live Postgres and was not executed in my environment, included for CI/reviewer verification.Verification run locally
vitest run(drizzle-orm package): 568/568 passedtsc -p tests/tsconfig.json --noEmit(strict): 0 errorsdprint check+eslinton changed files: clean