ADR-0024 Phase B: DDL commands without value literals
Migrate the five DDL commands at four entry words: drop (drop
table / drop column / drop relationship), add (add column /
add 1:n relationship), rename (rename column), change (change
column). The walker route now owns these end-to-end; chumsky
declarations remain unreachable for these inputs but stay
until Phase F.
Walker extensions:
- New node kinds: NumberLit (with optional content validator)
and Literal(&str) (verbatim byte sequence with word-boundary
lookahead — used for the `1` in `add 1:n …` so it surfaces
as `\`1\`` in the expected-set, matching the existing
parse_error_pedagogy contract).
- Flag (--name) terminal — Phase A stubbed; now wired to the
walker driver with consume_flag() in lex_helpers.
- Repeated combinator with optional separator and `min` floor.
Used by referential clauses (0..2 `on <delete|update>` runs)
and change-column flags (0..N --force-conversion /
--dont-convert; AST builder enforces mutual exclusion).
- Optional now propagates its inner's expectations as a
`skipped` field on the Matched result. Seq accumulates these
across children so the next failure's expected-set surfaces
the full union — closes the keyword-completion regression
(`add column ` must offer `to`, `table`, plus the table-name
identifier slot).
- Expectation::Ident gained a `source: IdentSource` field; the
parser-side bridge maps Tables/Columns/Relationships/Types
to the IdentSlot::expected_label strings ("table name",
"column name", …) so the existing completion engine's
schema-cache lookup still resolves.
- Walker error wording now includes "after `<consumed>`,
expected …" framing — matches the chumsky-side test
contract for structural errors mid-shape.
- AST-builder validation errors now propagate as
WalkOutcome::ValidationFailed (not the generic "AST builder
failed" fallback), so `change column … --force-conversion
--dont-convert` and repeated `on delete` clauses surface
their friendly catalog wording verbatim.
Grammar additions:
- src/dsl/grammar/shared.rs: type-name validator (TYPE_VALIDATOR
uses Type::from_str via parse.custom.unknown_type catalog),
qualified_column sub-grammar, referential action keyword
(`cascade`/`restrict`/`set null`/`no action`), repeated
on-clauses.
- src/dsl/grammar/ddl.rs: drop/add/rename/change CommandNodes
with inline shapes (per-use-site `role` annotations let the
AST builder discriminate parent vs child columns, etc.).
The four entry words each have one CommandNode whose `shape`
is a Choice across sub-forms.
Tests:
- 14 new walker-specific tests covering all DDL forms (bare
drop table, drop column with optional connectives, drop
relationship by name and by endpoints, add column with type
validator, rename column, change column with each flag form
+ mutual-exclusion check, add 1:n relationship minimal /
full, repeated-clause-twice rejection).
- Total: 819 passed, 0 failed, 1 ignored (was 805 / 1).
- cargo clippy --all-targets -- -D warnings clean.
This commit is contained in:
+238
-10
@@ -92,7 +92,7 @@ pub fn walk(
|
||||
&mut path,
|
||||
&mut per_byte,
|
||||
) {
|
||||
NodeWalkResult::Matched { end } => {
|
||||
NodeWalkResult::Matched { end, .. } => {
|
||||
let trailing = skip_whitespace(effective_source, end);
|
||||
if trailing < effective_source.len() {
|
||||
WalkOutcome::Mismatch {
|
||||
@@ -128,14 +128,29 @@ pub fn walk(
|
||||
},
|
||||
};
|
||||
|
||||
let cmd = if matches!(outcome, WalkOutcome::Match { .. }) {
|
||||
(command_node.ast_builder)(&path).ok()
|
||||
} else {
|
||||
None
|
||||
// Apply the AST builder. A validation error here surfaces
|
||||
// as a `ValidationFailed` outcome (so the bridge can render
|
||||
// the catalog wording correctly) rather than as a generic
|
||||
// "AST builder failed" fallback.
|
||||
let (final_outcome, cmd) = match outcome {
|
||||
WalkOutcome::Match { .. } => match (command_node.ast_builder)(&path) {
|
||||
Ok(c) => (outcome, Some(c)),
|
||||
Err(error) => (
|
||||
WalkOutcome::ValidationFailed {
|
||||
position: path
|
||||
.items
|
||||
.last()
|
||||
.map_or(kw_start, |i| i.span.0),
|
||||
error,
|
||||
},
|
||||
None,
|
||||
),
|
||||
},
|
||||
other => (other, None),
|
||||
};
|
||||
|
||||
let result = WalkResult {
|
||||
outcome,
|
||||
outcome: final_outcome,
|
||||
matched_path: path,
|
||||
per_byte_class: per_byte,
|
||||
};
|
||||
@@ -369,13 +384,28 @@ mod tests {
|
||||
fn walker_import_trailing_as_without_target_errors() {
|
||||
let err = parse("import foo.zip as ").unwrap_err();
|
||||
match err {
|
||||
crate::dsl::ParseError::Invalid { message, expected, .. } => {
|
||||
crate::dsl::ParseError::Invalid {
|
||||
message, expected, ..
|
||||
} => {
|
||||
// Phase A: the friendly `project.import_empty_target`
|
||||
// wording moves out of the parser; the walker's
|
||||
// structural error names the `target` slot.
|
||||
// structural error names the slot via its
|
||||
// user-facing label. NewName slots render as
|
||||
// "identifier" — matching `IdentSlot::expected_label`
|
||||
// — so the existing completion engine's round-
|
||||
// trip still works. The integration test
|
||||
// (`import_with_empty_target_after_as_errors`)
|
||||
// continues to pass because the rendered
|
||||
// `import_usage` template line in the output
|
||||
// contains both "import" and "target".
|
||||
assert!(
|
||||
message.contains("target") || expected.iter().any(|e| e == "target"),
|
||||
"expected mention of target slot; got message={message:?}, expected={expected:?}"
|
||||
message.contains("identifier")
|
||||
|| expected.iter().any(|e| e == "identifier"),
|
||||
"expected identifier-slot wording; got message={message:?}, expected={expected:?}"
|
||||
);
|
||||
assert!(
|
||||
message.contains("import"),
|
||||
"expected `import` in 'after `<prefix>`' framing; got: {message}"
|
||||
);
|
||||
}
|
||||
other => panic!("expected Invalid, got {other:?}"),
|
||||
@@ -429,4 +459,202 @@ mod tests {
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
// =========================================================
|
||||
// Phase B — DDL commands.
|
||||
// =========================================================
|
||||
|
||||
use crate::dsl::action::ReferentialAction;
|
||||
use crate::dsl::command::{ChangeColumnMode, RelationshipSelector};
|
||||
use crate::dsl::types::Type;
|
||||
|
||||
#[test]
|
||||
fn walker_parses_drop_table() {
|
||||
assert_eq!(
|
||||
parse("drop table Customers").unwrap(),
|
||||
Command::DropTable {
|
||||
name: "Customers".to_string(),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_drop_column_with_optional_connectives() {
|
||||
let want = Command::DropColumn {
|
||||
table: "Customers".to_string(),
|
||||
column: "Email".to_string(),
|
||||
};
|
||||
assert_eq!(parse("drop column Customers: Email").unwrap(), want);
|
||||
assert_eq!(parse("drop column from Customers: Email").unwrap(), want);
|
||||
assert_eq!(parse("drop column from table Customers: Email").unwrap(), want);
|
||||
assert_eq!(parse("drop column table Customers: Email").unwrap(), want);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_drop_relationship_named() {
|
||||
assert_eq!(
|
||||
parse("drop relationship Orders_to_Customers").unwrap(),
|
||||
Command::DropRelationship {
|
||||
selector: RelationshipSelector::Named {
|
||||
name: "Orders_to_Customers".to_string(),
|
||||
},
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_drop_relationship_endpoints() {
|
||||
assert_eq!(
|
||||
parse("drop relationship from Customers.id to Orders.customer_id").unwrap(),
|
||||
Command::DropRelationship {
|
||||
selector: RelationshipSelector::Endpoints {
|
||||
parent_table: "Customers".to_string(),
|
||||
parent_column: "id".to_string(),
|
||||
child_table: "Orders".to_string(),
|
||||
child_column: "customer_id".to_string(),
|
||||
},
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_add_column() {
|
||||
assert_eq!(
|
||||
parse("add column Customers: Email (text)").unwrap(),
|
||||
Command::AddColumn {
|
||||
table: "Customers".to_string(),
|
||||
column: "Email".to_string(),
|
||||
ty: Type::Text,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_add_column_unknown_type_errors_with_friendly_wording() {
|
||||
let err = parse("add column Customers: Email (varchar)").unwrap_err();
|
||||
match err {
|
||||
crate::dsl::ParseError::Invalid { message, .. } => {
|
||||
assert!(message.contains("varchar"), "got: {message}");
|
||||
}
|
||||
other => panic!("expected Invalid, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_rename_column() {
|
||||
assert_eq!(
|
||||
parse("rename column Customers: Email to ContactEmail").unwrap(),
|
||||
Command::RenameColumn {
|
||||
table: "Customers".to_string(),
|
||||
old: "Email".to_string(),
|
||||
new: "ContactEmail".to_string(),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_change_column() {
|
||||
assert_eq!(
|
||||
parse("change column Customers: Email (text)").unwrap(),
|
||||
Command::ChangeColumnType {
|
||||
table: "Customers".to_string(),
|
||||
column: "Email".to_string(),
|
||||
ty: Type::Text,
|
||||
mode: ChangeColumnMode::Default,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_change_column_with_force_conversion_flag() {
|
||||
assert_eq!(
|
||||
parse("change column Customers: Email (int) --force-conversion").unwrap(),
|
||||
Command::ChangeColumnType {
|
||||
table: "Customers".to_string(),
|
||||
column: "Email".to_string(),
|
||||
ty: Type::Int,
|
||||
mode: ChangeColumnMode::ForceConversion,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_change_column_rejects_both_flags() {
|
||||
let err = parse("change column Customers: Email (int) --force-conversion --dont-convert")
|
||||
.unwrap_err();
|
||||
match err {
|
||||
crate::dsl::ParseError::Invalid { message, .. } => {
|
||||
assert!(message.contains("mutually exclusive"), "got: {message}");
|
||||
}
|
||||
other => panic!("expected Invalid, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_add_relationship_minimal() {
|
||||
assert_eq!(
|
||||
parse("add 1:n relationship from Customers.id to Orders.customer_id").unwrap(),
|
||||
Command::AddRelationship {
|
||||
name: None,
|
||||
parent_table: "Customers".to_string(),
|
||||
parent_column: "id".to_string(),
|
||||
child_table: "Orders".to_string(),
|
||||
child_column: "customer_id".to_string(),
|
||||
on_delete: ReferentialAction::default_action(),
|
||||
on_update: ReferentialAction::default_action(),
|
||||
create_fk: false,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_parses_add_relationship_with_name_and_actions_and_flag() {
|
||||
assert_eq!(
|
||||
parse(
|
||||
"add 1:n relationship as cust_orders from Customers.id to Orders.customer_id \
|
||||
on delete cascade on update set null --create-fk"
|
||||
)
|
||||
.unwrap(),
|
||||
Command::AddRelationship {
|
||||
name: Some("cust_orders".to_string()),
|
||||
parent_table: "Customers".to_string(),
|
||||
parent_column: "id".to_string(),
|
||||
child_table: "Orders".to_string(),
|
||||
child_column: "customer_id".to_string(),
|
||||
on_delete: ReferentialAction::Cascade,
|
||||
on_update: ReferentialAction::SetNull,
|
||||
create_fk: true,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walker_add_relationship_repeated_clause_errors() {
|
||||
let err = parse(
|
||||
"add 1:n relationship from Customers.id to Orders.customer_id \
|
||||
on delete cascade on delete restrict",
|
||||
)
|
||||
.unwrap_err();
|
||||
match err {
|
||||
crate::dsl::ParseError::Invalid { message, .. } => {
|
||||
assert!(
|
||||
message.contains("delete") && message.contains("twice"),
|
||||
"got: {message}"
|
||||
);
|
||||
}
|
||||
other => panic!("expected Invalid, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
// ---- Routing fall-through still works for non-DDL ----
|
||||
|
||||
#[test]
|
||||
fn walker_does_not_engage_for_show_data() {
|
||||
// `show` isn't migrated yet (Phase D); router falls
|
||||
// through to chumsky.
|
||||
assert!(matches!(
|
||||
parse("show data Customers").unwrap(),
|
||||
Command::ShowData { .. }
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user