feat: ADR-0035 4f — ALTER TABLE … ALTER COLUMN TYPE
Fourth AlterTableAction (AlterColumnType), runtime-decomposed to the existing change_column_type executor with ForceConversion — which IS the §7 advanced policy: lossy converts with a note (no force flag), incompatible + the ADR-0017 static refusals (↔blob, same-type, date↔datetime, non-int→serial) still refuse, while int→serial is allowed (auto-fills nulls + UNIQUE, ADR-0018 §8). No new mode/note/persistence; undo is the advanced safety net. Grammar adds a fourth action branch leading on `alter`, discriminated in the builder by the `type` keyword (unique — ADD COLUMN's type is an ident); the type slot reuses SQL_TYPE. The internal-__rdbms_* guard was folded into do_change_column_type (user-confirmed), closing the simple `change column` exposure. Tests: 7 Tier-3 e2e via run_replay + 4 Tier-1 parse (incl. a column-named- `type` discriminator probe) + the simple-surface guard. Help/usage refreshed; ADR-0035 §13 4f + README + requirements.md in lockstep.
This commit is contained in:
@@ -1597,6 +1597,11 @@ impl App {
|
||||
Some(table.as_str()),
|
||||
Some(old.as_str()),
|
||||
),
|
||||
AlterTableAction::AlterColumnType { column, .. } => (
|
||||
Operation::ChangeColumnType,
|
||||
Some(table.as_str()),
|
||||
Some(column.as_str()),
|
||||
),
|
||||
},
|
||||
C::SqlCreateTable { name, .. } => {
|
||||
(Operation::CreateTable, Some(name.as_str()), None)
|
||||
|
||||
@@ -4279,6 +4279,11 @@ fn do_change_column_type(
|
||||
ty: Type,
|
||||
mode: ChangeColumnMode,
|
||||
) -> Result<ChangeColumnTypeResult, DbError> {
|
||||
// Refuse the internal `__rdbms_*` tables up-front (as "no such
|
||||
// table"), like the sibling column executors. Closes the simple
|
||||
// `change column` exposure and the SQL `ALTER COLUMN TYPE`
|
||||
// decomposition target (ADR-0035 §4f); user-confirmed 2026-05-25.
|
||||
reject_internal_table_name(table)?;
|
||||
let old_schema = read_schema(conn, table)?;
|
||||
let col_info = old_schema
|
||||
.columns
|
||||
|
||||
+11
-3
@@ -714,9 +714,11 @@ pub enum IndexSelector {
|
||||
Columns { table: String, columns: Vec<String> },
|
||||
}
|
||||
|
||||
/// The action of an advanced-mode `ALTER TABLE` (ADR-0035 §4). Sub-phase
|
||||
/// 4e carries the column actions; 4f/4g/4h add `AlterColumnType`,
|
||||
/// `AddConstraint`/`AddForeignKey`/`DropConstraint`, and `RenameTo`.
|
||||
/// The action of an advanced-mode `ALTER TABLE` (ADR-0035 §4).
|
||||
///
|
||||
/// Sub-phase 4e carries the column actions; 4f adds `AlterColumnType`;
|
||||
/// 4g/4h add `AddConstraint`/`AddForeignKey`/`DropConstraint`, and
|
||||
/// `RenameTo`.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum AlterTableAction {
|
||||
/// `ADD COLUMN <name> <type> [NOT NULL] [UNIQUE] [DEFAULT …]
|
||||
@@ -731,6 +733,12 @@ pub enum AlterTableAction {
|
||||
DropColumn { column: String },
|
||||
/// `RENAME COLUMN <old> TO <new>` — reuses `do_rename_column`.
|
||||
RenameColumn { old: String, new: String },
|
||||
/// `ALTER COLUMN <name> TYPE <type>` — reuses `do_change_column_type`
|
||||
/// with `ChangeColumnMode::ForceConversion`, which is the ADR-0035 §7
|
||||
/// advanced-mode policy (lossy cells are *performed* with a note, no
|
||||
/// force flag; static-refused / incompatible still refuse). One undo
|
||||
/// step (the executor's rebuild). ADR-0035 §4f.
|
||||
AlterColumnType { column: String, ty: Type },
|
||||
}
|
||||
|
||||
impl std::fmt::Display for IndexSelector {
|
||||
|
||||
+152
-7
@@ -1905,9 +1905,24 @@ static AT_RENAME_COLUMN_NODES: &[Node] = &[
|
||||
];
|
||||
const AT_RENAME_COLUMN: Node = Node::Seq(AT_RENAME_COLUMN_NODES);
|
||||
|
||||
// Each action branch leads on a concrete keyword (`add`/`drop`/
|
||||
// `rename`) — trap-safe.
|
||||
static AT_ACTION_CHOICES: &[Node] = &[AT_ADD_COLUMN, AT_DROP_COLUMN, AT_RENAME_COLUMN];
|
||||
// `ALTER COLUMN <col> TYPE <type>` (ADR-0035 §4f). The type slot reuses
|
||||
// SQL_TYPE (the same alias map + `double precision` pair the CREATE
|
||||
// TABLE / ADD COLUMN forms use). The builder keys on the `type` keyword
|
||||
// — unique to this action (ADD COLUMN's type is a `col_type` ident).
|
||||
static AT_ALTER_COLUMN_NODES: &[Node] = &[
|
||||
Node::Word(Word::keyword("alter")),
|
||||
Node::Word(Word::keyword("column")),
|
||||
COLUMN_NAME,
|
||||
Node::Word(Word::keyword("type")),
|
||||
super::sql_create_table::SQL_TYPE,
|
||||
];
|
||||
const AT_ALTER_COLUMN: Node = Node::Seq(AT_ALTER_COLUMN_NODES);
|
||||
|
||||
// Each action branch leads on a concrete keyword (`add`/`drop`/`rename`/
|
||||
// `alter`) — trap-safe. (The branch's `alter` is the action word; the
|
||||
// entry-word `alter` was already consumed by dispatch.)
|
||||
static AT_ACTION_CHOICES: &[Node] =
|
||||
&[AT_ADD_COLUMN, AT_DROP_COLUMN, AT_RENAME_COLUMN, AT_ALTER_COLUMN];
|
||||
const AT_ACTION: Node = Node::Choice(AT_ACTION_CHOICES);
|
||||
|
||||
static SQL_ALTER_TABLE_SHAPE_NODES: &[Node] = &[
|
||||
@@ -1989,12 +2004,54 @@ fn build_alter_add_column_spec(
|
||||
})
|
||||
}
|
||||
|
||||
/// Build `Command::SqlAlterTable` (ADR-0035 §4e). The action is the
|
||||
/// leading concrete keyword (`add`/`drop`/`rename` — exactly one matches
|
||||
/// per the action `Choice`).
|
||||
/// Extract the `ALTER COLUMN <col> TYPE <type>` action (ADR-0035 §4f).
|
||||
/// The type slot reuses SQL_TYPE, so the target-type extraction mirrors
|
||||
/// `build_alter_add_column_spec`'s: a `col_type` ident via
|
||||
/// `Type::from_sql_name` (alias map applied), or the two-word
|
||||
/// `double precision` → `Type::Real`.
|
||||
fn build_alter_column_type(path: &MatchedPath) -> Result<AlterTableAction, ValidationError> {
|
||||
let column = require_ident(path, "column_name")?;
|
||||
let mut ty: Option<Type> = None;
|
||||
let mut items = path.items.iter().peekable();
|
||||
while let Some(item) = items.next() {
|
||||
match &item.kind {
|
||||
MatchedKind::Ident { role: "col_type", .. } => {
|
||||
ty = Some(Type::from_sql_name(&item.text).ok_or_else(|| ValidationError {
|
||||
message_key: "parse.error_wrapper",
|
||||
args: vec![("detail", "unknown type".to_string())],
|
||||
})?);
|
||||
}
|
||||
MatchedKind::Word("double") => {
|
||||
if matches!(
|
||||
items.peek().map(|i| &i.kind),
|
||||
Some(MatchedKind::Word("precision"))
|
||||
) {
|
||||
items.next();
|
||||
}
|
||||
ty = Some(Type::Real);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
let ty = ty.ok_or_else(|| ValidationError {
|
||||
message_key: "parse.error_wrapper",
|
||||
args: vec![("detail", "alter column needs a target type".to_string())],
|
||||
})?;
|
||||
Ok(AlterTableAction::AlterColumnType { column, ty })
|
||||
}
|
||||
|
||||
/// Build `Command::SqlAlterTable` (ADR-0035 §4e/§4f). The action is the
|
||||
/// leading concrete keyword (`add`/`drop`/`rename`/`alter` — exactly one
|
||||
/// matches per the action `Choice`). The `type` keyword is checked
|
||||
/// **first**: it is unique to ALTER COLUMN TYPE (ADD COLUMN's type is a
|
||||
/// `col_type` *ident*, not the literal word), and an `alter column …`
|
||||
/// input contains none of add/drop/rename, so without this it would fall
|
||||
/// through to the DropColumn arm.
|
||||
fn build_sql_alter_table(path: &MatchedPath, source: &str) -> Result<Command, ValidationError> {
|
||||
let table = require_ident(path, "table_name")?;
|
||||
let action = if path.contains_word("add") {
|
||||
let action = if path.contains_word("type") {
|
||||
build_alter_column_type(path)?
|
||||
} else if path.contains_word("add") {
|
||||
AlterTableAction::AddColumn(Box::new(build_alter_add_column_spec(path, source)?))
|
||||
} else if path.contains_word("rename") {
|
||||
AlterTableAction::RenameColumn {
|
||||
@@ -2551,6 +2608,94 @@ mod sql_alter_table_tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alter_column_type_parses() {
|
||||
// ADR-0035 §4f: the fourth action, discriminated by the `type`
|
||||
// keyword (ADD COLUMN's type is an ident, not the literal word).
|
||||
let (table, action) = alter("alter table T alter column qty type int");
|
||||
assert_eq!(table, "T");
|
||||
match action {
|
||||
AlterTableAction::AlterColumnType { column, ty } => {
|
||||
assert_eq!(column, "qty");
|
||||
assert_eq!(ty, crate::dsl::types::Type::Int);
|
||||
}
|
||||
other => panic!("expected AlterColumnType, got {other:?}"),
|
||||
}
|
||||
// trailing semicolon tolerated
|
||||
assert!(matches!(
|
||||
alter("alter table T alter column qty type int;").1,
|
||||
AlterTableAction::AlterColumnType { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alter_column_type_accepts_sql_type_alias() {
|
||||
// `integer` → int, `double precision` → real (ADR-0035 §3),
|
||||
// reusing SQL_TYPE for the type slot.
|
||||
match alter("alter table T alter column n type integer").1 {
|
||||
AlterTableAction::AlterColumnType { ty, .. } => {
|
||||
assert_eq!(ty, crate::dsl::types::Type::Int);
|
||||
}
|
||||
other => panic!("expected AlterColumnType, got {other:?}"),
|
||||
}
|
||||
match alter("alter table T alter column n type double precision").1 {
|
||||
AlterTableAction::AlterColumnType { ty, .. } => {
|
||||
assert_eq!(ty, crate::dsl::types::Type::Real);
|
||||
}
|
||||
other => panic!("expected AlterColumnType, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn four_branch_dispatch_still_routes_the_column_actions() {
|
||||
// The new `alter column type` branch does not steal add/drop/
|
||||
// rename: each still routes to its own action.
|
||||
assert!(matches!(
|
||||
alter("alter table T add column note text").1,
|
||||
AlterTableAction::AddColumn(_)
|
||||
));
|
||||
assert!(matches!(
|
||||
alter("alter table T drop column note").1,
|
||||
AlterTableAction::DropColumn { .. }
|
||||
));
|
||||
assert!(matches!(
|
||||
alter("alter table T rename column a to b").1,
|
||||
AlterTableAction::RenameColumn { .. }
|
||||
));
|
||||
assert!(matches!(
|
||||
alter("alter table T alter column a type text").1,
|
||||
AlterTableAction::AlterColumnType { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn type_discriminator_probe_column_named_type() {
|
||||
// PROBE (DA): the `type`-keyword discriminator keys on the literal
|
||||
// `type` *keyword* node, which only the ALTER COLUMN TYPE branch
|
||||
// carries. Verify a column whose *name* is `type` does not get
|
||||
// misrouted (it is an ident, not a Word). If `type` is reserved
|
||||
// and rejected as an ident, the parse errors — either outcome is
|
||||
// fine; the failure we guard against is silent misrouting to
|
||||
// AlterColumnType (which would then error on a missing type).
|
||||
let dropped = parse_command_in_mode("alter table T drop column type", Mode::Advanced);
|
||||
match dropped {
|
||||
Ok(Command::SqlAlterTable {
|
||||
action: AlterTableAction::DropColumn { column },
|
||||
..
|
||||
}) => assert_eq!(column, "type", "a column named `type` drops correctly"),
|
||||
Ok(other) => panic!("`drop column type` misrouted to {other:?}"),
|
||||
Err(_) => { /* `type` rejected as an ident — acceptable, no misroute */ }
|
||||
}
|
||||
// And the real ALTER COLUMN TYPE still routes (sanity).
|
||||
assert!(matches!(
|
||||
parse_command_in_mode("alter table T alter column c type int", Mode::Advanced),
|
||||
Ok(Command::SqlAlterTable {
|
||||
action: AlterTableAction::AlterColumnType { .. },
|
||||
..
|
||||
})
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alter_is_advanced_only() {
|
||||
// No simple `alter`; in simple mode it does not parse as a
|
||||
|
||||
@@ -273,7 +273,8 @@ help:
|
||||
sql_alter_table: |-
|
||||
alter table <T> add column <col> <type> [not null] [unique] [default …] [check …]
|
||||
alter table <T> drop column <col>
|
||||
alter table <T> rename column <old> to <new> — change a table's columns (advanced SQL)
|
||||
alter table <T> rename column <old> to <new>
|
||||
alter table <T> alter column <col> type <type> — change a table's columns (advanced SQL)
|
||||
drop: |-
|
||||
drop table <T> — remove a table
|
||||
drop column [from] [table] <T>: <col> [--cascade] — remove a column
|
||||
@@ -473,6 +474,7 @@ parse:
|
||||
alter table <Table> add column <Name> <Type> [not null] [unique] [default <expr>] [check (<expr>)]
|
||||
alter table <Table> drop column <Name>
|
||||
alter table <Table> rename column <Old> to <New>
|
||||
alter table <Table> alter column <Name> type <Type>
|
||||
drop_table: "drop table <Name>"
|
||||
drop_column: "drop column [from] [table] <Table>: <Name>"
|
||||
drop_relationship: |-
|
||||
@@ -871,8 +873,10 @@ ok:
|
||||
client_side:
|
||||
# Per-cell transformation notice when `change column ...` rewrote
|
||||
# at least one stored value (storage-class change or non-identity
|
||||
# mapping). `lossy` variant fires under --force-conversion when
|
||||
# information was discarded.
|
||||
# mapping). `lossy` variant fires when information was discarded —
|
||||
# under simple-mode `--force-conversion`, and under advanced-mode
|
||||
# `alter table … alter column … type …`, which always converts
|
||||
# (ADR-0035 §7).
|
||||
transformed: |-
|
||||
[client-side] {count} row(s) were transformed before being stored. In raw SQL this would need an explicit `CAST` or application-level code.
|
||||
transformed_lossy: |-
|
||||
|
||||
+11
-1
@@ -33,7 +33,7 @@ use crate::db::{
|
||||
Database, DbError, DeleteResult, DropColumnResult, DropIndexOutcome, DropOutcome, InsertResult,
|
||||
QueryPlan, TableDescription, UpdateResult,
|
||||
};
|
||||
use crate::dsl::{AlterTableAction, Command, ColumnSpec};
|
||||
use crate::dsl::{AlterTableAction, ChangeColumnMode, Command, ColumnSpec};
|
||||
use crate::dsl::walker::Severity;
|
||||
use crate::event::AppEvent;
|
||||
use crate::project::{
|
||||
@@ -2114,6 +2114,16 @@ async fn execute_command_typed(
|
||||
.rename_column(table, old, new, src)
|
||||
.await
|
||||
.map(|d| CommandOutcome::Schema(Some(d))),
|
||||
// `ALTER COLUMN … TYPE` reuses the simple `change column`
|
||||
// executor with `ForceConversion` — the ADR-0035 §7
|
||||
// advanced policy (lossy converts with a note; no force
|
||||
// flag; static-refused / incompatible still refuse). The
|
||||
// ChangeColumn outcome surfaces the client-side lossy note,
|
||||
// shared with simple mode.
|
||||
AlterTableAction::AlterColumnType { column, ty } => database
|
||||
.change_column_type(table, column, ty, ChangeColumnMode::ForceConversion, src)
|
||||
.await
|
||||
.map(CommandOutcome::ChangeColumn),
|
||||
},
|
||||
Command::AddConstraint {
|
||||
table,
|
||||
|
||||
Reference in New Issue
Block a user