docs: fix generated-column refusal, reason buckets, dangling refs

[Kiran01bm]

Follow-up corrections to the MySQL-comparison and binary-coercible docs from #68: dependency refusals, temporal-precision rewrite reasons, the complete planner-reason cost map, and dead design-doc references.

Why

This commit was pushed to the #68 branch after the PR had merged, so it never reached main. Its content fixes factual statements readers would otherwise take as PostgreSQL behaviour: ALTER COLUMN TYPE on a column a STORED generated column depends on is refused outright, temporal precision reductions rewrite because the rounding survives, and the planner-reason cost map needs to distinguish online forms that scan from those that do not. It also replaces dead design-document references with their live sources.

What

• binary-coercible-type-changes.md: move the generated-column case next to the view/rule refusal and correct the bucket-B/C wording for rounding versus stored typmod.
• postgres-online-ddl-reference.md: map all planner reasons to their cost buckets, distinguish the two online-idiom cost shapes, and surface dependency refusals beside the binary-coercible verdict.
• low-level-design.md, high-level-design.md, design-principles.md, invariants.md, and postgresql-version-support.md: replace dead references with the Spirit README, PostgreSQL primitive mapping, and current implementation-status section.
• plan-report.md and capabilities.md: align the canonical reason and support tables with planner behaviour and PostgreSQL's dependency refusals.

Before / after

Before (origin/main after #68)              After
generated column + ALTER TYPE:              generated column + ALTER TYPE:
  "not re-recorded; decided on own            "refused: cannot alter type of a column
   transform expression"                       used by a generated column"

reasons -> buckets: incomplete/coarse       reasons -> buckets: complete, with online
                                               no-scan and full-scan forms distinguished

refs: nonexistent design notes              refs: Spirit README, PostgreSQL primitive
      and build-plan files                         mapping, current implementation status

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 Adversarial correctness review, requested by Armand and performed by his agent. Reviewed at head 74322ac0.

Verdict: the two substantive claims check out against a real PostgreSQL 16, and the reason map is mechanically complete. I verified the generated-column refusal directly, including the part that is easy to get wrong — it fires even when the change would have been a pure relabel:

CREATE TABLE g (a varchar(50), b text GENERATED ALWAYS AS (a || 'x') STORED);
ALTER TABLE g ALTER COLUMN a TYPE varchar(100);
ERROR:  cannot alter type of a column used by a generated column
DETAIL:  Column "a" is used by generated column "b".

Same for int → bigint, while the identical relabel on a column with only an index dependency succeeds — so the refusal really is about the dependency and not the transform. The temporal claim holds too: timestamp(6) → timestamp(3) rewrites, timestamp(3) → timestamp(6) and timestamp(3) → timestamp do not, which is exactly "TemporalSimplify only relabels when precision does not shrink." And I checked the cost map against pkg/planner: all twelve Reason constants are now accounted for. Two findings, one of them a factual error the PR carries forward into the cell it is rewriting.

Findings

1. interval field reduction does not rewrite — it relabels, and it leaves values the new declared type cannot describe. Bucket B's new cell reads "interval field/precision reduction (same, via interval_support)", which is right for precision and wrong for fields. Proved on PostgreSQL 16:

interval DAY TO SECOND  → interval HOUR TO SECOND    rewrote=false
  stored value after:  3 days 05:06:07.891234   (declared type: interval hour to second)
interval (unbounded)    → interval MINUTE TO SECOND  rewrote=false
  stored value after:  3 days 05:06:07.891234
interval DAY TO SECOND(6) → interval DAY TO SECOND(3) rewrote=true
  stored value after:  3 days 05:06:07.891     (rounded, as bucket B says)

Only the precision change rewrites. The field-only change is a bare relabel, and the day component survives in a column now declared hour to second — so this belongs in Common assumptions that turn out to be wrong in the safe direction rather than in bucket B, and it is a sharper example than the ones already listed there, because the declared type stops describing the stored data. Since this PR exists to correct that table, it is the right place to split the two.

2. The online-idiom cost split omits the one form whose cost is conditional. The new sentence enumerates NOT VALID, DROP INDEX CONCURRENTLY, DETACH PARTITION CONCURRENTLY as reading no rows and CREATE INDEX CONCURRENTLY, VALIDATE CONSTRAINT as reading every row — but plan-report.md defines online-idiom as "(CONCURRENTLY, NOT VALID, VALIDATE, USING INDEX)", and USING INDEX appears in neither list. Its cost is not fixed: adopting an existing unique index is catalog-only, while adopting one as a PRIMARY KEY on a nullable column makes PostgreSQL validate NOT NULL by scanning the heap under ACCESS EXCLUSIVE — which safer-sequences.md already spells out as the reason the primary-key substitution needs NOT NULL first. For a sentence whose whole point is a complete cost map, the conditional case is the one most worth naming.

Action items

1. (Finding 1) Narrow bucket B's cell to interval precision reduction, and add the field-only reduction to the no-rewrite surprises with the note that existing values keep components the new declared type excludes.
2. (Finding 2) Place ADD CONSTRAINT … USING INDEX in the online-idiom split — catalog-only for a unique adoption, full scan when adopting as a primary key on a nullable column.

Verified (tried to break, couldn't)

The generated-column refusal is real, fires regardless of transform, and RememberAllDependentForRebuilding is the right attribution (it is the same function the view/rule refusal already cited, which is why folding both into one source-map row is correct). The bucket-C narrowing to numeric is consistent with the source map's typmod-no-op list and with the display-scale explanation the document already carries. The cost map covers every planner reason: metadata-only, fast-default, binary-coercible, safer-idiom, online-idiom, type-rewrite, volatile-default, generated-stored, relocation, partition-parent-lock, app-breaking-rename, unsupported-operation — twelve constants in pkg/planner, twelve placed. Treating partition-parent-lock and app-breaking-rename as cheap-but-warning rather than forcing them into a cost bucket is the honest classification: both are instant and neither is safe to run unthinking. The new caveat that the planner cannot see view/rule or generated-column dependencies because Facts carries only column types is accurate and worth stating plainly — a dry run does report binary-coercible for a change PostgreSQL will refuse. The replaced references point at live sources. No code changes, so nothing to mutation-test; the docs CI jobs that apply are green and the code jobs are correctly skipped by path filter.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Second pass on the same head (74322ac0).

Lens 1 — outside adopter

These are the pages an adopter reads before trusting the tool with a live table, so a wrong statement in them costs more than a wrong statement in a code comment — and the two corrections here are both the kind someone would otherwise discover during an incident. Recovering a commit that landed on an already-merged branch is also the right instinct: the content was reviewed once, and letting it sit unmerged means main documents behaviour PostgreSQL does not have.

The generated-column entry is the standout, because it tells the reader the recovery path is not just "drop the dependency" — re-adding a STORED generated column is itself a full rewrite, so the workaround costs a rewrite even when the type change itself was a relabel. That is the sentence someone plans a maintenance window around.

Lens 2 — SchemaBot integration

Worth naming the operator-visible consequence of the caveat this PR adds, because it lands on our surface: the planner cannot see view/rule or generated-column dependencies, so a change on such a column plans as binary-coercible — "runs as written, no rewrite" — and the apply then fails with PostgreSQL's refusal. In SchemaBot terms the PR comment tells the operator the change is cheap and safe, and the apply dies. The failure is loud rather than silent, so this is not a safety hole, but it is a plan that misled.

The encouraging part is that the fix looks cheap on pg-sprite's side and does not need a new mechanism: preflight.LookupTargetFacts already runs at plan time for a live table — it is how the partitioned-parent refusal works — and the dependency is a single catalog read (pg_depend, or pg_attribute.attgenerated for the generated-column case). That would turn a misleading plan into a plan-time refusal with an actionable remedy, the same shape as the create-path shape refusals. Not this PR's job, but this PR is what makes the gap legible, so it is worth a T2 line rather than living only as a caveat paragraph.

Nothing to change on the SchemaBot side today.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Approving — I verified both substantive claims against a real PostgreSQL 16 (the generated-column refusal fires even on a pure relabel, and the temporal-precision behaviour is exactly as described), and the cost map covers all twelve planner reasons. Two follow-ups: interval field reduction relabels rather than rewrites (proved), and the online-idiom split omits ADD CONSTRAINT … USING INDEX. Details in the two review comments above.

This review was generated by Claude Code (claude-opus-5).