Skip to content

Our differences to upstream#1

Open
splitice wants to merge 47 commits into
upstreamfrom
master
Open

Our differences to upstream#1
splitice wants to merge 47 commits into
upstreamfrom
master

Conversation

@splitice
Copy link
Copy Markdown

No description provided.

@splitice
Copy link
Copy Markdown
Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d67aa0f04

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/pg/lib/client.js
Comment on lines 177 to 179
con.on('authenticationCleartextPassword', this._handleAuthCleartextPassword.bind(this))
// password request handling
con.on('authenticationMD5Password', this._handleAuthMD5Password.bind(this))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reinstate SCRAM authentication event handling

The client now only registers cleartext/MD5 auth listeners, but the protocol parser still emits authenticationSASL, authenticationSASLContinue, and authenticationSASLFinal for SCRAM flows. On PostgreSQL instances configured for SCRAM-SHA-256, the client never sends the SASL handshake messages, so authentication cannot complete even with correct credentials.

Useful? React with 👍 / 👎.

Comment thread packages/pg/lib/query.js
const utils = require('./utils')

class Query extends EventEmitter {
class Query {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep Query as an EventEmitter for streaming APIs

Changing Query to a plain class removes the evented query interface (row, end, error) that callers rely on when using client.query(new Query(...)) and other streaming patterns. This is a behavioral regression for existing consumers that attach listeners instead of using callbacks/promises.

Useful? React with 👍 / 👎.

Comment thread packages/pg/lib/result.js
const cacheKey = fieldDescriptions.map(desc => desc.dataTypeID + "|" + desc.name).join(',')
parseFn = parserCache.get(cacheKey)
if(!parseFn) {
parseFn = 'return function(rowData){return {'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect rowMode='array' when parsing result rows

Row parsing is now hardcoded to build object literals, so rowMode: 'array' is ignored and results are returned as objects instead of positional arrays. Any caller indexing result rows by position will break once this code path is used.

Useful? React with 👍 / 👎.

Comment thread packages/pg/lib/result.js Outdated
Comment on lines +63 to +64
const cacheKey = fieldDescriptions.map(desc => desc.dataTypeID + "|" + desc.name).join(',')
parseFn = parserCache.get(cacheKey)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include parser context in the Result cache key

The global parser cache key only uses dataTypeID and column name, but compiled parsers also depend on format (text/binary) and the active type parser registry. Reusing cached functions across different clients or parser configurations can silently decode column values with the wrong parser.

Useful? React with 👍 / 👎.

* The SubtleCrypto API for low level crypto operations.
* @type SubtleCrypto
*/
const subtleCrypto = webCrypto.subtle
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve legacy crypto fallback for older Node versions

This module now dereferences webCrypto.subtle at load time without guarding for runtimes that lack WebCrypto. With engines still advertising Node 8+, requiring pg can throw immediately on older Node versions instead of using the previous legacy crypto path.

Useful? React with 👍 / 👎.

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