From dbcfac147befd3721971314e350ae8ddedbef1ed Mon Sep 17 00:00:00 2001 From: Alexey Lesovsky Date: Mon, 18 May 2026 18:10:51 +0500 Subject: [PATCH 1/5] Fix test coverage gaps for pg_stat_statements on PG 17+ (issue #127) 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 --- internal/query/statements_test.go | 74 +++++++++++++++++++------------ 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/internal/query/statements_test.go b/internal/query/statements_test.go index 11c3760..c9736d3 100644 --- a/internal/query/statements_test.go +++ b/internal/query/statements_test.go @@ -18,11 +18,16 @@ func TestSelectStatStatementsTimingQuery(t *testing.T) { {version: 110000, want: PgStatStatementsTimingPG12}, {version: 120000, want: PgStatStatementsTimingPG12}, {version: 130000, want: PgStatStatementsTimingDefault}, + {version: 140000, want: PgStatStatementsTimingDefault}, + {version: 160000, want: PgStatStatementsTimingDefault}, + // PG 17+: blk_read_time/blk_write_time replaced by shared_blk_read_time etc. + {version: 170000, want: PgStatStatementsTimingPG17}, + {version: 180000, want: PgStatStatementsTimingPG17}, } for _, tc := range testcases { got := SelectStatStatementsTimingQuery(tc.version) - assert.Equal(t, tc.want, got) + assert.Equal(t, tc.want, got, "version %d", tc.version) } } @@ -56,8 +61,11 @@ func Test_StatStatementsQueries(t *testing.T) { }) } - t.Run("pg_stat_statements_timing", func(t *testing.T) { - for _, version := range versions { + // Each version in its own sub-test so a missing PG instance skips only that version, + // not the entire timing suite (fixes the t.Skipf-in-loop bug). + for _, version := range versions { + version := version + t.Run(fmt.Sprintf("pg_stat_statements_timing/%d", version), func(t *testing.T) { tmpl := SelectStatStatementsTimingQuery(version) opts := NewOptions(version, "f", "off", 256, "public") q, err := Format(tmpl, opts) @@ -67,32 +75,31 @@ func Test_StatStatementsQueries(t *testing.T) { if err != nil { t.Skipf("postgres %d not available in test environment", version) } + defer conn.Close() _, err = conn.Exec(q) assert.NoError(t, err) + }) + } - conn.Close() - } - }) - - t.Run("pg_stat_statements_wal", func(t *testing.T) { - for _, version := range []int{130000} { - tmpl := PgStatStatementsWalDefault + // WAL stats are available since PG 13; test all supported versions. + for _, version := range []int{130000, 140000, 150000, 160000, 170000, 180000} { + version := version + t.Run(fmt.Sprintf("pg_stat_statements_wal/%d", version), func(t *testing.T) { opts := NewOptions(version, "f", "off", 256, "public") - q, err := Format(tmpl, opts) + q, err := Format(PgStatStatementsWalDefault, opts) assert.NoError(t, err) conn, err := postgres.NewTestConnectVersion(version) if err != nil { t.Skipf("postgres %d not available in test environment", version) } + defer conn.Close() _, err = conn.Exec(q) assert.NoError(t, err) - - conn.Close() - } - }) + }) + } } func TestSelectQueryReportQuery(t *testing.T) { @@ -106,32 +113,41 @@ func TestSelectQueryReportQuery(t *testing.T) { {version: 110000, want: PgStatStatementsReportQueryPG12}, {version: 120000, want: PgStatStatementsReportQueryPG12}, {version: 130000, want: PgStatStatementsReportQueryDefault}, + {version: 140000, want: PgStatStatementsReportQueryDefault}, + {version: 160000, want: PgStatStatementsReportQueryDefault}, + // PG 17+: blk_read_time/blk_write_time replaced by shared_blk_read_time etc. + {version: 170000, want: PgStatStatementsReportQueryPG17}, + {version: 180000, want: PgStatStatementsReportQueryPG17}, } for _, tc := range testcases { got := SelectQueryReportQuery(tc.version) - assert.Equal(t, tc.want, got) + assert.Equal(t, tc.want, got, "version %d", tc.version) } } +// Test_StatStatementsReportQueries runs each version in its own sub-test so a missing +// PG instance skips only that version, not the entire suite (fixes the t.Skipf-in-loop bug). func Test_StatStatementsReportQueries(t *testing.T) { versions := []int{90500, 90600, 100000, 110000, 120000, 130000, 140000, 150000, 160000, 170000, 180000} for _, version := range versions { - tmpl := SelectQueryReportQuery(version) - opts := NewOptions(version, "f", "off", 256, "public") - q, err := Format(tmpl, opts) - assert.NoError(t, err) - - conn, err := postgres.NewTestConnectVersion(version) - if err != nil { - t.Skipf("postgres %d not available in test environment", version) - } + version := version + t.Run(fmt.Sprintf("version/%d", version), func(t *testing.T) { + tmpl := SelectQueryReportQuery(version) + opts := NewOptions(version, "f", "off", 256, "public") + q, err := Format(tmpl, opts) + assert.NoError(t, err) - // Use fake query_id, just test queries are executed with no errors. - _, err = conn.Exec(q, "1234567890") - assert.NoError(t, err) + conn, err := postgres.NewTestConnectVersion(version) + if err != nil { + t.Skipf("postgres %d not available in test environment", version) + } + defer conn.Close() - conn.Close() + // Use fake query_id, just test queries are executed with no errors. + _, err = conn.Exec(q, "1234567890") + assert.NoError(t, err) + }) } } From 1a98105d2c695ac2f0d6bbf787c0ba471ee4aa89 Mon Sep 17 00:00:00 2001 From: Alexey Lesovsky Date: Mon, 18 May 2026 18:17:02 +0500 Subject: [PATCH 2/5] Align versioned query naming: Default always refers to the latest PG version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/query/statements.go | 26 ++++++++++++++------------ internal/query/statements_test.go | 20 ++++++++++---------- internal/query/wal.go | 12 ++++++------ internal/view/view.go | 4 ++-- 4 files changed, 32 insertions(+), 30 deletions(-) diff --git a/internal/query/statements.go b/internal/query/statements.go index 29aa409..29e89e2 100644 --- a/internal/query/statements.go +++ b/internal/query/statements.go @@ -4,8 +4,8 @@ const ( // NOTES: // 1. regexp_replace() removes extra spaces, tabs and newlines from queries - // PgStatStatementsTimingDefault defines default query for getting timings stats from pg_stat_statements view. - PgStatStatementsTimingDefault = "SELECT pg_get_userbyid(p.userid) AS user, d.datname AS database, " + + // PgStatStatementsTimingPG13 defines timing query for pg_stat_statements (PG 13-16). + PgStatStatementsTimingPG13 = "SELECT pg_get_userbyid(p.userid) AS user, d.datname AS database, " + "date_trunc('seconds', round(p.total_plan_time + p.total_exec_time) / 1000 * '1 second'::interval)::text AS all_total, " + "date_trunc('seconds', round(p.blk_read_time) / 1000 * '1 second'::interval)::text AS read_total, " + "date_trunc('seconds', round(p.blk_write_time) / 1000 * '1 second'::interval)::text AS write_total, " + @@ -29,8 +29,9 @@ const ( `regexp_replace({{.PgSSQueryLenFn}}, E'\\s+', ' ', 'g') AS query ` + "FROM {{.PGSSSchema}}.pg_stat_statements p JOIN pg_database d ON d.oid=p.dbid" - // the timing query for Postgres 17 and newer. v17 splits up the read-write timing stats into more granular columns. - PgStatStatementsTimingPG17 = ` + // PgStatStatementsTimingDefault defines timing query for pg_stat_statements (PG 17+). + // PG 17 splits read/write timing into shared_blk_*, local_blk_*, temp_blk_* columns. + PgStatStatementsTimingDefault = ` SELECT pg_get_userbyid(p.userid) AS user, d.datname AS database, date_trunc('seconds', round(p.total_exec_time + p.total_plan_time) / 1000 * '1 second'::interval)::text AS all_total, date_trunc('seconds', round(p.shared_blk_read_time + p.local_blk_read_time + p.temp_blk_read_time) / 1000 * '1 second'::interval)::text AS read_total, @@ -96,8 +97,8 @@ FROM {{.PGSSSchema}}.pg_stat_statements p JOIN pg_database d ON d.oid=p.dbid `regexp_replace({{.PgSSQueryLenFn}}, E'\\s+', ' ', 'g') AS query ` + "FROM {{.PGSSSchema}}.pg_stat_statements p JOIN pg_database d ON d.oid=p.dbid" - // PgStatStatementsReportQueryDefault defines query used for calculating per-statement report based on pg_stat_statements. - PgStatStatementsReportQueryDefault = "WITH totals AS (SELECT " + + // PgStatStatementsReportQueryPG13 defines report query for pg_stat_statements (PG 13-16). + PgStatStatementsReportQueryPG13 = "WITH totals AS (SELECT " + "sum(calls) AS total_calls," + "sum(rows) AS total_rows," + "sum(total_plan_time + total_exec_time) AS total_all_time," + @@ -158,8 +159,9 @@ FROM {{.PGSSSchema}}.pg_stat_statements p JOIN pg_database d ON d.oid=p.dbid "to_char(100*s.wal_bytes/(SELECT coalesce(nullif(total_wal_bytes, 0), 1) FROM totals), 'FM990.00') AS wal_bytes_ratio " + "FROM stmt s JOIN pg_database d ON d.oid=s.dbid LIMIT 1" - // The reports query for Postgres 17 and newer. v17 splits up the read-write timing stats into more granular columns. - PgStatStatementsReportQueryPG17 = ` + // PgStatStatementsReportQueryDefault defines report query for pg_stat_statements (PG 17+). + // PG 17 splits read/write timing into shared_blk_*, local_blk_*, temp_blk_* columns. + PgStatStatementsReportQueryDefault = ` WITH totals AS ( SELECT sum(calls) AS total_calls, @@ -306,9 +308,9 @@ func SelectStatStatementsTimingQuery(version int) string { case version < 130000: return PgStatStatementsTimingPG12 case version >= 170000: - return PgStatStatementsTimingPG17 - default: return PgStatStatementsTimingDefault + default: + return PgStatStatementsTimingPG13 } } @@ -318,8 +320,8 @@ func SelectQueryReportQuery(version int) string { case version < 130000: return PgStatStatementsReportQueryPG12 case version >= 170000: - return PgStatStatementsReportQueryPG17 - default: return PgStatStatementsReportQueryDefault + default: + return PgStatStatementsReportQueryPG13 } } diff --git a/internal/query/statements_test.go b/internal/query/statements_test.go index c9736d3..4d9ff27 100644 --- a/internal/query/statements_test.go +++ b/internal/query/statements_test.go @@ -17,12 +17,12 @@ func TestSelectStatStatementsTimingQuery(t *testing.T) { {version: 100000, want: PgStatStatementsTimingPG12}, {version: 110000, want: PgStatStatementsTimingPG12}, {version: 120000, want: PgStatStatementsTimingPG12}, - {version: 130000, want: PgStatStatementsTimingDefault}, - {version: 140000, want: PgStatStatementsTimingDefault}, - {version: 160000, want: PgStatStatementsTimingDefault}, + {version: 130000, want: PgStatStatementsTimingPG13}, + {version: 140000, want: PgStatStatementsTimingPG13}, + {version: 160000, want: PgStatStatementsTimingPG13}, // PG 17+: blk_read_time/blk_write_time replaced by shared_blk_read_time etc. - {version: 170000, want: PgStatStatementsTimingPG17}, - {version: 180000, want: PgStatStatementsTimingPG17}, + {version: 170000, want: PgStatStatementsTimingDefault}, + {version: 180000, want: PgStatStatementsTimingDefault}, } for _, tc := range testcases { @@ -112,12 +112,12 @@ func TestSelectQueryReportQuery(t *testing.T) { {version: 100000, want: PgStatStatementsReportQueryPG12}, {version: 110000, want: PgStatStatementsReportQueryPG12}, {version: 120000, want: PgStatStatementsReportQueryPG12}, - {version: 130000, want: PgStatStatementsReportQueryDefault}, - {version: 140000, want: PgStatStatementsReportQueryDefault}, - {version: 160000, want: PgStatStatementsReportQueryDefault}, + {version: 130000, want: PgStatStatementsReportQueryPG13}, + {version: 140000, want: PgStatStatementsReportQueryPG13}, + {version: 160000, want: PgStatStatementsReportQueryPG13}, // PG 17+: blk_read_time/blk_write_time replaced by shared_blk_read_time etc. - {version: 170000, want: PgStatStatementsReportQueryPG17}, - {version: 180000, want: PgStatStatementsReportQueryPG17}, + {version: 170000, want: PgStatStatementsReportQueryDefault}, + {version: 180000, want: PgStatStatementsReportQueryDefault}, } for _, tc := range testcases { diff --git a/internal/query/wal.go b/internal/query/wal.go index e1862d6..c17286d 100644 --- a/internal/query/wal.go +++ b/internal/query/wal.go @@ -1,8 +1,8 @@ package query const ( - // PgStatWALDefault defines query for pg_stat_wal (PG 14-17). - PgStatWALDefault = "SELECT 'WAL' AS source, " + + // PgStatWALPG14 defines query for pg_stat_wal (PG 14-17). + PgStatWALPG14 = "SELECT 'WAL' AS source, " + "(SELECT pg_size_pretty(count(1) * pg_size_bytes(current_setting('wal_segment_size'))) AS waldir_size FROM pg_ls_waldir()) AS waldir_size, " + `round(wal_bytes / 1024, 2) AS "wal,KiB", ` + "wal_records AS records, wal_fpi AS fpi, " + @@ -10,9 +10,9 @@ const ( "date_trunc('seconds', now() - stats_reset)::text AS stats_age " + "FROM pg_stat_wal" - // PgStatWALPG18 defines query for pg_stat_wal (PG 18+). + // PgStatWALDefault defines query for pg_stat_wal (PG 18+). // wal_write, wal_sync, wal_write_time, wal_sync_time removed in PG 18. - PgStatWALPG18 = "SELECT 'WAL' AS source, " + + PgStatWALDefault = "SELECT 'WAL' AS source, " + "(SELECT pg_size_pretty(count(1) * pg_size_bytes(current_setting('wal_segment_size'))) AS waldir_size FROM pg_ls_waldir()) AS waldir_size, " + `round(wal_bytes / 1024, 2) AS "wal,KiB", ` + "wal_records AS records, wal_fpi AS fpi, " + @@ -25,8 +25,8 @@ const ( func SelectStatWALQuery(version int) (string, int, [2]int) { if version >= 180000 { // PG 18 removed wal_write/wal_sync columns; stats_age is col 6 and must not be diffed. - return PgStatWALPG18, 7, [2]int{2, 5} + return PgStatWALDefault, 7, [2]int{2, 5} } // cols 2-9: wal,KiB..buffers_full; col 10 (stats_age) excluded. - return PgStatWALDefault, 11, [2]int{2, 9} + return PgStatWALPG14, 11, [2]int{2, 9} } diff --git a/internal/view/view.go b/internal/view/view.go index 4d8a13d..a8f8b60 100644 --- a/internal/view/view.go +++ b/internal/view/view.go @@ -124,7 +124,7 @@ func New() Views { "wal": { Name: "wal", MinRequiredVersion: query.PostgresV14, - QueryTmpl: query.PgStatWALDefault, + QueryTmpl: query.PgStatWALPG14, DiffIntvl: [2]int{2, 9}, Ncols: 11, OrderKey: 0, @@ -135,7 +135,7 @@ func New() Views { }, "statements_timings": { Name: "statements_timings", - QueryTmpl: query.PgStatStatementsTimingDefault, + QueryTmpl: query.PgStatStatementsTimingPG13, DiffIntvl: [2]int{6, 10}, Ncols: 13, OrderKey: 0, From 272fd05f5a4a3642af09cea15b88b239cfcbb09a Mon Sep 17 00:00:00 2001 From: Alexey Lesovsky Date: Mon, 18 May 2026 18:35:34 +0500 Subject: [PATCH 3/5] Fix 'invalid result' when reading tar files recorded after cbfa0a4 (issue #122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- report/report.go | 5 ++++- report/report_test.go | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/report/report.go b/report/report.go index c7262ec..89bc2ce 100644 --- a/report/report.go +++ b/report/report.go @@ -289,8 +289,11 @@ func processData(app *app, v view.View, config Config, dataCh chan data, doneCh } // readMeta creates metadata object from stat.PGresult. +// The metadata query SelectCommonProperties had 7 columns before cbfa0a4 (without +// shared_preload_libraries) and 8 columns after. Accept any result with at least 2 +// columns so that tar files recorded with either version can be read. func readMeta(res stat.PGresult) (metadata, error) { - if res.Nrows != 1 || res.Ncols != 7 { + if res.Nrows != 1 || res.Ncols < 2 { return metadata{}, fmt.Errorf("invalid result") } diff --git a/report/report_test.go b/report/report_test.go index 1dcb16f..2accadf 100644 --- a/report/report_test.go +++ b/report/report_test.go @@ -338,6 +338,25 @@ func Test_readMeta(t *testing.T) { }, want: metadata{version: 140000}, }, + // Reproduces issue #122: shared_preload_libraries was added to SelectCommonProperties + // in cbfa0a4, making it 8 columns. Tar files recorded after that commit have Ncols=8 + // and were incorrectly rejected by the strict "!= 7" check. + { + valid: true, + res: stat.PGresult{ + Values: [][]sql.NullString{ + { + {String: "14.9", Valid: true}, {String: "140009", Valid: true}, + {String: "off", Valid: true}, {String: "100", Valid: true}, {String: "3", Valid: true}, + {String: "pg_stat_statements", Valid: true}, + {String: "false", Valid: true}, {String: "1622828486655396e-6", Valid: true}, + }, + }, + Cols: []string{"version", "version_num", "track_commit_timestamp", "max_connections", "autovacuum_max_workers", "shared_preload_libraries", "recovery", "start_time_unix"}, + Ncols: 8, Nrows: 1, Valid: true, + }, + want: metadata{version: 140009}, + }, {valid: false, res: stat.PGresult{Ncols: 1, Nrows: 1, Valid: true}}, {valid: false, res: stat.PGresult{Ncols: 7, Nrows: 0, Valid: true}}, } From 739c55e0fe128811e004a1c643b2c544d6428808 Mon Sep 17 00:00:00 2001 From: Alexey Lesovsky Date: Mon, 18 May 2026 18:43:10 +0500 Subject: [PATCH 4/5] Add regression test for sizes query with non-default schema (issue #116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/query/sizes_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/internal/query/sizes_test.go b/internal/query/sizes_test.go index ce3d797..884f47b 100644 --- a/internal/query/sizes_test.go +++ b/internal/query/sizes_test.go @@ -30,3 +30,35 @@ func Test_StatSizesQueries(t *testing.T) { }) } } + +// Test_StatSizesQueries_NonDefaultSchema reproduces issue #116: the old query used +// (schemaname||'.'||relname)::regclass which failed for tables in non-default schemas +// when the name required quoting (mixed case, special chars) or the schema wasn't in +// search_path. The fix was to use s.relid (OID) instead. +func Test_StatSizesQueries_NonDefaultSchema(t *testing.T) { + conn, err := postgres.NewTestConnect() + if err != nil { + t.Skipf("postgres not available in test environment") + } + defer conn.Close() + + // Create a non-default schema and a table inside it. + _, err = conn.Exec(`CREATE SCHEMA IF NOT EXISTS test_dbo`) + assert.NoError(t, err) + + _, err = conn.Exec(`CREATE TABLE IF NOT EXISTS test_dbo.t1hlog (id int)`) + assert.NoError(t, err) + + defer func() { + _, _ = conn.Exec(`DROP TABLE IF EXISTS test_dbo.t1hlog`) + _, _ = conn.Exec(`DROP SCHEMA IF EXISTS test_dbo`) + }() + + opts := NewOptions(170000, "f", "off", 256, "public") + q, err := Format(PgTablesSizesDefault, opts) + assert.NoError(t, err) + + // Must not error with "relation does not exist" for tables in non-default schemas. + _, err = conn.Exec(q) + assert.NoError(t, err) +} From 065a899a414d36b781194cb50dfef56031f6c4f5 Mon Sep 17 00:00:00 2001 From: Alexey Lesovsky Date: Mon, 18 May 2026 19:09:50 +0500 Subject: [PATCH 5/5] Fix panic on view switch when column count changes (issue #99) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- top/stat.go | 23 +++++++++++++----- top/stat_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/top/stat.go b/top/stat.go index 6060ce4..59bde54 100644 --- a/top/stat.go +++ b/top/stat.go @@ -286,6 +286,22 @@ func formatInfoString(cfg postgres.Config, state, version, uptime, recovery stri ) } +// alignViewToResult ensures config.view.ColsWidth is consistent with r.Ncols. +// It is called before every render. The Aligned flag alone is insufficient because +// after a view switch the first stat batch may still carry the OLD view's column +// count: SetAlign then populates ColsWidth for the wrong number of columns, and the +// next batch (with the correct column count) skips realignment and reads zero widths +// for missing keys — causing "slice bounds out of range [:-1]" (issue #99). +func alignViewToResult(config *config, r stat.PGresult) { + if config.view.Aligned && len(config.view.ColsWidth) == r.Ncols { + return + } + widthes, cols := align.SetAlign(r, 1000, false) // high limit avoids truncating the last value + config.view.Cols = cols + config.view.ColsWidth = widthes + config.view.Aligned = true +} + // printDbstat prints main Postgres stats on UI. func printDbstat(v *gocui.View, config *config, s stat.Stat) error { // If reading stats failed, print the error occurred and return. @@ -299,12 +315,7 @@ func printDbstat(v *gocui.View, config *config, s stat.Stat) error { } // Align values within columns, use fixed aligning instead of dynamic. - if !config.view.Aligned { - widthes, cols := align.SetAlign(s.Result, 1000, false) // use high limit (1000) to avoid truncating last value. - config.view.Cols = cols - config.view.ColsWidth = widthes - config.view.Aligned = true - } + alignViewToResult(config, s.Result) // Print header. err := printStatHeader(v, s, config) diff --git a/top/stat_test.go b/top/stat_test.go index 5c6b1d3..defc355 100644 --- a/top/stat_test.go +++ b/top/stat_test.go @@ -1,10 +1,13 @@ package top import ( + "database/sql" "fmt" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/lesovsky/pgcenter/internal/postgres" + "github.com/lesovsky/pgcenter/internal/stat" + "github.com/lesovsky/pgcenter/internal/view" "github.com/stretchr/testify/assert" "testing" ) @@ -47,3 +50,63 @@ func Test_formatError(t *testing.T) { assert.Equal(t, tc.want, got) } } + +// Test_alignViewToResult reproduces issue #99: pressing 'x' to cycle pg_stat_statements +// views caused "slice bounds out of range [:-1]" / "zero or negative width, skip". +// +// Root cause: after a view switch, the first stat batch may still carry the OLD view's +// column count. SetAlign fires on it (Aligned=false), populating ColsWidth for N columns. +// The next batch has M > N columns from the new view, but Aligned=true skips SetAlign, +// so ColsWidth[N..M-1] returns 0 → panic or error on truncation. +// +// Fix: alignViewToResult also re-aligns when len(ColsWidth) != r.Ncols. +func Test_alignViewToResult(t *testing.T) { + makeResult := func(ncols int) stat.PGresult { + cols := make([]string, ncols) + row := make([]sql.NullString, ncols) + for i := 0; i < ncols; i++ { + cols[i] = fmt.Sprintf("col%d", i+1) + row[i] = sql.NullString{String: "value", Valid: true} + } + return stat.PGresult{Valid: true, Ncols: ncols, Nrows: 1, Cols: cols, + Values: [][]sql.NullString{row}} + } + + t.Run("first render sets alignment", func(t *testing.T) { + cfg := &config{view: view.View{Aligned: false, ColsWidth: map[int]int{}}} + alignViewToResult(cfg, makeResult(8)) + assert.True(t, cfg.view.Aligned) + assert.Equal(t, 8, len(cfg.view.ColsWidth)) + }) + + t.Run("no re-alignment when column count matches", func(t *testing.T) { + original := map[int]int{0: 99, 1: 99, 2: 99} + cfg := &config{view: view.View{Aligned: true, ColsWidth: original}} + alignViewToResult(cfg, makeResult(3)) + assert.Equal(t, 99, cfg.view.ColsWidth[0], "ColsWidth must not change when counts match") + }) + + t.Run("re-aligns when new result has MORE columns than ColsWidth (was: panic)", func(t *testing.T) { + // Simulate: view was aligned with 5 columns (e.g. statements_general), + // then a batch with 13 columns arrives (e.g. statements_timings after rapid 'x'). + // Before fix: ColsWidth[5..12] = 0 → "slice bounds out of range [:-1]". + cfg := &config{view: view.View{ + Aligned: true, + ColsWidth: map[int]int{0: 10, 1: 10, 2: 10, 3: 10, 4: 10}, + }} + alignViewToResult(cfg, makeResult(13)) + assert.Equal(t, 13, len(cfg.view.ColsWidth)) + for i := 0; i < 13; i++ { + assert.Greater(t, cfg.view.ColsWidth[i], 0, "ColsWidth[%d] must be > 0", i) + } + }) + + t.Run("re-aligns when new result has FEWER columns than ColsWidth", func(t *testing.T) { + cfg := &config{view: view.View{ + Aligned: true, + ColsWidth: map[int]int{0: 10, 1: 10, 2: 10, 3: 10, 4: 10, 5: 10, 6: 10}, + }} + alignViewToResult(cfg, makeResult(4)) + assert.Equal(t, 4, len(cfg.view.ColsWidth)) + }) +}