feat: ADR-0035 4g — ALTER TABLE add/drop constraint + add FK

ALTER TABLE <T> ADD [CONSTRAINT <name>] (CHECK | UNIQUE | FOREIGN KEY)
and DROP CONSTRAINT <name>. ADD = table-CHECK + composite UNIQUE + FK
(ADD PRIMARY KEY and a named UNIQUE refused — composite UNIQUE is
anonymous in our model). Each ADD reuses a low-level path with a dry-run
guard (table-CHECK/UNIQUE rebuild; FK -> add_relationship, bare
REFERENCES -> parent single PK). DROP CONSTRAINT resolves the name to a
named table-CHECK then a child-side FK, else refuses. One undo step each.

Named table-CHECKs round-trip: a nullable `name` column on
__rdbms_playground_table_checks (rebuild-only arrival; a named add on a
pre-4g project is refused with a "rebuild first" hint) plus a project.yaml
check_constraints {expr, name} extension (bare-string form still reads).
The internal-__rdbms_* guard was folded into do_add_constraint /
do_add_relationship, completing that guard class.

Grammar: the action Choice keeps one branch per verb (add/drop/rename/
alter) with an inner Choice fanning out on the distinct second keyword,
since the walker's Choice does not backtrack between same-led branches.

Tests: 7 Tier-1 parse + 2 yaml round-trip + 1 internal-guard + 9 Tier-3
e2e. Help/usage refreshed; ADR-0035 §13 4g + README + requirements.md in
lockstep.
This commit is contained in:
claude@clouddev1
2026-05-25 22:07:50 +00:00
parent 5b76315d1e
commit 6ff97f6e20
16 changed files with 1747 additions and 84 deletions
+338 -31
View File
@@ -14,7 +14,7 @@
use crate::dsl::action::ReferentialAction;
use crate::dsl::command::{
AlterTableAction, ChangeColumnMode, ColumnSpec, Command, Constraint, ConstraintKind, Expr,
IndexSelector, RelationshipSelector, SqlForeignKey,
IndexSelector, RelationshipSelector, SqlForeignKey, TableConstraint,
};
use crate::dsl::value::Value;
use crate::dsl::grammar::{
@@ -1880,21 +1880,29 @@ const AT_ADD_CONSTRAINT_SUFFIX: Node = Node::Repeated {
min: 0,
};
static AT_ADD_COLUMN_NODES: &[Node] = &[
Node::Word(Word::keyword("add")),
// The walker's `Choice` selects a branch by its **leading** token and
// does not backtrack into a sibling once a branch's first keyword
// matched. So the action `Choice` keeps ONE branch per leading verb
// (`add`/`drop`/`rename`/`alter`); the `add` and `drop` verbs then
// fan out to an **inner** `Choice` whose branches each lead on a
// *distinct* second keyword (column / constraint / check / unique /
// foreign / primary), so no two same-led branches ever sit in one
// `Choice`.
// `add column <col> <type> [constraints]` — the column-def tail (the
// leading `add` is consumed by `AT_ADD`).
static AT_ADD_COLUMN_TAIL_NODES: &[Node] = &[
Node::Word(Word::keyword("column")),
super::sql_create_table::COL_NAME,
super::sql_create_table::SQL_TYPE,
AT_ADD_CONSTRAINT_SUFFIX,
];
const AT_ADD_COLUMN: Node = Node::Seq(AT_ADD_COLUMN_NODES);
const AT_ADD_COLUMN_TAIL: Node = Node::Seq(AT_ADD_COLUMN_TAIL_NODES);
static AT_DROP_COLUMN_NODES: &[Node] = &[
Node::Word(Word::keyword("drop")),
Node::Word(Word::keyword("column")),
COLUMN_NAME,
];
const AT_DROP_COLUMN: Node = Node::Seq(AT_DROP_COLUMN_NODES);
// `drop column <col>` / `drop constraint <name>` tails (leading `drop`
// consumed by `AT_DROP`).
static AT_DROP_COLUMN_TAIL_NODES: &[Node] = &[Node::Word(Word::keyword("column")), COLUMN_NAME];
const AT_DROP_COLUMN_TAIL: Node = Node::Seq(AT_DROP_COLUMN_TAIL_NODES);
static AT_RENAME_COLUMN_NODES: &[Node] = &[
Node::Word(Word::keyword("rename")),
@@ -1918,11 +1926,76 @@ static AT_ALTER_COLUMN_NODES: &[Node] = &[
];
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];
// --- 4g: ADD [CONSTRAINT <name>] table-constraint / DROP CONSTRAINT ---
//
// `ADD [CONSTRAINT <name>] (check (…) | unique (…) | foreign key (…)
// references … | primary key (…))` and `DROP CONSTRAINT <name>`
// (ADR-0035 §4g). The constraint bodies reuse the `sql_create_table`
// table-element nodes; the §4g name comes from the `CONSTRAINT <name>`
// prefix (a dedicated `constraint`-led inner branch — never a leading
// `Optional`). UNIQUE/PRIMARY KEY may carry a name syntactically but the
// builder refuses it (composite UNIQUE is anonymous; PK is unsupported).
const CONSTRAINT_NAME: Node = Node::Ident {
source: IdentSource::NewName,
role: "constraint_name",
validator: None,
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
writes_table_alias: false,
writes_cte_name: false,
writes_projection_alias: false,
};
// The constraint bodies — each leads on a distinct concrete keyword.
static AT_CONSTRAINT_BODY_CHOICES: &[Node] = &[
super::sql_create_table::TABLE_CHECK,
super::sql_create_table::TABLE_UNIQUE,
super::sql_create_table::TABLE_FK,
// `primary key (…)` parses so the builder can refuse it with a
// specific message rather than a generic "unexpected `primary`".
super::sql_create_table::TABLE_PK,
];
const AT_CONSTRAINT_BODY: Node = Node::Choice(AT_CONSTRAINT_BODY_CHOICES);
// `constraint <name> <body>` — the named-constraint tail (leads on the
// concrete `constraint` keyword, so it is a safe `Choice` sibling).
static AT_CONSTRAINT_NAMED_NODES: &[Node] = &[
Node::Word(Word::keyword("constraint")),
CONSTRAINT_NAME,
AT_CONSTRAINT_BODY,
];
const AT_CONSTRAINT_NAMED: Node = Node::Seq(AT_CONSTRAINT_NAMED_NODES);
// The `add` tail: a column def, a named constraint, or one of the bare
// (unnamed) constraint bodies — each branch leads on a distinct keyword
// (column / constraint / check / unique / foreign / primary).
static AT_ADD_TAIL_CHOICES: &[Node] = &[
AT_ADD_COLUMN_TAIL,
AT_CONSTRAINT_NAMED,
super::sql_create_table::TABLE_CHECK,
super::sql_create_table::TABLE_UNIQUE,
super::sql_create_table::TABLE_FK,
super::sql_create_table::TABLE_PK,
];
const AT_ADD_TAIL: Node = Node::Choice(AT_ADD_TAIL_CHOICES);
static AT_ADD_NODES: &[Node] = &[Node::Word(Word::keyword("add")), AT_ADD_TAIL];
const AT_ADD: Node = Node::Seq(AT_ADD_NODES);
// The `drop` tail: a column or a named constraint (distinct second
// keywords `column` / `constraint`).
static AT_DROP_CONSTRAINT_TAIL_NODES: &[Node] =
&[Node::Word(Word::keyword("constraint")), CONSTRAINT_NAME];
const AT_DROP_CONSTRAINT_TAIL: Node = Node::Seq(AT_DROP_CONSTRAINT_TAIL_NODES);
static AT_DROP_TAIL_CHOICES: &[Node] = &[AT_DROP_COLUMN_TAIL, AT_DROP_CONSTRAINT_TAIL];
const AT_DROP_TAIL: Node = Node::Choice(AT_DROP_TAIL_CHOICES);
static AT_DROP_NODES: &[Node] = &[Node::Word(Word::keyword("drop")), AT_DROP_TAIL];
const AT_DROP: Node = Node::Seq(AT_DROP_NODES);
// One branch per leading verb (`add`/`drop`/`rename`/`alter`) — distinct
// concrete keywords, trap-safe. (The branch's `alter` is the action
// word; the entry-word `alter` was already consumed by dispatch.) The
// second-keyword fan-out happens in `AT_ADD` / `AT_DROP`'s inner Choice.
static AT_ACTION_CHOICES: &[Node] = &[AT_ADD, AT_DROP, AT_RENAME_COLUMN, AT_ALTER_COLUMN];
const AT_ACTION: Node = Node::Choice(AT_ACTION_CHOICES);
static SQL_ALTER_TABLE_SHAPE_NODES: &[Node] = &[
@@ -2040,32 +2113,142 @@ fn build_alter_column_type(path: &MatchedPath) -> Result<AlterTableAction, Valid
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.
/// Build `Command::SqlAlterTable` (ADR-0035 §4e/§4f/§4g). Exactly one
/// action `Choice` branch matched; the builder recovers which from the
/// matched words. Discrimination order matters:
///
/// 1. **`type`** first — unique to ALTER COLUMN TYPE (ADD COLUMN's type
/// is a `col_type` *ident*, not the literal word), and an `alter
/// column …` input also contains `column`, so it must be caught
/// before the column branch.
/// 2. **`column`** — the column ops (add/drop/rename column), routed by
/// `add`/`rename`/else-drop. Checked before the bare `add`/`drop`
/// keywords so `add column … unique`/`… check` (a column constraint)
/// still routes to AddColumn.
/// 3. **`add`** — a table-level constraint (CHECK / UNIQUE / FK / the
/// refused PRIMARY KEY).
/// 4. else **`drop`** — `drop constraint <name>`.
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("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 {
old: require_ident(path, "column_name")?,
new: require_ident(path, "new_column_name")?,
} else if path.contains_word("column") {
if path.contains_word("add") {
AlterTableAction::AddColumn(Box::new(build_alter_add_column_spec(path, source)?))
} else if path.contains_word("rename") {
AlterTableAction::RenameColumn {
old: require_ident(path, "column_name")?,
new: require_ident(path, "new_column_name")?,
}
} else {
AlterTableAction::DropColumn {
column: require_ident(path, "column_name")?,
}
}
} else if path.contains_word("add") {
build_alter_add_table_constraint(path, source)?
} else {
AlterTableAction::DropColumn {
column: require_ident(path, "column_name")?,
AlterTableAction::DropConstraint {
name: require_ident(path, "constraint_name")?,
}
};
Ok(Command::SqlAlterTable { table, action })
}
/// Build the `ADD [CONSTRAINT <name>] (CHECK | UNIQUE | FOREIGN KEY | …)`
/// action (ADR-0035 §4g). The body is discriminated by its leading
/// concrete keyword. The optional `CONSTRAINT <name>` prefix becomes the
/// action-level `name` (used by CHECK + FK at execution; refused on
/// UNIQUE). `ADD PRIMARY KEY` parses (for a clean message) but is
/// refused — every playground table already has a PK.
fn build_alter_add_table_constraint(
path: &MatchedPath,
source: &str,
) -> Result<AlterTableAction, ValidationError> {
let name = ident(path, "constraint_name").map(str::to_string);
if path.contains_word("primary") {
return Err(ValidationError {
message_key: "parse.custom.alter_add_primary_key",
args: vec![],
});
}
let constraint = if path.contains_word("check") {
TableConstraint::Check {
expr_sql: capture_table_check_sql(path, source)?,
}
} else if path.contains_word("unique") {
if name.is_some() {
return Err(ValidationError {
message_key: "parse.custom.alter_named_unique",
args: vec![],
});
}
TableConstraint::Unique {
columns: collect_idents(path, "unique_column"),
}
} else {
// FOREIGN KEY — the §4g name lives at the action level, so the
// FK body itself is parsed unnamed.
TableConstraint::ForeignKey(build_alter_fk(path))
};
Ok(AlterTableAction::AddTableConstraint {
name,
constraint: Box::new(constraint),
})
}
/// Capture the raw SQL text of an `ADD … CHECK (<expr>)` (ADR-0035 §4g).
/// `sql_expr` is validate-only, so the expression is captured by byte
/// span — the 4a.2 / 4e mechanism.
fn capture_table_check_sql(
path: &MatchedPath,
source: &str,
) -> Result<String, ValidationError> {
let mut items = path.items.iter().peekable();
while let Some(item) = items.next() {
if matches!(item.kind, MatchedKind::Word("check"))
&& let Some((start, end)) = capture_parenthesised_span(&mut items)
{
return Ok(source[start..end].trim().to_string());
}
}
Err(ValidationError {
message_key: "parse.error_wrapper",
args: vec![("detail", "add check needs an expression".to_string())],
})
}
/// Build the `SqlForeignKey` for an `ADD [CONSTRAINT <name>] FOREIGN KEY
/// (<col>) REFERENCES <P>[(<col>)] [ON …]` (ADR-0035 §4g). Mirrors the
/// table-level FK walk in `build_sql_create_table`, reusing
/// `consume_fk_reference`. The name is supplied at the action level (so
/// the FK is parsed unnamed here).
fn build_alter_fk(path: &MatchedPath) -> SqlForeignKey {
let mut items = path.items.iter().peekable();
// Advance to the `foreign` keyword.
while items
.peek()
.is_some_and(|it| !matches!(it.kind, MatchedKind::Word("foreign")))
{
items.next();
}
items.next(); // `foreign`
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("key"))) {
items.next();
}
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) {
items.next();
}
let child_column = items.next().map_or_else(String::new, |it| it.text.clone());
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct(')'))) {
items.next();
}
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) {
items.next();
}
consume_fk_reference(&mut items, None, child_column)
}
pub static SQL_ALTER_TABLE: CommandNode = CommandNode {
entry: Word::keyword("alter"),
shape: SQL_ALTER_TABLE_SHAPE,
@@ -2529,7 +2712,7 @@ mod sql_create_index_tests {
#[cfg(test)]
mod sql_alter_table_tests {
use crate::dsl::command::{AlterTableAction, ColumnSpec, Command};
use crate::dsl::command::{AlterTableAction, ColumnSpec, Command, TableConstraint};
use crate::dsl::parser::parse_command_in_mode;
use crate::mode::Mode;
@@ -2696,6 +2879,130 @@ mod sql_alter_table_tests {
));
}
#[test]
fn add_table_check_unnamed_and_named() {
// ADR-0035 §4g: table-level CHECK, unnamed and named.
match alter("alter table T add check (a < b)").1 {
AlterTableAction::AddTableConstraint { name, constraint } => {
assert_eq!(name, None);
assert!(matches!(*constraint, TableConstraint::Check { ref expr_sql } if expr_sql == "a < b"));
}
other => panic!("expected AddTableConstraint/Check, got {other:?}"),
}
match alter("alter table T add constraint a_lt_b check (a < b)").1 {
AlterTableAction::AddTableConstraint { name, constraint } => {
assert_eq!(name.as_deref(), Some("a_lt_b"));
assert!(matches!(*constraint, TableConstraint::Check { .. }));
}
other => panic!("expected named AddTableConstraint/Check, got {other:?}"),
}
}
#[test]
fn add_composite_unique() {
match alter("alter table T add unique (a, b)").1 {
AlterTableAction::AddTableConstraint { name, constraint } => {
assert_eq!(name, None);
assert!(matches!(*constraint, TableConstraint::Unique { ref columns } if columns == &["a".to_string(), "b".to_string()]));
}
other => panic!("expected AddTableConstraint/Unique, got {other:?}"),
}
}
#[test]
fn named_unique_is_refused() {
// §4g: composite UNIQUE is anonymous in our model — naming it is
// refused by the BUILDER (it parses, then the builder rejects),
// so the error is the friendly message, not a parse error.
let err = parse_command_in_mode(
"alter table T add constraint u unique (a, b)",
Mode::Advanced,
)
.expect_err("a named UNIQUE constraint is refused");
assert!(
err.to_string().to_lowercase().contains("unique constraint cannot be named"),
"expected the builder's named-UNIQUE refusal, got: {err}"
);
}
#[test]
fn add_primary_key_is_refused() {
// §4g: adding a PK to an existing table is refused by the BUILDER
// (it parses for a clean message, then the builder rejects it).
let err = parse_command_in_mode("alter table T add primary key (id)", Mode::Advanced)
.expect_err("ADD PRIMARY KEY is refused");
assert!(
err.to_string().to_lowercase().contains("primary key is fixed at creation"),
"expected the builder's ADD-PRIMARY-KEY refusal, got: {err}"
);
}
#[test]
fn add_foreign_key_named_and_bare() {
// `add foreign key (col) references P(id)` and the bare
// `references P` form; named via the CONSTRAINT prefix.
match alter("alter table C add foreign key (pid) references P(id)").1 {
AlterTableAction::AddTableConstraint { name, constraint } => {
assert_eq!(name, None);
match *constraint {
TableConstraint::ForeignKey(fk) => {
assert_eq!(fk.child_column, "pid");
assert_eq!(fk.parent_table, "P");
assert_eq!(fk.parent_column.as_deref(), Some("id"));
}
other => panic!("expected ForeignKey, got {other:?}"),
}
}
other => panic!("expected AddTableConstraint/FK, got {other:?}"),
}
match alter("alter table C add constraint fk_p foreign key (pid) references P").1 {
AlterTableAction::AddTableConstraint { name, constraint } => {
assert_eq!(name.as_deref(), Some("fk_p"));
match *constraint {
TableConstraint::ForeignKey(fk) => {
assert_eq!(fk.parent_column, None, "bare reference resolves at execution");
}
other => panic!("expected ForeignKey, got {other:?}"),
}
}
other => panic!("expected named AddTableConstraint/FK, got {other:?}"),
}
}
#[test]
fn drop_constraint_by_name() {
match alter("alter table T drop constraint a_lt_b").1 {
AlterTableAction::DropConstraint { name } => assert_eq!(name, "a_lt_b"),
other => panic!("expected DropConstraint, got {other:?}"),
}
}
#[test]
fn six_branch_dispatch_still_routes_column_actions() {
// The two new add/drop-constraint branches do not steal the four
// column actions.
assert!(matches!(
alter("alter table T add column note text").1,
AlterTableAction::AddColumn(_)
));
assert!(matches!(
alter("alter table T add column code text unique").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 alter_is_advanced_only() {
// No simple `alter`; in simple mode it does not parse as a
+14 -4
View File
@@ -263,7 +263,10 @@ static TABLE_PK_NODES: &[Node] = &[
},
Node::Punct(')'),
];
const TABLE_PK: Node = Node::Seq(TABLE_PK_NODES);
// `pub(crate)` so `ALTER TABLE … ADD PRIMARY KEY (…)` (ADR-0035 §4g)
// reuses the body — it parses, then the ALTER builder refuses it with a
// specific message (adding a PK to an existing table is unsupported).
pub(crate) const TABLE_PK: Node = Node::Seq(TABLE_PK_NODES);
// Table-level `UNIQUE ( col, … )`. A single column normalises into
// that column's `unique` flag (round-trips via the existing
@@ -292,7 +295,9 @@ static TABLE_UNIQUE_NODES: &[Node] = &[
},
Node::Punct(')'),
];
const TABLE_UNIQUE: Node = Node::Seq(TABLE_UNIQUE_NODES);
// `pub(crate)` so `ALTER TABLE … ADD UNIQUE (…)` (ADR-0035 §4g) reuses
// the same composite-UNIQUE body.
pub(crate) const TABLE_UNIQUE: Node = Node::Seq(TABLE_UNIQUE_NODES);
// Table-level `CHECK ( <expr> )` (ADR-0035 §4a.3) — a multi-column
// CHECK referencing several columns. Same paren-bounded shape as the
@@ -306,7 +311,9 @@ static TABLE_CHECK_NODES: &[Node] = &[
Node::Subgrammar(&sql_expr::SQL_OR_EXPR),
Node::Punct(')'),
];
const TABLE_CHECK: Node = Node::Seq(TABLE_CHECK_NODES);
// `pub(crate)` so `ALTER TABLE … ADD [CONSTRAINT <name>] CHECK (…)`
// (ADR-0035 §4g) reuses the same table-CHECK body.
pub(crate) const TABLE_CHECK: Node = Node::Seq(TABLE_CHECK_NODES);
// Table-level foreign key (ADR-0035 §5, sub-phase 4b):
// `[CONSTRAINT <name>] FOREIGN KEY ( <child col> ) REFERENCES
@@ -356,7 +363,10 @@ static FOREIGN_KEY_BODY_NODES: &[Node] = &[
];
const FOREIGN_KEY_BODY: Node = Node::Seq(FOREIGN_KEY_BODY_NODES);
// `FOREIGN KEY (…) …` — the unnamed table-level FK (auto-named).
const TABLE_FK: Node = FOREIGN_KEY_BODY;
// `pub(crate)` so `ALTER TABLE … ADD [CONSTRAINT <name>] FOREIGN KEY
// (…)` (ADR-0035 §4g) reuses the FK body (the §4g `CONSTRAINT <name>`
// prefix is supplied by the ALTER grammar, not this body).
pub(crate) const TABLE_FK: Node = FOREIGN_KEY_BODY;
// `CONSTRAINT <name> FOREIGN KEY (…) …` — the named table-level FK.
static TABLE_FK_NAMED_NODES: &[Node] = &[
Node::Word(Word::keyword("constraint")),