Skip to content

fix: repair invalid indexes left by a cancelled CREATE INDEX CONCURRENTLY (#598) - #611

Open
christophostertag wants to merge 1 commit into
pgplex:mainfrom
christophostertag:codex/fix-invalid-concurrent-index-598
Open

christophostertag wants to merge 1 commit into
pgplex:mainfrom
christophostertag:codex/fix-invalid-concurrent-index-598

Conversation

@christophostertag

@christophostertag christophostertag commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #598.

Problem

Cancelling a native CREATE INDEX CONCURRENTLY leaves an invalid index behind. On 11678c5, replanning against an unchanged desired schema reports "no changes" because the definition still matches. The invalid index stays in place and never gets repaired.

Fix

  • The IR schema metadata now records validity, readiness and liveness, but only for unhealthy indexes. Healthy fingerprints are unchanged.
  • Constraint-backed index health is recorded separately from managed index definitions.
  • For an unchanged ordinary index definition left invalid by a failed build, the plan generates a fully qualified REINDEX INDEX CONCURRENTLY. PostgreSQL preserves the definition, dependencies and any existing unique protection, so no drop and recreate is needed.
  • Recovery is non-transactional. Pre and post SQL assertions fail the run if the index is unsafe to repair or unusable afterwards.
  • A unique violation during REINDEX surfaces as an explicit duplicate error. Rows and the original index are preserved.
  • A fresh plan can retry after an interrupted REINDEX. Artifacts that are not desired are dropped through the normal dependency-respecting path after repair, without any suffix heuristics.
  • Active build or maintenance detection is conservative: it uses pg_stat_progress_create_index, with a table-lock fallback for progress rows hidden by role visibility. Only affected unhealthy indexes are checked. Intentionally invalid partition parents that are not touched do not block unrelated work.
  • Saved-plan fingerprints are rechecked after interactive approval.

Boundaries and limits

  • An index that is both invalid and has a changed definition is refused, with advice to author the migration or repair the original first.
  • An invalid partitioned index whose desired state is valid needs its attach indexes; this is reported, not repaired.
  • Required unhealthy constraint-backed indexes and dead required indexes are refused. Explicit removal of an undesired index, constraint or table uses ordinary migration semantics.
  • No integrity constraint is ever dropped to force a recovery.
  • This is not a global fence against external DDL running concurrently.

Existing behavior and concurrency for healthy migrations are unchanged.

Validation

Reproduced the false no-op on untouched upstream 11678c582923fc1a27ed2edf37f3503d1fc466a8 with actual interrupted concurrent builds. The final source candidate passes 45 synthetic native CLI cases on PostgreSQL 14.18, 15.13, 16.9, 17.5 and 18.3 (nine per version): both readiness stages, ordinary/unique indexes, rich definitions, duplicate failure, interrupted replacement and retry, and stale saved plans. No generated SQL was manually repaired.

Independent database checks cover exact rows and index definitions, all three validity/readiness/liveness flags, uniqueness before/during/after recovery (including NULLS NOT DISTINCT), materialized views, preserved healthy PK/UNIQUE/FK constraints, hidden-role progress, permission failure, and post-swap interruption cleanup. Intentional partition states and explicit index/constraint/table removal have native regression coverage, including newly active maintenance during saved-plan apply.

Executed local commands include:

go test -count=1 -v ./cmd -run 'TestIssue598(Interrupted|Duplicate|Quoted|PartitionStates)'
go test -count=1 -v ./cmd -run TestIssue598PartitionConstraintRemoval
PGSCHEMA_POSTGRES_VERSION=14 go test -count=1 -v ./cmd -run TestIssue598Partition
go test -count=1 ./internal/diff ./internal/plan ./internal/fingerprint ./cmd/apply -run 'TestIndexRecovery|Test.*Fingerprint|Test.*Retry|Test.*Concurrent'
go vet ./ir/... ./internal/diff/... ./internal/plan/... ./internal/fingerprint/... ./cmd/...

The new actual-interruption regression fails on untouched upstream and passes with this change. Existing create-index and online-DDL golden controls also pass; sqlc 1.30.0 regeneration is deterministic. Local checks use Go 1.26.4 on macOS; the complete upstream CI suite passed on Go 1.24.13/Linux. Its two existing pgvector cases were skipped because embedded-postgres lacks that third-party extension; all new #598 tests ran and passed.

Independent design/code reviews and a separate exact-commit binary verification passed. Automated review reported 5/5 confidence with no actionable findings.

@greptile-apps

greptile-apps Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, or repository-rule violations were identified.

Summary

This PR adds state-aware recovery for invalid indexes left by interrupted concurrent builds.

  • Records unhealthy index validity, readiness, liveness, constraint ownership, and active-operation observations during inspection.
  • Generates isolated precondition, REINDEX INDEX CONCURRENTLY, and postcondition steps for unchanged recoverable indexes.
  • Revalidates saved plans after interactive approval and provides recovery-specific PostgreSQL error guidance.
  • Preserves ordinary dependency-aware removal behavior and explicitly refuses unsupported constraint-backed, changed-definition, dead, and incomplete partition-parent states.
  • Adds focused unit, fingerprint, planner, and native PostgreSQL integration coverage plus operator documentation.

Diagram

sequenceDiagram
    participant Operator
    participant Planner
    participant Inspector
    participant PostgreSQL
    participant Apply

    Operator->>Planner: Generate plan from desired schema
    Planner->>Inspector: Inspect current and desired states
    Inspector->>PostgreSQL: Read index definition and health
    PostgreSQL-->>Inspector: Invalid/live index and activity state
    Inspector-->>Planner: IR with unhealthy IndexState
    Planner->>Planner: Verify unchanged, unconstrained repair candidate
    Planner-->>Operator: Precheck → REINDEX CONCURRENTLY → postcheck
    Operator->>Apply: Approve or load saved plan
    Apply->>Inspector: Revalidate fingerprint and affected health
    Apply->>PostgreSQL: Assert safe recovery state
    Apply->>PostgreSQL: REINDEX INDEX CONCURRENTLY
    Apply->>PostgreSQL: Assert valid, ready, and live
    PostgreSQL-->>Apply: Recovered index or actionable error
Loading

Reviews (1) · Last reviewed commit: "fix: recover interrupted concurrent inde..."

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Index ignore patterns can incorrectly suppress unhealthy managed constraint-backed indexes.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds safe recovery for unhealthy indexes left by interrupted concurrent PostgreSQL index builds.

Changes:

  • Records unhealthy index catalog state and fingerprints it.
  • Plans isolated REINDEX INDEX CONCURRENTLY recovery with safety assertions.
  • Adds stale-plan checks, documentation, and regression coverage.
File Description
cmd/​apply/​apply.go Revalidates plans and improves recovery errors.
cmd/​issue_598_integration_test.go Tests interrupted-build recovery scenarios.
cmd/​plan/​plan.go Prepends index recovery diffs.
docs/​workflow/​online-ddl.mdx Documents recovery behavior and limitations.
internal/​diff/​index_recovery.go Classifies and plans index repairs.
internal/​diff/​index_recovery_test.go Tests recovery classification.
internal/​fingerprint/​index_state_test.go Tests health-state fingerprints.
internal/​plan/​index_recovery.go Generates recovery SQL and assertions.
internal/​plan/​index_recovery_test.go Tests isolation and quoting.
internal/​plan/​rewrite.go Dispatches recovery rewrites.
ir/​index_state.go Builds operational index health metadata.
ir/​inspector.go Integrates index-state inspection.
ir/​ir.go Adds unhealthy index state to the IR.
ir/​queries/​queries.sql Queries unhealthy index catalog state.
ir/​queries/​queries.sql.go Adds generated query bindings.
Files not reviewed (1)
  • ir/queries/queries.sql.go: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ir/index_state.go
Comment on lines +34 to +36
if i.ignoreConfig != nil && (i.ignoreConfig.ShouldIgnoreIndex(row.IndexName) || (row.ConstraintName != "" && i.ignoreConfig.ShouldIgnoreConstraint(row.ConstraintName))) {
continue
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replanning after interrupted CREATE INDEX CONCURRENTLY ignores the invalid index

2 participants