Fix missing FK index detection with richer PostgreSQL index metadata - #38
Conversation
## Summary Replace string parsing of `pg_indexes.indexdef` with catalog-based index metadata from `pg_index`, `pg_class`, `pg_attribute`, `pg_opclass`, and `pg_collation`. ## Why Parsing `CREATE INDEX` text is fragile for expression indexes, partial indexes, opclasses, sort order, collations, and `INCLUDE` columns. PostgreSQL catalogs expose the structured metadata needed to preserve those cases correctly.
## Summary Update `IndexInfo` and its printer to expose key columns, included columns, index method, uniqueness, primary/partial flags, and predicates. ## Why Consumers need both display-oriented index columns and clean key column names. Keeping them separate avoids mixing metadata like `text_pattern_ops`, `DESC`, `COLLATE`, or `INCLUDE` into logical column checks.
## Summary Update `missing_fk_indexes` to use clean key column metadata and add specs for expression, partial, sorted, opclass, collation, `INCLUDE`, and composite index cases. ## Why Foreign key support depends on a usable leftmost key column. Expression and partial indexes should not be treated as general FK coverage, while sorted/opclass/collated indexes and indexes with included columns can still cover the FK when the FK column is leftmost.
## Summary Document the catalog-based index metadata behavior in the README. ## Why `index_info` now reports richer PostgreSQL index metadata, including included columns, index method, uniqueness, primary/partial flags, and predicates. The README also clarifies that `missing_fk_indexes` only treats composite indexes as covering a foreign key when the FK column is the leftmost key column, and that partial indexes are ignored for FK coverage because they do not cover every row. ## Notable Behavior - Expression indexes are displayed correctly but do not cover raw FK lookups. - Partial indexes keep their predicate separate and are ignored for FK coverage. - Operator classes, sort order, and collations stay visible in display columns. - `INCLUDE` columns are shown separately from key columns. - FK detection uses clean key column names instead of parsing rendered index definitions.
Update the logic in `missing_fk_indexes` to correctly identify foreign keys covered by partial indexes with a not-null predicate.
|
@pawurb Hey! I believe this is good to go! it allowed me to fix some false positives in my production app! |
|
@chaadow thanks! I'll have time to look into it and release next week. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a10725792
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def usable_index?(row, column_name:) | ||
| # PostgreSQL can use a composite index for FK checks only when the FK column is leftmost. | ||
| return false unless first_key_column(row) == column_name |
There was a problem hiding this comment.
When the only index on a text FK is created with an explicit collation that differs from the column's collation, the FK trigger probe (fk_column = $1) uses the column/query collation and that index is not a usable support index unless the collations match. This change now reduces every index to first_key_column(row) == column_name, so a table like the new collated_indexed_codes fixture with CREATE INDEX ... (code COLLATE "C") is reported as covered even though code still needs an index using its own collation; the old string comparison would not have hidden that missing index.
Useful? React with 👍 / 👎.
|
thanks, is live! |
2 participants
Summary
Fix
missing_fk_indexesso it uses structured PostgreSQL index metadata instead of parsing rendered index definitions, and enrichindex_infowith more accurate index details.Why
The previous implementation parsed
pg_indexes.indexdefas text. That works for simple indexes, but it becomes fragile or misleading for PostgreSQL index features like expressions, partial predicates, operator classes, sort order, collations, andINCLUDEcolumns.This PR moves index metadata extraction to PostgreSQL catalogs so display data and logical key-column data stay separate.
Details
index_infoindex_infonow exposes richer metadata for each index:columns: rendered key columns, including PostgreSQL-specific details when relevant:lower(email::text)email text_pattern_opscreated_at DESC NULLS LASTemail COLLATE "C"key_columns: clean key column names used for logical checks, without display metadata like opclasses, sort order, or collations.included_columns: columns added throughINCLUDE (...), kept separate because they are not part of the index search key.index_method: index access method, for examplebtree.unique: whether the index is unique.primary: whether the index backs a primary key.partial: whether the index has a predicate.predicate: the partial index condition when present.missing_fk_indexesmissing_fk_indexesnow uses clean key column metadata instead of parsing the renderedcolumnsstring.This fixes FK index detection for:
company_id DESC NULLS LASTstill counts as coveringcompany_id.code text_pattern_opsstill counts as coveringcode.code COLLATE "C"still counts as coveringcode.INCLUDEcolumns:(topic_id) INCLUDE (id)counts becausetopic_idis still the leftmost key column.(user_id, topic_id)does not count as coveringtopic_id, becausetopic_idis not leftmost.(topic_id::text)does not count as covering raw FK lookups ontopic_id.(topic_id) WHERE ...does not count because it does not cover every row needed for FK maintenance.Test Plan
Added specs covering:
INCLUDEcolumns