schemadiff: plan-level advisory for an invalid index the diff declines to drop
- Dominant language
- Go
- Stars
- 7
- Forks
- 1
- Avg merge
- 18h 2m
- Merged PRs (30d)
- 74
Description
## Problem
`schemadiff.Diff` never emits a `DROP INDEX` for an invalid index on a plain table: the entry is an unfinished concurrent build — abandoned, or still running — and a drop by name cannot tell the two apart (OC-2, LK-5). That rule has two consequences the plan cannot express today:
1. **Removed invalid index → empty plan.** When the desired file no longer names the index, the observation is discarded and `diff` reports the table as converged. An orchestrator re-planning after a crash sees "no changes" while the table carries debris.
2. **A non-destructive create that destroys.** When the desired file names the index (same or redefined definition), `diff` plans a single `create-index` with `Destructive: false`. That statement cannot run as-is against the occupied name; completing the plan through `executor.RebuildAbandonedIndex` renames and drops a relation. Nothing in the plan says so.
Both are documented as deliberate trades (`docs/invariants.md` OC-2, `docs/limitations.md` desired-file rows, `docs/pull.md`), and both are correct fail-closed behaviour for the changes themselves. The gap is observability: the plan is the contract an orchestrator reads, and today the plan is silent.
## Proposal
Add a typed, non-fatal advisory alongside `[]Change` in the diff result — one entry per invalid index the diff observed and declined to drop, carrying the index name, the table, and whether desired names it. Surface it in `diff --json` and in the human report. Machine-readable first (a typed field, not prose), so `demo/tour.sh` and orchestrator adapters can assert on it.
Constraints:
- Additive to the plan JSON contract; needs a `format_version` bump, coordinated with any other pending bump so consumers see one version step.
- Advisory only — the change list stays as it is, and the destructive gate is unchanged.
- Not an execution path: the "prove and drop" itself stays in `executor.RebuildAbandonedIndex` and the runbook.
## Acceptance
- Removed-invalid case: `diff` returns zero changes and one advisory naming the entry.
- Matching- and redefined-invalid cases: `diff` returns the create and one advisory stating that completion goes through the proven-removal path.
- Partitioned parent: no advisory (an invalid parent index is not an unfinished build).
- Integration tests on real debris (the `TestDiffRebuildsInvalidIndex` fixture) for all of the above; `demo/tour.sh` asserts the JSON field.
Contributor guide
Research direction
Start with the schemadiff.Diff result and the existing diff --json and human-report paths, then read the TestDiffRebuildsInvalidIndex fixture and demo/tour.sh assertions. Trace how invalid indexes are observed and how executor.RebuildAbandonedIndex is documented, without changing execution behavior. Done means removed, matching, and redefined invalid-index cases expose the typed advisory, partitioned parents do not, and integration tests cover the JSON contract and format-version bump.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, postgresql
- Domain
- databases
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100