Fix duration column sorting for values with 3+ digit hours (issue #50)

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)
+}

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
 	}
 }

internal/query/statements_test.go

@@ -17,12 +17,17 @@ func TestSelectStatStatementsTimingQuery(t *testing.T) {
 		{version: 100000, want: PgStatStatementsTimingPG12},
 		{version: 110000, want: PgStatStatementsTimingPG12},
 		{version: 120000, want: PgStatStatementsTimingPG12},
-		{version: 130000, 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: PgStatStatementsTimingDefault},
+		{version: 180000, want: PgStatStatementsTimingDefault},
 	}
 
 	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) {
@@ -105,33 +112,42 @@ func TestSelectQueryReportQuery(t *testing.T) {
 		{version: 100000, want: PgStatStatementsReportQueryPG12},
 		{version: 110000, want: PgStatStatementsReportQueryPG12},
 		{version: 120000, want: PgStatStatementsReportQueryPG12},
-		{version: 130000, 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: PgStatStatementsReportQueryDefault},
+		{version: 180000, want: PgStatStatementsReportQueryDefault},
 	}
 
 	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)
+		})
 	}
 }

internal/query/wal.go

@@ -1,18 +1,18 @@
 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, " +
 		`wal_write AS write, wal_sync AS sync, wal_write_time AS "write,ms", wal_sync_time AS "sync,ms", wal_buffers_full AS buffers_full, ` +
 		"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}
 }

internal/stat/postgres.go

@@ -363,20 +363,32 @@ func (r *PGresult) sort(key int, desc bool) {
 		return /* nothing to sort */
 	}
 
-	_, err := strconv.ParseFloat(r.Values[0][key].String, 64)
-	if err == nil {
-		// value is numeric
+	sample := r.Values[0][key].String
+
+	if _, err := strconv.ParseFloat(sample, 64); err == nil {
+		// numeric sort
 		sort.SliceStable(r.Values, func(i, j int) bool {
 			// TODO: handle errors
 			l, _ := strconv.ParseFloat(r.Values[i][key].String, 64)
-			r, _ := strconv.ParseFloat(r.Values[j][key].String, 64)
+			m, _ := strconv.ParseFloat(r.Values[j][key].String, 64)
+			if desc {
+				return l > m /* desc order: 10 -> 0 */
+			}
+			return l < m /* asc order: 0 -> 10 */
+		})
+	} else if _, err := parseDuration(sample); err == nil {
+		// duration sort: handles "HH:MM:SS" and "N days HH:MM:SS" so that values
+		// with 3+ digit hours (e.g. "791:04:45") sort correctly instead of as strings.
+		sort.SliceStable(r.Values, func(i, j int) bool {
+			li, _ := parseDuration(r.Values[i][key].String)
+			lj, _ := parseDuration(r.Values[j][key].String)
 			if desc {
-				return l > r /* desc order: 10 -> 0 */
+				return li > lj
 			}
-			return l < r /* asc order: 0 -> 10 */
+			return li < lj
 		})
 	} else {
-		// value is string
+		// string sort (fallback)
 		sort.SliceStable(r.Values, func(i, j int) bool {
 			if desc {
 				return r.Values[i][key].String > r.Values[j][key].String /* desc order: 'z' -> 'a' */
@@ -386,6 +398,49 @@ func (r *PGresult) sort(key int, desc bool) {
 	}
 }
 
+// parseDuration parses a PostgreSQL interval string into total seconds.
+// Supported formats:
+//   - "HH:MM:SS" where HH may have any number of digits (e.g. "791:04:45")
+//   - "N days HH:MM:SS" / "N day HH:MM:SS" (PostgreSQL interval text output)
+func parseDuration(s string) (int64, error) {
+	var days, hours, minutes, seconds int64
+
+	if idx := strings.Index(s, " day"); idx != -1 {
+		dayStr := strings.TrimSpace(s[:idx])
+		var err error
+		days, err = strconv.ParseInt(dayStr, 10, 64)
+		if err != nil {
+			return 0, fmt.Errorf("parse days in %q: %w", s, err)
+		}
+		rest := strings.TrimSpace(strings.TrimPrefix(s[idx+len(" day"):], "s"))
+		if rest == "" {
+			return days * 86400, nil
+		}
+		s = rest
+	}
+
+	parts := strings.SplitN(s, ":", 3)
+	if len(parts) != 3 {
+		return 0, fmt.Errorf("not a duration %q: expected HH:MM:SS", s)
+	}
+	var err error
+	hours, err = strconv.ParseInt(parts[0], 10, 64)
+	if err != nil {
+		return 0, fmt.Errorf("parse hours in %q: %w", s, err)
+	}
+	minutes, err = strconv.ParseInt(parts[1], 10, 64)
+	if err != nil {
+		return 0, fmt.Errorf("parse minutes in %q: %w", s, err)
+	}
+	// seconds may carry fractional part; truncate to integer
+	secStr := strings.SplitN(parts[2], ".", 2)[0]
+	seconds, err = strconv.ParseInt(secStr, 10, 64)
+	if err != nil {
+		return 0, fmt.Errorf("parse seconds in %q: %w", s, err)
+	}
+	return days*86400 + hours*3600 + minutes*60 + seconds, nil
+}
+
 // diffPair produces a delta of two string values.
 func diffPair(curr, prev string, itv int) (string, error) {
 	if strings.Contains(prev, ".") || strings.Contains(prev, "e") ||