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>
…ssue #122) Commit cbfa0a4 added shared_preload_libraries to SelectCommonProperties, increasing the metadata column count from 7 to 8. readMeta's strict "Ncols != 7" check rejected all tar files recorded after that commit, printing "invalid result" for every report row. Fix: replace "!= 7" with "< 2" — readMeta only reads column 1 (version_num), so the minimum required is 2 columns. This accepts both old (7-col) and current (8-col) metadata without breaking forward compatibility for any future column additions. Adds a regression test with 8-column metadata that previously failed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In v0.6.4 the pg_tables_size query used: pg_total_relation_size((schemaname||'.'||relname)::regclass) The ::regclass text cast failed with "relation does not exist" for tables in non-default schemas when the name required quoting (mixed case, special characters) or the schema was not in search_path. The fix (OID-based lookup via s.relid) was applied in an earlier commit. This test reproduces the original failure scenario — table in non-default schema test_dbo — and confirms the OID-based query handles it correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Root cause: after switching pg_stat_statements views with 'x', the first stat batch arriving at printDbstat may still carry the OLD view's column count. SetAlign fires on it (Aligned=false) and populates ColsWidth for N columns. The next batch has M != N columns from the new view; Aligned=true skips SetAlign, so ColsWidth[N..M-1] returns 0. In printStatData the zero width produces slice bounds [:-1] → panic, or "zero or negative width" error after the symptom fix in 9adb560. Fix: extract alignViewToResult() from printDbstat. It re-runs SetAlign whenever len(ColsWidth) != r.Ncols, regardless of the Aligned flag. This handles both first-time visits and the post-switch mismatch window. Test: Test_alignViewToResult covers all four scenarios: - first render (Aligned=false, empty ColsWidth) - stable render (Aligned=true, matching column count — no recalc) - MORE columns than ColsWidth had (the original panic path) - FEWER columns than ColsWidth had Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Time columns like "791:04:45" were sorted as strings. String comparison
places "791..." between "79:..." and "77:..." because '1' (49) < ':' (58),
so 791 hours appeared lower than 96-hour values in descending sort.
Fix: add parseDuration() which converts "HH:MM:SS" and "N days HH:MM:SS"
to total seconds. The sort() method now detects duration-formatted values
(sample parses successfully) and uses numeric comparison on total seconds
before falling back to lexicographic string sort.
parseDuration handles:
- "HH:MM:SS" with any number of hour digits ("791:04:45", "96:58:35")
- "N days HH:MM:SS" / "N day HH:MM:SS" (PostgreSQL interval text output)
- fractional seconds (truncated to integer)
Tests: Test_sort_duration reproduces the original issue report exactly;
Test_parseDuration covers the full input space including error cases.
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
Root cause
Time columns use PostgreSQL's interval text format: `HH:MM:SS` where `HH` can have any number of digits for large values. The `sort()` method fell back to string comparison for non-numeric values. String comparison of `"791:04:45"` vs `"79:18:40"` gives `"791..." < "79:..."` because ASCII `'1'` (49) < `':'` (58), placing 791 hours below 79-hour values in descending sort.
Fix
`parseDuration(s string) (int64, error)` converts a PostgreSQL interval string to total seconds:
`sort()` now probes the sample value: if it parses as a duration, numeric comparison on total seconds is used. Otherwise falls back to string sort (existing behavior).
Test plan
Closes #50
🤖 Generated with Claude Code