fix(postgres): key the type parser cache by wire format by WikiRik · Pull Request #18330 · sequelize/sequelize · GitHub
Skip to content

fix(postgres): key the type parser cache by wire format - #18330

Open
WikiRik wants to merge 3 commits into
WikiRik/mariadb-json-column-metadatafrom
WikiRik/postgres-type-parser-format
Open

fix(postgres): key the type parser cache by wire format#18330
WikiRik wants to merge 3 commits into
WikiRik/mariadb-json-column-metadatafrom
WikiRik/postgres-type-parser-format

Conversation

@WikiRik

@WikiRik WikiRik commented Sep 3, 2026

Copy link
Copy Markdown
Member

Pull Request Checklist

  • Have you added new tests to prevent regressions?
  • If a documentation update is necessary, have you opened a PR to the documentation repository?
  • Did you update the typescript typings accordingly (if applicable)?
  • Does the description below contain a link to an existing issue or a description of the issue you are solving?
  • Does the name of your PR follow our conventions?

Description of Changes

Stacked on #18329.

The bug

getTypeParser cached a parser under its OID alone:

const cachedParser = this.#oidParserCache.get(oid);

But pg asks for a parser per OID and wire format. Whichever format was requested first therefore won for both: once a text parser was cached for an OID, a later binary request for the same OID got that text parser back, and vice versa.

This matters because the dialect's parsers all take a string. Handing one to the binary protocol means it gets called with a Buffer.

The fix

Key the cache by format and OID, and only consult the dialect's custom parsers for the text format — letting pg's own parsers serve the binary format:

const cacheKey = `${format ?? 'text'}:${oid}`;
// ...
const customParser = format === 'binary' ? null : this.#getCustomTypeParser(oid);

#getCustomTypeParser now only ever runs for the text format, so it no longer takes a format parameter.

The same bug, from the other direction

The element parsers of arrays and ranges had the mirror-image problem: the requested format was forwarded to them, so a binary request built an array parser wrapped around pg's binary element parser — even though postgres-array only ever hands the element parser a string. They are text parsers now, which is the only thing they can be.

Tests

packages/postgres/src/connection-manager.test.ts covers the cache. The two format-crossover cases are genuine regression tests — verified failing against the parent branch and passing here:

1) does not hand out a text parser for the binary format
2) does not hand out a binary parser for the text format

40 postgres unit tests passing.


🤖 Generated with Claude Code

https://claude.ai/code/session_01EgPPfqYYk6PFGbanp5mAFh

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@WikiRik
WikiRik force-pushed the WikiRik/postgres-type-parser-format branch from fe7406f to 582b0e6 Compare September 3, 2026 12:47
@WikiRik
WikiRik force-pushed the WikiRik/postgres-type-parser-format branch from 582b0e6 to c5b5444 Compare September 3, 2026 13:13
@WikiRik
WikiRik force-pushed the WikiRik/postgres-type-parser-format branch from c5b5444 to 3aa3583 Compare September 3, 2026 16:33
@WikiRik
WikiRik force-pushed the WikiRik/postgres-type-parser-format branch from 3aa3583 to b64187d Compare September 3, 2026 17:53
`getTypeParser` cached a parser under its OID alone, but pg asks for a parser
per OID *and* wire format. Whichever format was requested first therefore won
for both: once a text parser was cached, a later binary request for the same
OID got that text parser, and vice versa.

The dialect's parsers all take a string, so handing one to the binary protocol
means it is called with a Buffer. Key the cache by format and OID, and only
consult the dialect's parsers for the text format, letting pg's own parsers
serve the binary format.

The element parsers of arrays and ranges had the same problem from the other
direction: the requested format was forwarded to them, so a binary request
built an array parser around pg's binary element parser, even though
`postgres-array` only ever hands the element parser a string. They are text
parsers now, which is the only thing they can be.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgPPfqYYk6PFGbanp5mAFh
@WikiRik
WikiRik force-pushed the WikiRik/postgres-type-parser-format branch from b64187d to 73d2a84 Compare September 3, 2026 20:14
Comment thread packages/postgres/src/connection-manager.test.ts Outdated
Comment thread packages/postgres/src/connection-manager.test.ts Outdated
Comment thread packages/postgres/src/connection-manager.test.ts Outdated
Comment thread packages/postgres/src/connection-manager.ts Outdated
Comment thread packages/postgres/src/connection-manager.ts Outdated
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
@WikiRik
WikiRik marked this pull request as ready for review September 3, 2026 20:16
@WikiRik
WikiRik requested a review from a team as a code owner September 3, 2026 20:16
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