Skip to content

chore: cleanup multigres workaround for statement timeout#1228

Merged
ferhatelmas merged 1 commit into
masterfrom
ferhat/cleanup-multigres
Jul 14, 2026
Merged

chore: cleanup multigres workaround for statement timeout#1228
ferhatelmas merged 1 commit into
masterfrom
ferhat/cleanup-multigres

Conversation

@ferhatelmas

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

chore

What is the current behavior?

Multigres keeps returning zero for statement timeout via current_setting and required top level show instead.

What is the new behavior?

multigres/multigres#1258 fixed the problem so dropping the workaround and making it consistent for engines.

Additional context

Related to #1154 (comment) where the original bug was found.

@ferhatelmas ferhatelmas requested a review from a team as a code owner July 14, 2026 09:03
Copilot AI review requested due to automatic review settings July 14, 2026 09:03

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

Removes a Multigres-specific workaround in tests by switching statement timeout verification from SHOW statement_timeout to current_setting('statement_timeout', ...), aligning behavior across supported Postgres engines.

Changes:

  • Update storage-pg-db test helper to read statement_timeout via current_setting.
  • Update pg-connection test to assert transaction-local statement_timeout via current_setting and remove the extra SHOW query/workaround comments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/test/storage-pg-db.test.ts Reads statement_timeout via current_setting for consistent cross-engine behavior in bucket metadata tests.
src/test/pg-connection.test.ts Verifies scope + statement timeout using a single current_setting-based query and removes the prior SHOW workaround.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/pg-connection.test.ts Outdated
Comment thread src/test/storage-pg-db.test.ts Outdated
@coveralls

coveralls commented Jul 14, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 29320846308

Coverage increased (+0.09%) to 79.466%

Details

  • Coverage increased (+0.09%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 12635
Covered Lines: 10492
Line Coverage: 83.04%
Relevant Branches: 7327
Covered Branches: 5371
Branch Coverage: 73.3%
Branches in Coverage %: Yes
Coverage Strength: 424.04 hits per line

💛 - Coveralls

via current_setting instead of top level show.

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas merged commit d257ec4 into master Jul 14, 2026
26 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/cleanup-multigres branch July 14, 2026 10:16
@GuptaManan100

Copy link
Copy Markdown
Contributor

Awesome! Thank you!

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.

5 participants