Skip to content

Commit a668a77

Browse files
committed
feat(H001): add decision support fields and recommendations for invalid indexes
Extends H001 check with data to support the decision flowchart for invalid index remediation: - Add SQL fields: is_unique_constraint, is_primary_key, table_row_estimate, has_valid_covering_index - Add recommendation object with action (drop/recreate/uncertain), reason, and SQL - Decision logic: 1. If valid index on same columns exists → DROP (duplicate) 2. If backs UNIQUE/PK constraint → REINDEX INDEX CONCURRENTLY 3. If table < 10K rows → DROP INDEX CONCURRENTLY and monitor 4. Otherwise → uncertain (needs RCA to check query patterns) - All SQL commands use CONCURRENTLY to avoid blocking - Updates both express (TypeScript CLI) and full (Python reporter) paths
1 parent 6d64dfc commit a668a77

7 files changed

Lines changed: 256 additions & 12 deletions

File tree

cli/lib/checkup.ts

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,18 @@ export interface ClusterMetric {
107107
description: string;
108108
}
109109

110+
/**
111+
* Recommendation for invalid index remediation (H001)
112+
*/
113+
export interface InvalidIndexRecommendation {
114+
/** Recommended action: drop (safe to remove), recreate (use REINDEX INDEX CONCURRENTLY), or uncertain (needs additional RCA) */
115+
action: "drop" | "recreate" | "uncertain";
116+
/** Human-readable explanation for the recommendation */
117+
reason: string;
118+
/** SQL command to execute (DROP INDEX CONCURRENTLY or REINDEX INDEX CONCURRENTLY) */
119+
sql?: string;
120+
}
121+
110122
/**
111123
* Invalid index entry (H001) - matches H001.schema.json invalidIndex
112124
*/
@@ -120,6 +132,16 @@ export interface InvalidIndex {
120132
/** Full CREATE INDEX statement from pg_get_indexdef(), useful for DROP/CREATE migrations */
121133
index_definition: string;
122134
supports_fk: boolean;
135+
/** True if index backs a UNIQUE constraint */
136+
is_unique_constraint: boolean;
137+
/** True if index backs a PRIMARY KEY constraint */
138+
is_primary_key: boolean;
139+
/** Estimated number of rows in the table (from pg_class.reltuples) */
140+
table_row_estimate: number;
141+
/** True if a valid index exists on the same columns (duplicate) */
142+
has_valid_covering_index: boolean;
143+
/** Recommendation for handling this invalid index */
144+
recommendation: InvalidIndexRecommendation;
123145
}
124146

125147
/**
@@ -562,29 +584,104 @@ export async function getClusterInfo(client: Client, pgMajorVersion: number = 16
562584
return info;
563585
}
564586

587+
/**
588+
* Generate recommendation for an invalid index based on decision flowchart.
589+
*
590+
* Decision tree:
591+
* 1. If valid index on same columns exists → DROP (it's a duplicate)
592+
* 2. Else if backs UNIQUE or PK constraint → RECREATE
593+
* 3. Else if table < 10K rows → DROP and monitor
594+
* 4. Else → UNCERTAIN (need to investigate if queries filter on this column)
595+
*/
596+
function generateInvalidIndexRecommendation(
597+
schemaName: string,
598+
indexName: string,
599+
hasValidCoveringIndex: boolean,
600+
isUniqueConstraint: boolean,
601+
isPrimaryKey: boolean,
602+
tableRowEstimate: number
603+
): InvalidIndexRecommendation {
604+
const qualifiedName = schemaName === "public"
605+
? `"${indexName}"`
606+
: `"${schemaName}"."${indexName}"`;
607+
608+
// 1. Valid index on same columns exists → DROP (duplicate)
609+
if (hasValidCoveringIndex) {
610+
return {
611+
action: "drop",
612+
reason: "A valid index already exists on the same column(s). This invalid index is a duplicate and can be safely dropped.",
613+
sql: `DROP INDEX CONCURRENTLY ${qualifiedName};`,
614+
};
615+
}
616+
617+
// 2. Backs UNIQUE or PK constraint → RECREATE
618+
if (isUniqueConstraint || isPrimaryKey) {
619+
const constraintType = isPrimaryKey ? "PRIMARY KEY" : "UNIQUE";
620+
return {
621+
action: "recreate",
622+
reason: `This index backs a ${constraintType} constraint and must be recreated to restore constraint enforcement.`,
623+
sql: `REINDEX INDEX CONCURRENTLY ${qualifiedName};`,
624+
};
625+
}
626+
627+
// 3. Small table (< 10K rows) → DROP and monitor
628+
if (tableRowEstimate < 10000) {
629+
return {
630+
action: "drop",
631+
reason: `The table has only ~${tableRowEstimate.toLocaleString()} rows. For small tables, it's safe to drop the invalid index and monitor query performance. If needed, recreate with REINDEX INDEX CONCURRENTLY.`,
632+
sql: `DROP INDEX CONCURRENTLY ${qualifiedName};`,
633+
};
634+
}
635+
636+
// 4. Large table, no constraint → UNCERTAIN (need to investigate query patterns)
637+
return {
638+
action: "uncertain",
639+
reason: `The table has ~${tableRowEstimate.toLocaleString()} rows. Additional investigation is needed to determine if queries filter on the indexed column(s). If queries depend on this index, use REINDEX INDEX CONCURRENTLY to recreate it. Otherwise, DROP INDEX CONCURRENTLY and monitor.`,
640+
};
641+
}
642+
565643
/**
566644
* Get invalid indexes from the database (H001).
567645
* Invalid indexes are indexes that failed to build (e.g., due to CONCURRENTLY failure).
568646
*
569647
* @param client - Connected PostgreSQL client
570648
* @param pgMajorVersion - PostgreSQL major version (default: 16)
571-
* @returns Array of invalid index entries with size and FK support info
649+
* @returns Array of invalid index entries with size, FK support info, and remediation recommendation
572650
*/
573651
export async function getInvalidIndexes(client: Client, pgMajorVersion: number = 16): Promise<InvalidIndex[]> {
574652
const sql = getMetricSql(METRIC_NAMES.H001, pgMajorVersion);
575653
const result = await client.query(sql);
576654
return result.rows.map((row) => {
577655
const transformed = transformMetricRow(row);
578656
const indexSizeBytes = parseInt(String(transformed.index_size_bytes || 0), 10);
657+
const schemaName = String(transformed.schema_name || "");
658+
const indexName = String(transformed.index_name || "");
659+
const isUniqueConstraint = toBool(transformed.is_unique_constraint);
660+
const isPrimaryKey = toBool(transformed.is_primary_key);
661+
const tableRowEstimate = parseInt(String(transformed.table_row_estimate || 0), 10);
662+
const hasValidCoveringIndex = toBool(transformed.has_valid_covering_index);
663+
579664
return {
580-
schema_name: String(transformed.schema_name || ""),
665+
schema_name: schemaName,
581666
table_name: String(transformed.table_name || ""),
582-
index_name: String(transformed.index_name || ""),
667+
index_name: indexName,
583668
relation_name: String(transformed.relation_name || ""),
584669
index_size_bytes: indexSizeBytes,
585670
index_size_pretty: formatBytes(indexSizeBytes),
586671
index_definition: String(transformed.index_definition || ""),
587672
supports_fk: toBool(transformed.supports_fk),
673+
is_unique_constraint: isUniqueConstraint,
674+
is_primary_key: isPrimaryKey,
675+
table_row_estimate: tableRowEstimate,
676+
has_valid_covering_index: hasValidCoveringIndex,
677+
recommendation: generateInvalidIndexRecommendation(
678+
schemaName,
679+
indexName,
680+
hasValidCoveringIndex,
681+
isUniqueConstraint,
682+
isPrimaryKey,
683+
tableRowEstimate
684+
),
588685
};
589686
});
590687
}

cli/test/checkup.integration.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ describe.skipIf(!!skipReason)("checkup integration: express mode schema compatib
255255
expect(typeof nodeResult.data).toBe("object");
256256
});
257257

258-
test("H001 returns index_definition with CREATE INDEX statement", async () => {
258+
test("H001 returns index_definition with CREATE INDEX statement and recommendation", async () => {
259259
// Create a table and an index, then mark the index as invalid
260260
await client.query(`
261261
CREATE TABLE IF NOT EXISTS test_invalid_idx_table (id serial PRIMARY KEY, value text);
@@ -290,6 +290,21 @@ describe.skipIf(!!skipReason)("checkup integration: express mode schema compatib
290290
expect(testIndex.index_definition).toMatch(/^CREATE INDEX/);
291291
expect(testIndex.index_definition).toContain("test_invalid_idx");
292292
expect(testIndex.index_definition).toContain("test_invalid_idx_table");
293+
294+
// Verify new decision support fields
295+
expect(typeof testIndex.is_unique_constraint).toBe("boolean");
296+
expect(typeof testIndex.is_primary_key).toBe("boolean");
297+
expect(typeof testIndex.table_row_estimate).toBe("number");
298+
expect(typeof testIndex.has_valid_covering_index).toBe("boolean");
299+
300+
// Verify recommendation object
301+
expect(testIndex.recommendation).toBeDefined();
302+
expect(["drop", "recreate", "uncertain"]).toContain(testIndex.recommendation.action);
303+
expect(typeof testIndex.recommendation.reason).toBe("string");
304+
// SQL is present for drop/recreate actions, absent for uncertain
305+
if (testIndex.recommendation.action !== "uncertain") {
306+
expect(testIndex.recommendation.sql).toMatch(/CONCURRENTLY/);
307+
}
293308
} finally {
294309
// Cleanup: restore the index and drop test objects
295310
await client.query(`

config/pgwatch-prometheus/metrics.yml

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1575,6 +1575,7 @@ metrics:
15751575
This metric identifies invalid indexes in the database.
15761576
It provides insights into the number of invalid indexes and their details.
15771577
This metric helps administrators identify and fix invalid indexes to improve database performance.
1578+
Extended with decision-support fields: has_valid_covering_index, is_unique_constraint, is_primary_key, table_row_estimate.
15781579
sqls:
15791580
11: |
15801581
with fk_indexes as ( /* pgwatch_generated */
@@ -1612,7 +1613,22 @@ metrics:
16121613
where
16131614
fi.tag_fk_table_ref = pct.relname
16141615
and fi.tag_opclasses like (array_to_string(pidx.indclass, ', ') || '%')
1615-
) > 0)::int as supports_fk
1616+
) > 0)::int as supports_fk,
1617+
/* Decision support fields for invalid index remediation */
1618+
pidx.indisunique as is_unique_constraint,
1619+
pidx.indisprimary as is_primary_key,
1620+
pct.reltuples::bigint as table_row_estimate,
1621+
/* Check if there's a valid index covering the same columns */
1622+
exists(
1623+
select 1
1624+
from pg_index valid_idx
1625+
join pg_class valid_idx_class on valid_idx_class.oid = valid_idx.indexrelid
1626+
where valid_idx.indrelid = pidx.indrelid
1627+
and valid_idx.indisvalid = true
1628+
and valid_idx.indkey = pidx.indkey
1629+
and valid_idx.indclass = pidx.indclass
1630+
and valid_idx.indexrelid != pidx.indexrelid
1631+
)::int as has_valid_covering_index
16161632
from pg_index pidx
16171633
join pg_class as pci on pci.oid = pidx.indexrelid
16181634
join pg_class as pct on pct.oid = pidx.indrelid

reporter/postgres_reports.py

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -596,17 +596,68 @@ def generate_a007_altered_settings_report(self, cluster: str = "local", node_nam
596596

597597
return self.format_report_data("A007", altered_settings, node_name, postgres_version=self._get_postgres_version_info(cluster, node_name))
598598

599+
@staticmethod
600+
def _generate_invalid_index_recommendation(
601+
schema_name: str,
602+
index_name: str,
603+
has_valid_covering_index: bool,
604+
is_unique_constraint: bool,
605+
is_primary_key: bool,
606+
table_row_estimate: int
607+
) -> Dict[str, Any]:
608+
"""
609+
Generate recommendation for an invalid index based on decision flowchart.
610+
611+
Decision tree:
612+
1. If valid index on same columns exists → DROP (it's a duplicate)
613+
2. Else if backs UNIQUE or PK constraint → RECREATE
614+
3. Else if table < 10K rows → DROP and monitor
615+
4. Else → UNCERTAIN (need to investigate if queries filter on this column)
616+
"""
617+
qualified_name = f'"{index_name}"' if schema_name == "public" else f'"{schema_name}"."{index_name}"'
618+
619+
# 1. Valid index on same columns exists → DROP (duplicate)
620+
if has_valid_covering_index:
621+
return {
622+
"action": "drop",
623+
"reason": "A valid index already exists on the same column(s). This invalid index is a duplicate and can be safely dropped.",
624+
"sql": f"DROP INDEX CONCURRENTLY {qualified_name};"
625+
}
626+
627+
# 2. Backs UNIQUE or PK constraint → RECREATE
628+
if is_unique_constraint or is_primary_key:
629+
constraint_type = "PRIMARY KEY" if is_primary_key else "UNIQUE"
630+
return {
631+
"action": "recreate",
632+
"reason": f"This index backs a {constraint_type} constraint and must be recreated to restore constraint enforcement.",
633+
"sql": f"REINDEX INDEX CONCURRENTLY {qualified_name};"
634+
}
635+
636+
# 3. Small table (< 10K rows) → DROP and monitor
637+
if table_row_estimate < 10000:
638+
return {
639+
"action": "drop",
640+
"reason": f"The table has only ~{table_row_estimate:,} rows. For small tables, it's safe to drop the invalid index and monitor query performance. If needed, recreate with REINDEX INDEX CONCURRENTLY.",
641+
"sql": f"DROP INDEX CONCURRENTLY {qualified_name};"
642+
}
643+
644+
# 4. Large table, no constraint → UNCERTAIN (need to investigate query patterns)
645+
return {
646+
"action": "uncertain",
647+
"reason": f"The table has ~{table_row_estimate:,} rows. Additional investigation is needed to determine if queries filter on the indexed column(s). If queries depend on this index, use REINDEX INDEX CONCURRENTLY to recreate it. Otherwise, DROP INDEX CONCURRENTLY and monitor."
648+
}
649+
599650
def generate_h001_invalid_indexes_report(self, cluster: str = "local", node_name: str = "node-01") -> Dict[
600651
str, Any]:
601652
"""
602653
Generate H001 Invalid Indexes report.
603-
654+
604655
Args:
605656
cluster: Cluster name
606657
node_name: Node name
607-
658+
608659
Returns:
609-
Dictionary containing invalid indexes information
660+
Dictionary containing invalid indexes information with remediation recommendations
610661
"""
611662
logger.info("Generating H001 Invalid Indexes report...")
612663

@@ -647,6 +698,12 @@ def generate_h001_invalid_indexes_report(self, cluster: str = "local", node_name
647698
index_size_bytes = float(item['value'][1]) if item.get('value') else 0
648699
supports_fk = item['metric'].get('supports_fk', '0')
649700

701+
# Decision support fields
702+
is_unique_constraint = bool(int(item['metric'].get('is_unique_constraint', '0')))
703+
is_primary_key = bool(int(item['metric'].get('is_primary_key', '0')))
704+
table_row_estimate = int(float(item['metric'].get('table_row_estimate', '0')))
705+
has_valid_covering_index = bool(int(item['metric'].get('has_valid_covering_index', '0')))
706+
650707
invalid_index = {
651708
"schema_name": schema_name,
652709
"table_name": table_name,
@@ -655,12 +712,24 @@ def generate_h001_invalid_indexes_report(self, cluster: str = "local", node_name
655712
"index_size_bytes": index_size_bytes,
656713
"index_size_pretty": self.format_bytes(index_size_bytes),
657714
"index_definition": index_definitions.get(index_name, "Definition not available"),
658-
"supports_fk": bool(int(supports_fk))
715+
"supports_fk": bool(int(supports_fk)),
716+
"is_unique_constraint": is_unique_constraint,
717+
"is_primary_key": is_primary_key,
718+
"table_row_estimate": table_row_estimate,
719+
"has_valid_covering_index": has_valid_covering_index,
720+
"recommendation": self._generate_invalid_index_recommendation(
721+
schema_name,
722+
index_name,
723+
has_valid_covering_index,
724+
is_unique_constraint,
725+
is_primary_key,
726+
table_row_estimate
727+
)
659728
}
660729

661730
invalid_indexes.append(invalid_index)
662731
total_size += index_size_bytes
663-
732+
664733
# Skip databases with no invalid indexes
665734
if not invalid_indexes:
666735
continue

reporter/schemas/H001.schema.json

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,26 @@
3939
"server_minor_ver": { "type": "string" }
4040
}
4141
},
42+
"recommendation": {
43+
"type": "object",
44+
"additionalProperties": false,
45+
"required": ["action", "reason"],
46+
"properties": {
47+
"action": {
48+
"type": "string",
49+
"enum": ["drop", "recreate", "uncertain"],
50+
"description": "Recommended action: drop (safe to remove), recreate (use REINDEX INDEX CONCURRENTLY), or uncertain (needs additional RCA)"
51+
},
52+
"reason": {
53+
"type": "string",
54+
"description": "Human-readable explanation for the recommendation"
55+
},
56+
"sql": {
57+
"type": "string",
58+
"description": "SQL command to execute (DROP INDEX CONCURRENTLY or REINDEX INDEX CONCURRENTLY)"
59+
}
60+
}
61+
},
4262
"invalidIndex": {
4363
"type": "object",
4464
"additionalProperties": false,
@@ -50,7 +70,12 @@
5070
"index_size_bytes",
5171
"index_size_pretty",
5272
"index_definition",
53-
"supports_fk"
73+
"supports_fk",
74+
"is_unique_constraint",
75+
"is_primary_key",
76+
"table_row_estimate",
77+
"has_valid_covering_index",
78+
"recommendation"
5479
],
5580
"properties": {
5681
"schema_name": { "type": "string" },
@@ -60,7 +85,12 @@
6085
"index_size_bytes": { "type": "number" },
6186
"index_size_pretty": { "type": "string" },
6287
"index_definition": { "type": "string", "description": "Full CREATE INDEX statement from pg_get_indexdef(), useful for DROP/CREATE migrations" },
63-
"supports_fk": { "type": "boolean" }
88+
"supports_fk": { "type": "boolean" },
89+
"is_unique_constraint": { "type": "boolean", "description": "True if index backs a UNIQUE constraint" },
90+
"is_primary_key": { "type": "boolean", "description": "True if index backs a PRIMARY KEY constraint" },
91+
"table_row_estimate": { "type": "number", "description": "Estimated number of rows in the table (from pg_class.reltuples)" },
92+
"has_valid_covering_index": { "type": "boolean", "description": "True if a valid index exists on the same columns (duplicate)" },
93+
"recommendation": { "$ref": "#/$defs/recommendation" }
6494
}
6595
},
6696
"dbEntry": {

0 commit comments

Comments
 (0)