block / block/pg-sprite

schemadiff: plan-level advisory for an invalid index the diff declines to drop

Open
#89 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.