Skip to content

fix: fold timeout into scope query#1154

Open
ferhatelmas wants to merge 3 commits into
masterfrom
ferhat/timeout-fold-scope
Open

fix: fold timeout into scope query#1154
ferhatelmas wants to merge 3 commits into
masterfrom
ferhat/timeout-fold-scope

Conversation

@ferhatelmas

@ferhatelmas ferhatelmas commented Jun 15, 2026

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Refactor for perf

What is the current behavior?

Right now, statement timeout is sent separately which introduces extra round trip.

What is the new behavior?

Delay until the first statement (set_config) and combine them together in the same query to reduce one round trip to db.

Additional context

Additionally, make query text for scope module level, and cache headers/jwt payload for multiple queries in the same connection.

@ferhatelmas ferhatelmas requested a review from a team as a code owner June 15, 2026 09:32
Copilot AI review requested due to automatic review settings June 15, 2026 09:32
@coveralls

coveralls commented Jun 15, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 28108508086

Coverage increased (+0.05%) to 78.573%

Details

  • Coverage increased (+0.05%) from the base build.
  • Patch coverage: 4 uncovered changes across 1 file (39 of 43 lines covered, 90.7%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/internal/database/pg-connection.ts 43 39 90.7%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 12158
Covered Lines: 10005
Line Coverage: 82.29%
Relevant Branches: 7019
Covered Branches: 5063
Branch Coverage: 72.13%
Branches in Coverage %: Yes
Coverage Strength: 414.24 hits per line

💛 - Coveralls

Comment thread src/internal/database/pg-connection.ts
@fenos

fenos commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

It's an interesting approach, but it might beat us in case we never call setScope on a query, and expect the timeout to still work.

Not sure if it's worth the performance gain here (1ms or less) vs a bit of a hidden timeout logic

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Comment thread src/internal/database/pg-connection.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@ferhatelmas

Copy link
Copy Markdown
Member Author

it might beat us in case we never call setScope on a query

that's fine and covered by a test

cache serialization of headers/jwt payload per request

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the ferhat/timeout-fold-scope branch from 827b5b4 to 7e10952 Compare June 24, 2026 14:17
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
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.

4 participants