Merge branch 'fix/system-indexes-345' into 'main'

Index checks: never report system-catalog indexes (H001/H002/H004, rarely_used, index_definitions)

Closes #345

See merge request postgres-ai/postgresai!407

Author: Sarumyan

CHANGELOG.md

@@ -2,4 +2,5 @@
 
 ## Unreleased
 
+- Fixed index health checks recommending that built-in catalog indexes be dropped. H001 (invalid indexes), H002 (unused indexes) and H004 (redundant indexes) — and the `unused_indexes`, `redundant_indexes`, `rarely_used_indexes` and `index_definitions` pgwatch metrics behind them — no longer report indexes in `pg_catalog`, `information_schema`, `pg_toast` or per-backend temp schemas. Such indexes cannot be dropped, so recommending their removal was always wrong; `pg_catalog.pg_class_tblspc_relfilenode_index` was the case that surfaced it. Expect a one-off step drop in unused/redundant index counts and total sizes on the first report after upgrading: the catalog rows that were being counted are simply gone. Bloat (F004/F005) and wraparound (F002) still include catalogs on purpose.
 - Fixed express checkup on databases without the optional `postgres_ai` schema. F004/F005 now degrade with machine-readable status and warning summaries instead of reporting a misleading healthy empty result.

cli/lib/checkup.ts

@@ -54,7 +54,7 @@ import { Client } from "pg";
 import * as fs from "fs";
 import * as path from "path";
 import * as pkg from "../package.json";
-import { getMetricSql, transformMetricRow, METRIC_NAMES } from "./metrics-loader";
+import { getMetricSql, transformMetricRow, isSystemSchema, METRIC_NAMES } from "./metrics-loader";
 import { buildCheckInfoMap } from "./checkup-dictionary";
 import { generateCheckSummary, CheckSummary } from "./checkup-summary";
 
@@ -1058,7 +1058,9 @@ export async function getInvalidIndexes(client: Client, pgMajorVersion: number =
       valid_duplicate_name: transformed.valid_index_name ? String(transformed.valid_index_name) : null,
       valid_duplicate_definition: transformed.valid_index_definition ? String(transformed.valid_index_definition) : null,
     };
-  });
+  // #345: the metric SQL is the primary filter; this second pass keeps
+  // catalog rows out of the report JSON if a future SQL edit re-admits them.
+  }).filter((index) => !isSystemSchema(index.schema_name));
 }
 
 /**
@@ -1087,7 +1089,8 @@ export async function getUnusedIndexes(client: Client, pgMajorVersion: number =
       supports_fk: toBool(transformed.supports_fk),
       index_size_pretty: formatBytes(indexSizeBytes),
     };
-  });
+  // #345: never report catalog indexes (see isSystemSchema).
+  }).filter((index) => !isSystemSchema(index.schema_name));
 }
 
 /**
@@ -1242,7 +1245,8 @@ export async function getRedundantIndexes(client: Client, pgMajorVersion: number
     }
 
     return result;
-  });
+  // #345: never report catalog indexes (see isSystemSchema).
+  }).filter((index) => !isSystemSchema(index.schema_name));
 }
 
 /**

cli/lib/metrics-loader.ts

@@ -22,21 +22,33 @@ export function getMetricSql(metricName: string, pgMajorVersion: number = 16): s
     throw new Error(`Metric "${metricName}" not found. Available metrics: ${Object.keys(METRICS).join(", ")}`);
   }
 
-  // Find the best matching version: highest version <= pgMajorVersion
+  // Find the best matching version: highest version <= pgMajorVersion.
+  // Keep each original key next to its parsed number — parseInt("9.6") is 9,
+  // and looking 9 back up would miss the "9.6" entry and yield undefined
+  // instead of raising. Non-numeric keys are dropped rather than sorted as NaN.
   const availableVersions = Object.keys(metric.sqls)
-    .map(v => parseInt(v, 10))
-    .sort((a, b) => b - a); // Sort descending
+    .map(key => ({ key, version: parseInt(key, 10) }))
+    .filter(entry => Number.isFinite(entry.version))
+    .sort((a, b) => b.version - a.version); // Sort descending
 
-  const matchingVersion = availableVersions.find(v => v <= pgMajorVersion);
+  const match = availableVersions.find(entry => entry.version <= pgMajorVersion);
 
-  if (matchingVersion === undefined) {
+  if (match === undefined) {
     throw new Error(
       `No compatible SQL version for metric "${metricName}" with PostgreSQL ${pgMajorVersion}. ` +
-      `Available versions: ${availableVersions.join(", ")}`
+      `Available versions: ${availableVersions.map(entry => entry.key).join(", ")}`
     );
   }
 
-  return metric.sqls[matchingVersion];
+  const sql = (metric.sqls as unknown as Record<string, string>)[match.key];
+
+  if (!sql) {
+    throw new Error(
+      `Metric "${metricName}" has no SQL under version key "${match.key}"`
+    );
+  }
+
+  return sql;
 }
 
 /**
@@ -95,6 +107,25 @@ export const METRIC_NAMES = {
   I001: "pg_stat_io",
 } as const;
 
+/** Schemas whose objects belong to PostgreSQL itself, not to the user. */
+const SYSTEM_SCHEMAS = new Set(["pg_catalog", "information_schema", "pg_toast"]);
+
+/**
+ * True for a PostgreSQL-owned schema (#345).
+ * Catalog and temp indexes cannot be dropped, so index-health checks must
+ * never report them. The metric SQL is the primary filter; applying this in
+ * the getters keeps the report JSON clean if a future SQL edit re-admits a
+ * system schema.
+ */
+export function isSystemSchema(name: unknown): boolean {
+  const schema = String(name ?? "");
+  return (
+    SYSTEM_SCHEMAS.has(schema) ||
+    schema.startsWith("pg_temp_") ||
+    schema.startsWith("pg_toast_temp_")
+  );
+}
+
 /**
  * Transform a row from metrics query output to JSON report format.
  * Metrics use `tag_` prefix for dimensions; we strip it for JSON reports.