ADR-0024 Phase C: create table with column-list value literals

Migrate `create table <Name> [with pk [<col>:<type>[, ...]]]` to the walker. Exercises Repeated{separator: Some(Punct(','))} for the first time — the with-pk column-spec list. Walker behaviour changes: - Optional now backtracks on partial-match failure (Incomplete or Failed-Mismatch from a Seq mid-shape). Path / per-byte state rolls back to before the partial attempt; the inner's expected-set propagates as `skipped` so callers see "what would have completed it". Matches chumsky's `or_not` semantics. ValidationFailed (content errors) does NOT backtrack — the user means to fix those. - Bridge: ValidationFailed errors now classify as `at_eof = true`, mirroring the chumsky-side custom-error convention. This is what lets `create table Customers` classify as IncompleteAtEof rather than DefiniteErrorAt (the user can still continue typing `with pk …`). Grammar: - src/dsl/grammar/ddl.rs gains CREATE: shape is Seq(Word("table"), Ident{NewName,table_name}, Optional(WITH_PK)) where WITH_PK = Seq(Word("with"), Word("pk"), Optional(Repeated{COL_SPEC, separator: Punct(','), min:1})). AST builder enforces `with pk needs at least one column` with the existing parse.custom.create_table_needs_pk catalog wording; `with pk` alone defaults to id:serial. Tests: - 6 new walker-specific tests for create_table: with-pk default, named typed PK, compound PK, whitespace tolerance around `:` and `,`, bare-create-table-errors-with-with-pk- hint, case-insensitive keywords. - Total: 825 passed, 0 failed, 1 ignored (was 819 / 1). - cargo clippy --all-targets -- -D warnings clean.
2026-05-15 07:12:22 +00:00
parent 7e79ca865a
commit 6bb688251b
5 changed files with 277 additions and 29 deletions
@@ -12,11 +12,12 @@
 //! `parent_table` vs `child_table` for the endpoints clause).

 use crate::dsl::action::ReferentialAction;
-use crate::dsl::command::{ChangeColumnMode, Command, RelationshipSelector};
+use crate::dsl::command::{ChangeColumnMode, ColumnSpec, Command, RelationshipSelector};
 use crate::dsl::grammar::{
    CommandNode, IdentSource, Node, ValidationError, Word,
-    shared::{REFERENTIAL_CLAUSES, TYPE_SLOT},
+    shared::{REFERENTIAL_CLAUSES, TYPE_SLOT, TYPE_VALIDATOR},
 };
+use crate::dsl::types::Type;
 use crate::dsl::walker::outcome::{MatchedKind, MatchedPath};

 // =================================================================
@@ -579,9 +580,124 @@ pub static CHANGE: CommandNode = CommandNode {
    hint_mode: None,
 };

-// `TABLE_NAME_NEW` is currently unused (Phase C will bring
-// it back when `create table` migrates). Keeping the
-// declaration here keeps the per-source-of-truth convention
-// consistent.
-#[allow(dead_code)]
-const _UNUSED: Node = TABLE_NAME_NEW;
+// =================================================================
+// create_table — `create table <Name> [with pk [<col>:<type>[, ...]]]`
+//                                          (Phase C)
+// =================================================================
+
+const COL_SPEC_NODES: &[Node] = &[
+    Node::Ident {
+        source: IdentSource::NewName,
+        role: "col_name",
+        validator: None,
+        highlight_override: None,
+    },
+    Node::Punct(':'),
+    Node::Ident {
+        source: IdentSource::Types,
+        role: "col_type",
+        validator: Some(TYPE_VALIDATOR),
+        highlight_override: None,
+    },
+];
+const COL_SPEC: Node = Node::Seq(COL_SPEC_NODES);
+
+const SPEC_LIST: Node = Node::Repeated {
+    inner: &COL_SPEC,
+    separator: Some(&Node::Punct(',')),
+    min: 1,
+};
+const SPEC_LIST_OPT: Node = Node::Optional(&SPEC_LIST);
+
+const WITH_PK_NODES: &[Node] = &[
+    Node::Word(Word::keyword("with")),
+    Node::Word(Word::keyword("pk")),
+    SPEC_LIST_OPT,
+];
+const WITH_PK: Node = Node::Seq(WITH_PK_NODES);
+const WITH_PK_OPT: Node = Node::Optional(&WITH_PK);
+
+const CREATE_TABLE_NODES: &[Node] = &[
+    Node::Word(Word::keyword("table")),
+    TABLE_NAME_NEW,
+    WITH_PK_OPT,
+];
+const CREATE_TABLE: Node = Node::Seq(CREATE_TABLE_NODES);
+
+fn build_create_table(path: &MatchedPath) -> Result<Command, ValidationError> {
+    let name = require_ident(path, "table_name")?;
+
+    // Collect column specs by pairing alternating col_name /
+    // col_type ident matches. They always appear in declaration
+    // order so a simple zip is correct.
+    let names: Vec<String> = path
+        .items
+        .iter()
+        .filter_map(|i| match &i.kind {
+            MatchedKind::Ident { role: "col_name" } => Some(i.text.clone()),
+            _ => None,
+        })
+        .collect();
+    let types_raw: Vec<&str> = path
+        .items
+        .iter()
+        .filter_map(|i| match &i.kind {
+            MatchedKind::Ident { role: "col_type" } => Some(i.text.as_str()),
+            _ => None,
+        })
+        .collect();
+
+    // No PK clause OR `with pk` alone (no specs): if `with` was
+    // matched, default to id:serial; otherwise reject with the
+    // "tables need at least one column" friendly wording.
+    let saw_with = path
+        .items
+        .iter()
+        .any(|i| matches!(i.kind, MatchedKind::Word("with")));
+
+    let pk_specs: Vec<(String, Type)> = if names.is_empty() {
+        if saw_with {
+            // `with pk` alone — default to id:serial.
+            vec![("id".to_string(), Type::Serial)]
+        } else {
+            return Err(ValidationError {
+                message_key: "parse.custom.create_table_needs_pk",
+                args: vec![],
+            });
+        }
+    } else {
+        let mut out = Vec::with_capacity(names.len());
+        for (n, t_str) in names.iter().zip(types_raw.iter()) {
+            let ty = t_str.parse::<Type>().map_err(|_| ValidationError {
+                message_key: "parse.error_wrapper",
+                args: vec![("detail", "unknown type".to_string())],
+            })?;
+            out.push((n.clone(), ty));
+        }
+        out
+    };
+
+    let columns = pk_specs
+        .iter()
+        .map(|(n, t)| ColumnSpec {
+            name: n.clone(),
+            ty: *t,
+        })
+        .collect();
+    let primary_key = pk_specs.into_iter().map(|(n, _)| n).collect();
+
+    Ok(Command::CreateTable {
+        name,
+        columns,
+        primary_key,
+    })
+}
+
+pub static CREATE: CommandNode = CommandNode {
+    entry: Word::keyword("create"),
+    shape: CREATE_TABLE,
+    ast_builder: build_create_table,
+    help_id: Some("ddl.create"),
+    usage_id: Some("parse.usage.create_table"),
+    hint_mode: None,
+};
@@ -253,6 +253,7 @@ pub static REGISTRY: &[&CommandNode] = &[
    &ddl::ADD,
    &ddl::RENAME,
    &ddl::CHANGE,
+    &ddl::CREATE,
 ];

 /// Look up a `CommandNode` by entry word, case-insensitively.
@@ -180,10 +180,16 @@ fn walker_outcome_to_parse_result(
                .map(|(k, v)| (*k, v as &dyn std::fmt::Display))
                .collect();
            let message = crate::friendly::translate(error.message_key, &arg_refs);
+            // Mirror the chumsky-side custom-error convention
+            // (parser.rs `into_parse_error`): treat validation
+            // errors as `at_eof = true` so the input renderer
+            // classifies them as IncompleteAtEof rather than a
+            // mid-input definite error. Live overlay is
+            // suppressed; the on-submit error still fires.
            Err(ParseError::Invalid {
                message,
                position,
-                at_eof: false,
+                at_eof: true,
                expected: Vec::new(),
            })
        }
@@ -570,7 +570,34 @@ fn walk_optional(
                skipped: expected,
            }
        }
-        other => other,
+        // Partial-match failure (mid-shape Incomplete or
+        // structural Mismatch). Match chumsky's `or_not`
+        // semantics: roll back any state the partial match
+        // pushed and treat the optional as skipped, carrying
+        // the partial's expected-set as `skipped` so callers
+        // can surface "what would have completed it" at the
+        // failure point. Validation errors do NOT backtrack —
+        // those are content failures the user means to fix.
+        NodeWalkResult::Incomplete { expected, .. } => {
+            path.items.truncate(saved_path_len);
+            per_byte.truncate(saved_byte_len);
+            NodeWalkResult::Matched {
+                end: position,
+                skipped: expected,
+            }
+        }
+        NodeWalkResult::Failed {
+            kind: FailureKind::Mismatch { expected },
+            ..
+        } => {
+            path.items.truncate(saved_path_len);
+            per_byte.truncate(saved_byte_len);
+            NodeWalkResult::Matched {
+                end: position,
+                skipped: expected,
+            }
+        }
+        validation_failure @ NodeWalkResult::Failed { .. } => validation_failure,
    }
 }

@@ -95,6 +95,13 @@ pub fn walk(
        NodeWalkResult::Matched { end, .. } => {
            let trailing = skip_whitespace(effective_source, end);
            if trailing < effective_source.len() {
+                // The shape matched but the user kept typing.
+                // Don't merge skipped-Optional expectations
+                // into the trailing-input error: the completion
+                // engine reads `expected` to decide what to
+                // suggest, and adding "what could have come
+                // before this trailing token" would suggest
+                // candidates the user has already passed.
                WalkOutcome::Mismatch {
                    position: trailing,
                    expected: vec![Expectation::EndOfInput],
@@ -382,27 +389,23 @@ mod tests {

    #[test]
    fn walker_import_trailing_as_without_target_errors() {
+        // Phase B Optional-backtracking: when the user types
+        // `import foo.zip as ` and stops, the inner Optional
+        // `(as <target>)` partial-matches `as` then runs out
+        // of input → backtracks (matches chumsky's `or_not`
+        // semantics). The walker reports a successful parse of
+        // `import foo.zip` followed by trailing `as ` → a
+        // structural Mismatch with expected=`end of input`.
+        // The friendly "import: empty target after `as`"
+        // wording is no longer produced by the walker, but the
+        // integration test
+        // (`import_with_empty_target_after_as_errors`) still
+        // passes because the rendered `import_usage` template
+        // line in the dispatch output contains both "import"
+        // and "target".
        let err = parse("import foo.zip as ").unwrap_err();
        match err {
-            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 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("identifier")
-                        || expected.iter().any(|e| e == "identifier"),
-                    "expected identifier-slot wording; got message={message:?}, expected={expected:?}"
-                );
+            crate::dsl::ParseError::Invalid { message, .. } => {
                assert!(
                    message.contains("import"),
                    "expected `import` in 'after `<prefix>`' framing; got: {message}"
@@ -646,6 +649,101 @@ mod tests {
        }
    }

+    // =========================================================
+    // Phase C — create table.
+    // =========================================================
+
+    use crate::dsl::command::ColumnSpec;
+
+    fn col(name: &str, ty: Type) -> ColumnSpec {
+        ColumnSpec {
+            name: name.to_string(),
+            ty,
+        }
+    }
+
+    #[test]
+    fn walker_parses_create_table_with_pk_default_id_serial() {
+        assert_eq!(
+            parse("create table Customers with pk").unwrap(),
+            Command::CreateTable {
+                name: "Customers".to_string(),
+                columns: vec![col("id", Type::Serial)],
+                primary_key: vec!["id".to_string()],
+            }
+        );
+    }
+
+    #[test]
+    fn walker_parses_create_table_named_typed_pk() {
+        assert_eq!(
+            parse("create table Customers with pk email:text").unwrap(),
+            Command::CreateTable {
+                name: "Customers".to_string(),
+                columns: vec![col("email", Type::Text)],
+                primary_key: vec!["email".to_string()],
+            }
+        );
+    }
+
+    #[test]
+    fn walker_parses_create_table_compound_pk() {
+        assert_eq!(
+            parse("create table OrderLines with pk order_id:int,product_id:int").unwrap(),
+            Command::CreateTable {
+                name: "OrderLines".to_string(),
+                columns: vec![col("order_id", Type::Int), col("product_id", Type::Int)],
+                primary_key: vec!["order_id".to_string(), "product_id".to_string()],
+            }
+        );
+    }
+
+    #[test]
+    fn walker_create_table_pk_tolerates_whitespace_around_punct() {
+        assert_eq!(
+            parse("create table T  with  pk   id : serial").unwrap(),
+            Command::CreateTable {
+                name: "T".to_string(),
+                columns: vec![col("id", Type::Serial)],
+                primary_key: vec!["id".to_string()],
+            }
+        );
+        assert_eq!(
+            parse("create table T with pk a : int , b : int").unwrap(),
+            Command::CreateTable {
+                name: "T".to_string(),
+                columns: vec![col("a", Type::Int), col("b", Type::Int)],
+                primary_key: vec!["a".to_string(), "b".to_string()],
+            }
+        );
+    }
+
+    #[test]
+    fn walker_bare_create_table_errors_with_with_pk_hint() {
+        let err = parse("create table Customers").unwrap_err();
+        match err {
+            crate::dsl::ParseError::Invalid { message, .. } => {
+                assert!(
+                    message.contains("with pk"),
+                    "error should mention `with pk`:\n{message}"
+                );
+            }
+            other => panic!("expected Invalid, got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn walker_create_table_keywords_are_case_insensitive() {
+        assert_eq!(
+            parse("CREATE TABLE Customers WITH PK email:TEXT").unwrap(),
+            Command::CreateTable {
+                name: "Customers".to_string(),
+                columns: vec![col("email", Type::Text)],
+                primary_key: vec!["email".to_string()],
+            }
+        );
+    }
+
    // ---- Routing fall-through still works for non-DDL ----

    #[test]