- docs/plans/20260525-adr-0035-sql-ddl-4f.md — ALTER TABLE ALTER COLUMN TYPE plan (/runda'd; forks resolved). Advanced lossy = ForceConversion (reuse do_change_column_type; existing transformed_lossy note); the internal-__rdbms_* guard folds into do_change_column_type both surfaces (user-confirmed); int->serial is ALLOWED (ADR-0018 §8), only non-int ->serial / blob / date<->datetime are static-refused. - docs/handoff/20260525-handoff-39.md — 4d/4e shipped; 4f is next (plan ready).
14 KiB
Plan: ADR-0035 Phase 4, sub-phase 4f — ALTER TABLE … ALTER COLUMN TYPE
Add ALTER TABLE <T> ALTER COLUMN <col> TYPE <type> as a fourth
AlterTableAction (AlterColumnType), runtime-decomposed to the
existing do_change_column_type executor with the advanced-mode
policy (ADR-0035 §7): lossy conversions are performed with a note,
no force flag; static-refused (→serial, ↔blob) and incompatible
conversions still refuse (ADR-0017, shared by both modes). This is the
smallest 4e-style slice — almost entirely reuse.
The single new executor change is folding the internal-__rdbms_* guard
into do_change_column_type (the 4e follow-up), so the simple change column surface is closed too.
1. Baseline (at handoff)
- Tests: 1854 passing, 0 failing, 0 skipped, 1 ignored; clippy clean.
Branch
main, HEADbbc2e34(4e). 4f starts here. (4a–4e are local-only commits sinceorigin/main.)
2. Decisions (settled — ADR §7/§13 4f + user-confirmed 2026-05-25)
- Advanced lossy =
ChangeColumnMode::ForceConversion— no new mode, no new note.ForceConversionalready is the §7 advanced policy:static_refusalrefuses→serial/↔blob; the dry-run refuses incompatible cells; lossy cells are transformed; theClientSideNote { transformed, lossy }is surfaced. The existingclient_side.transformed_lossycatalog note ("N row(s) transformed; M of those discarded information (lossy)…") is engine-neutral and reads correctly for advanced — it is the §7 "N values converted with loss" note. (Update the stale// fires under --force-conversioncomment inen-US.yamlto mention advanced ALTER COLUMN TYPE too.) - No SQL spelling for
--force-conversion/--dont-convert(ADR §7/§12). Advanced always performs the conversion (relying onundoas the safety net — a payoff of shipping ADR-0006 first). The PostgresUSING <expr>clause is not adopted (§12). - Static-refused + incompatible reuse the executor (ADR-0017,
shared by both modes). Verified
static_refusal(type_change.rs) refuses: same-type (already X); non-int →serial(onlyint → serialis allowed); any↔ blob; directdate ↔ datetime(route via text); and any pair not in the matrix. An incompatible per-cell conversion → the existing diagnostic. NOTE — ADR-0035 §7's "static-refused (→serial …)" summary is looser than the code:int → serialIS allowed (ADR-0018 §8 — auto-fills null cells with generated values, adds UNIQUE if non-PK), so advancedALTER COLUMN c TYPE serialon anintcolumn is a supported operation, not a refusal. The implementer must testint → serial(allowed) andtext → serial/↔ blob(refused) — do NOT "fix" int→serial to refuse. TYPEkeyword only —ALTER COLUMN <col> TYPE <type>(ADR §4). The PostgresSET DATA TYPEsynonym is not added (kept minimal; could be a later alias).- Internal-
__rdbms_*guard folded intodo_change_column_type(user-confirmed 2026-05-25) — closes the simplechange columnexposure alongside the parse-time guard the SQL ALTER table slot already gives.do_add_constraint/do_add_relationshipstay flagged for 4g.
3. Phase 1 — Requirements checklist (4f)
Grammar / dispatch:
ALTER TABLE <T> ALTER COLUMN <col> TYPE <type>parses in advanced mode →SqlAlterTable { AlterColumnType { column, ty } };<type>accepts the SQL type vocabulary + aliases (reuseSQL_TYPE).- The action
Choicenow has four branches (add/drop/rename/ alter-column), each leading on a distinct concrete keyword — trap-safe. The other three still parse;alterremains advanced-only; the table slot still rejects__rdbms_*at parse. - Trailing
;tolerated.
Execution (reuse do_change_column_type with ForceConversion):
- Clean conversion (e.g.
int → text) auto-converts; one undo step; auto-show. - Lossy conversion (e.g.
real → int,3.7 → 3) is performed and surfaces the[client-side] … discarded information (lossy)note (no force flag, no refusal) — the §7 advanced behaviour. - Incompatible conversion is refused (friendly, engine-neutral)
— e.g.
text → intwhere a cell holds a non-numeric string. - Static-refused refused:
text → serial(non-int source),↔ blob,date ↔ datetimedirect, same-type. Andint → serialis allowed (auto-fills nulls, adds UNIQUE — ADR-0018 §8): test it as a supported conversion, not a refusal. - FK-involved column type change refuses per the existing executor rules (outbound/inbound), via the SQL path too.
- The converted column round-trips and survives rebuild (the
user_typemetadata update is the existing path).
Internal-table guard (both surfaces):
do_change_column_typerefuses an internal__rdbms_*table (as "no such table"); the simplechange columnsurface is closed. A test on the simple surface.
Testing
- Tier 1 (extend
sql_alter_table_testsinddl.rs): the alter-column-type parse (incl. a type alias) →AlterColumnType; the four-branch dispatch still routes add/drop/rename correctly. - Tier 3 (extend
tests/sql_alter_table.rs, viarun_replay): clean convert (int → text); lossy convert (real → int) performs — assert the value was converted (e.g.3.7stored as3); incompatible refused;text → serial(and/or↔ blob) refused;int → serialallowed (auto-fill); rebuild survival; one undo step. (Assert the lossy data conversion as the primary signal; the[client-side] … lossynote is rendered through the shared ChangeColumn path.) - Internal guard (extend
tests/column_op_guards.rs): simplechange columnon__rdbms_*refused. - Catalog lockstep + vocab audit for the refreshed
sql_alter_tablehelp/usage (it now lists the alter-column-type form).
4. Architecture & design
4.1 Command (src/dsl/command.rs)
AlterTableActiongainsAlterColumnType { column: String, ty: Type }(small variant — no boxing needed; the boxedAddColumnalready dominates the enum's size).
4.2 Grammar (src/dsl/grammar/ddl.rs)
- New action branch
AT_ALTER_COLUMN = Seq[ Word("alter"), Word("column"), COLUMN_NAME, Word("type"), super::sql_create_table::SQL_TYPE ]. Add it toAT_ACTION_CHOICES(now[AT_ADD_COLUMN, AT_DROP_COLUMN, AT_RENAME_COLUMN, AT_ALTER_COLUMN]). Leads on the concretealterkeyword (distinct from add/drop/rename) — trap-safe. The action'salteris the 4th token (the entry-wordalteris consumed by dispatch first). build_sql_alter_table: add a branch. Discriminate on thetypekeyword (path.contains_word("type")) — unique to ALTER COLUMN TYPE (ADD COLUMN's type is acol_typeident, not the literaltypekeyword). Extractcolumn = require_ident("column_name")and the targetTypefrom thecol_typeident (Type::from_sql_name) / thedouble precisiontwo-word case — mirror the type-extraction inbuild_alter_add_column_spec. Order thetypecheck before add/rename/drop.
4.3 Worker / executor (src/db.rs)
- No new method. Add the internal-table guard:
reject_internal_table_name(table)?;at the top ofdo_change_column_type(mirrors the 4e column executors). ForceConversion's lossy path +ClientSideNoteare unchanged.
4.4 Runtime (src/runtime.rs)
- Extend the
SqlAlterTableaction match:AlterColumnType { column, ty } => database.change_column_type(table, column, ty, ChangeColumnMode::ForceConversion, src).await .map(CommandOutcome::ChangeColumn)— the same outcome the simplechange columnuses (its event/app rendering surfaces the client-side lossy note).
4.5 app.rs failure-translate + typing_surface
app.rs build_translate_context: theSqlAlterTablearm's innermatch actiongainsAlterColumnType { column, .. } => (Operation::ChangeColumnType, Some(table), Some(column)).typing_surfacealready labelsSqlAlterTable; no change (the action is internal to the variant).
4.6 Catalog (keys.rs + en-US.yaml)
- Refresh the existing
help.ddl.sql_alter_table+parse.usage.sql_alter_tablebodies to add thealter table <T> alter column <col> type <type>line. No new keys. - Update the stale
client_side// fires under --force-conversioncomment to note advanced ALTER COLUMN TYPE also triggers the lossy note (comment only).
5. Phase 2 — Candidate approaches
(M1) Reuse do_change_column_type with ForceConversion (lead) —
the mode already encodes the §7 advanced policy exactly; the lossy note
already exists and is engine-neutral. Zero executor logic beyond the
internal-table guard.
(M2) Add a new ChangeColumnMode::Advanced variant — rejected.
It would duplicate ForceConversion's behaviour; the only conceivable
difference (note wording) is already neutral and correct. No new variant
earns its keep. (If a future need to distinguish "user forced" from
"advanced auto" arises — e.g. for telemetry — revisit then.)
(G1) A fourth action branch leading on alter, discriminated in the
builder by the type keyword (lead) — consistent with the three
4e branches; type is an unambiguous discriminator.
(G2) A separate SQL_ALTER_COLUMN command/node — rejected. The
AlterTableAction enum is the established extension point (built for
this in 4e); a separate command would fork the alter table dispatch.
6. Phase 3 — Selection vs the checklist
M1 + G1 satisfy every §3 item: parse (G1 four-branch), the §7 tiers (clean/lossy/incompatible/static-refused — all from the reused executor
ForceConversion), the lossy note (existingtransformed_lossy), rebuild survival + undo parity (the executor's existing rebuild path), the internal-table guard (one-line add, both surfaces). No requirement unmet, no new model/persistence. Selected: M1 + G1.
7. Devil's Advocate review of this plan
- Fork escalated? The only 4f fork — the internal-table guard on
do_change_column_type— was put to the user (2026-05-25, "fix in 4f, both surfaces"). The §7 lossy policy and the no-force-flag/USINGexclusions are ADR-settled, not autonomous. ✓ - Is
ForceConversiontruly the §7 advanced policy? — verified in code.run_change_column_with_dry_run: incompatible cells refuse in both modes; lossy cells refuse only whenmode != ForceConversion(db.rs:4575) — so underForceConversionthey are performed and counted (ClientSideNote.lossy), firingclient_side.transformed_lossy(engine-neutral, no--force-conversionin the rendered text).static_refusalwas read in full (the §2.3 list) — correcting the plan's first draft:int → serialis allowed, not refused. Matches the §7 intent once that nuance is captured. ✓ - Grammar trap / discriminator? Four concrete-keyword-led branches;
the builder keys on the
typekeyword (unique — ADD COLUMN's type is an ident). A parse test for all four branches + the alias case is on the checklist. The implementer must probe thetype-keyword discriminator and the SQL_TYPE extraction (mirrorbuild_alter_add_column_spec) — low-risk, 4e-proven patterns. ✓ - Lossy note actually fires on the SQL path? The e2e (
run_replay) lossy test asserts it; the note path isCommandOutcome::ChangeColumn→ the existing app rendering, shared with simplechange column. ✓ - Undo parity? One
change_column_typecall = one snapshot (the executor's existing rebuild is one undo step). A per-action undo check in the e2e. ✓ - Anything dropped?
SET DATA TYPEsynonym and the--force/--dontSQL spellings are deliberately out (stated, ADR-backed). The broader internal-table guard (do_add_constraint/do_add_relationship) stays a 4g follow-up — flagged, not silent. ✓ - Help/usage refresh? The
sql_alter_tableskeleton is refreshed (existing node, existing keys) to list the new form — not deferred to 4i (that deferral is only for the CREATE TABLE skeleton). ✓
8. Out of 4f scope (tracked, not dropped)
ALTER TABLEadd/drop constraint + add FK (4g) — and the internal guard ondo_add_constraint/do_add_relationshiprides there.ALTER TABLE … RENAME TOtable rename (4h).SET DATA TYPEsynonym; the PostgresUSING <expr>clause; any SQL spelling of--force-conversion/--dont-convert(ADR §7/§12).- 4i verification sweep (the running deferral tail).
9. Implementation sequence (test-first)
- Internal-table guard (isolated, both surfaces).
reject_internal_table_nameat the top ofdo_change_column_type; a Tier-3 test via the simplechange column(column_op_guards.rs) → green. Lands the latent-bug fix on its own. - Command + grammar + builder.
AlterColumnTypevariant; theAT_ALTER_COLUMNbranch + the builder discriminator; Tier-1 parse + four-branch dispatch tests (probe thetypediscriminator first) → the exhaustive arms (app.rs) → green (parse only). - Runtime + catalog. Wire
AlterColumnType→change_column_type(…, ForceConversion); refresh thesql_alter_tablehelp/usage; Tier-3 (sql_alter_table.rsviarun_replay): clean / lossy+note / incompatible / static-refused / rebuild / undo → green. - Full sweep —
cargo test(no regression from 1854) +cargo clippy --all-targets -- -D warnings. - Docs — ADR-0035 Status + §13 4f implemented; README; requirements
Q1. Run
/rundaover the finished slice. Propose commit; wait for approval.
10. Exit gate
- All §3 items satisfied; four tiers green, zero skips; no regression
from 1854;
/runda/ written-DA PASS; clippy clean; ADR-0035 §13 4f + README + requirements.md lockstep.