Skip to content

feat: add checks for invalid index - #179

Draft
encima wants to merge 8 commits into
mainfrom
feat/add-invalid-index-check
Draft

feat: add checks for invalid index#179
encima wants to merge 8 commits into
mainfrom
feat/add-invalid-index-check

Conversation

@encima

@encima encima commented Aug 4, 2026

Copy link
Copy Markdown
Member

DO NOT MERGE

What kind of change does this PR introduce?

Feature

What is the current behavior?

Invalid indices not linted

What is the new behavior?

Looks for indisvalid in pg_index and alerts on the possible impact (i.e. index not being used by planner)

Additional context

Same as other lints, focusing only on schemas that are customer facing

@soedirgo

soedirgo commented Aug 4, 2026

Copy link
Copy Markdown
Member

You'll need to update bin/installcheck https://github.com/supabase/splinter/actions/runs/30881166942/job/91902682468?pr=179#step:4:112

This test setup is borked

@soedirgo

soedirgo commented Aug 4, 2026

Copy link
Copy Markdown
Member

A few more things:

Potential false positives, we should add them to the docs page:

  • partitioned parent indexes are expected to be invalid (known bug? ref)
  • in-progress CREATE INDEX CONCURRENTLY will have indisvalid = false

mkdocs.yaml needs to be updated with 0030

@encima

encima commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

A few more things:

Potential false positives, we should add them to the docs page:

  • partitioned parent indexes are expected to be invalid (known bug? ref)
  • in-progress CREATE INDEX CONCURRENTLY will have indisvalid = false

mkdocs.yaml needs to be updated with 0030

  • TIL, yep that is not an easy one to filter so will add it
  • For the in-progress, we could add an exclusion to indislive

soedirgo added a commit that referenced this pull request Aug 4, 2026
The test container ran `./bin/installcheck; cp ... || true`, so the trailing
copy decided the container's exit code and every run reported success. A
failing suite still produced a green check — see #179, where the new lint's
regress test errored with `relation "lint.0030_invalid_index" does not exist`
while the `tests` check stayed green.

Copy the artifacts out as before, then exit with installcheck's status.

Also add bin/check_lints.py, which catches the class of omission behind that
failure: a lint that is not loaded by bin/installcheck, not unioned in
queries_are_unionable.sql, undocumented, untested, or missing from the mkdocs
nav. It runs as a pre-commit hook, so it gates every PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@soedirgo

soedirgo commented Aug 4, 2026

Copy link
Copy Markdown
Member

There's a failing check fyi; these should get easier to surface once #180 is merged

Comment thread mkdocs.yaml Outdated
Comment thread lints/0030_invalid_index.sql Outdated
Co-authored-by: Bobbie Soedirgo <31685197+soedirgo@users.noreply.github.com>
Comment thread lints/0030_invalid_index.sql Outdated

@utkarash2991 utkarash2991 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 1 comments. Rest all looks good to me

soedirgo added a commit that referenced this pull request Aug 5, 2026
The test container ran `./bin/installcheck; cp ... || true`, so the trailing
copy decided the container's exit code and every run reported success. A
failing suite still produced a green check — see #179, where the new lint's
regress test errored with `relation "lint.0030_invalid_index" does not exist`
while the `tests` check stayed green.

Copy the artifacts out as before, then exit with installcheck's status.

Also add bin/check_lints.py, which catches the class of omission behind that
failure: a lint that is not loaded by bin/installcheck, not unioned in
queries_are_unionable.sql, undocumented, untested, or missing from the mkdocs
nav. It runs as a pre-commit hook, so it gates every PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@isabellafeitosasilva

Copy link
Copy Markdown

need help

@olirice
olirice marked this pull request as draft August 11, 2026 19:58
@olirice
olirice requested a review from deepthi August 11, 2026 19:59
…lid_index

Adds two cases to test/sql/0030_invalid_index.sql (expected output regenerated
via pg_regress; full suite passes):

- a leaf partition's invalid index IS flagged with concurrent-reindex advice
  (relkind 'i') -- the positive counterpart to the existing partitioned-parent
  ('I') exclusion test, so the partition handling can't silently over-exclude
- an invalid index in an excluded internal schema (repack) is NOT flagged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants