Conversation
Two structural problems in statements tests: 1. t.Skipf inside a bare for-loop caused the entire timing/report/wal suite to be skipped when the first PG version (9.5) was unavailable. Timing and report query integration tests never ran against PG 17/18. 2. TestSelectStatStatementsTimingQuery and TestSelectQueryReportQuery only asserted version routing up to PG 13, so a regression in the PG 17+ routing would go undetected. Fixes: - Split timing and WAL loops into per-version t.Run sub-tests; missing instances now skip only that version, not the entire suite. - Extend WAL stats test to cover PG 13-18 (was hardcoded to PG 13). - Add PG 14/16/17/18 cases to both unit routing tests, asserting that PG 17+ returns the blk_read_time-free PG17 query variants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…version Convention: PGxx suffix marks a query valid from PG xx (now superseded); Default marks the query for the current/latest supported version. Previously violated by two files: - wal.go: PgStatWALDefault covered PG 14-17 (not latest); PgStatWALPG18 covered PG 18+ (latest). - statements.go: PgStatStatementsTimingDefault / ReportQueryDefault covered PG 13-16; PG17 variants covered PG 17+ (latest). Renames: - PgStatWALDefault → PgStatWALPG14 - PgStatWALPG18 → PgStatWALDefault - PgStatStatementsTimingDefault → PgStatStatementsTimingPG13 - PgStatStatementsTimingPG17 → PgStatStatementsTimingDefault - PgStatStatementsReportQueryDefault → PgStatStatementsReportQueryPG13 - PgStatStatementsReportQueryPG17 → PgStatStatementsReportQueryDefault Updated: selector functions, view.go initial templates, all test references. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
statements_test.gothat caused PG 17/18 to be silently untestedRoot cause
Bug 1 —
t.Skipfinside a bareforloop.The
pg_stat_statements_timingandTest_StatStatementsReportQueriesloops calledt.Skipfwhen a PG instance wasn't available.t.Skipfmarks the current test as skipped and stops execution — so the very first missing version (9.5) caused the entire suite to skip, and PG 17/18 timing/report queries never ran against a real database.Bug 2 — Missing PG 17/18 cases in routing unit tests.
TestSelectStatStatementsTimingQueryandTestSelectQueryReportQueryonly asserted routing up to PG 13 (130000). A regression in the PG 17+ branch would go completely undetected by these tests.Fixes
t.Runsub-tests — missing instances now skip only that versionblk_read_time-free query variantsTest plan
TestSelectStatStatementsTimingQuery— PG 17/18 assertPgStatStatementsTimingPG17TestSelectQueryReportQuery— PG 17/18 assertPgStatStatementsReportQueryPG17pg_stat_statements_timing/170000and/180000— now run and pass against real PGTest_StatStatementsReportQueries/version/170000and/180000— samego test ./...passesCloses #127
🤖 Generated with Claude Code