fix: Apply statement_timeout via SET instead of startup param#50
Merged
Conversation
4 tasks
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/driver/postgres/PostgresDriver.ts">
<violation number="1" location="src/driver/postgres/PostgresDriver.ts:1539">
P2: Handle the promise returned by `client.query` (await or `.catch`) so errors from `SET statement_timeout` don’t become unhandled rejections.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Driver as PostgresDriver
participant Pool as pg.Pool
participant Client as pg.Client (Internal)
participant DB as Postgres Server / RDS Proxy
Note over Driver,DB: Pool Initialization
Driver->>Pool: Instantiate Pool
Note right of Driver: CHANGED: statement_timeout is no longer<br/>passed in the startup config object
opt NEW: If options.statementTimeout is set
Driver->>Pool: Register listener for 'connect' event
end
Note over Driver,DB: Connection Lifecycle (per new connection)
Pool->>DB: Open physical connection
DB-->>Pool: Connection established
Pool->>Driver: Emit 'connect' event (Client instance)
opt NEW: In 'connect' event handler
Driver->>Client: NEW: query("SET statement_timeout = ...")
Client->>DB: Execute SET command
Note over Client,DB: Avoids startup parameter rejection<br/>by proxies (e.g., AWS RDS Proxy)
DB-->>Client: Command complete
end
Driver->>Pool: pool.connect() (Request connection for usage)
Pool-->>Driver: Return active Connection
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
tomi
reviewed
Mar 23, 2026
| pool.on("connect", (client: any) => { | ||
| client | ||
| .query( | ||
| `SET statement_timeout = ${options.statementTimeout}`, |
Collaborator
There was a problem hiding this comment.
We also do SET for the schema in obtainMasterConnection. Would be nice to keep these per connection runtime configs in a single place. So either move this also there or move the SET search_path here
Move SET statement_timeout from pool connect event to obtainMasterConnection, alongside SET search_path. Both are per-connection session settings and belong in the same place. This also ensures the SET is awaited before the connection is used, avoiding a potential race with the fire-and-forget pool event handler.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/driver/postgres/PostgresDriver.ts">
<violation number="1" location="src/driver/postgres/PostgresDriver.ts:1199">
P1: `statement_timeout` is now applied only on master connections, so replica/slave queries can run without the configured timeout.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } else { | ||
| await connection.query("SET search_path TO public") | ||
| } | ||
| if (statementTimeout) { |
There was a problem hiding this comment.
P1: statement_timeout is now applied only on master connections, so replica/slave queries can run without the configured timeout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/driver/postgres/PostgresDriver.ts, line 1199:
<comment>`statement_timeout` is now applied only on master connections, so replica/slave queries can run without the configured timeout.</comment>
<file context>
@@ -1189,13 +1189,19 @@ export class PostgresDriver implements Driver {
} else {
await connection.query("SET search_path TO public")
}
+ if (statementTimeout) {
+ await connection.query(
+ `SET statement_timeout = ${statementTimeout}`,
</file context>
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
2 participants
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.
Follow-up to n8n-io/n8n#27295