diff --git a/docs/plans/20260525-adr-0035-sql-ddl-4a2.md b/docs/plans/20260525-adr-0035-sql-ddl-4a2.md index 00f6c25..91f9571 100644 --- a/docs/plans/20260525-adr-0035-sql-ddl-4a2.md +++ b/docs/plans/20260525-adr-0035-sql-ddl-4a2.md @@ -71,10 +71,17 @@ CHECK). Builds directly on the 4a `SqlCreateTable` command + grammar. ### 4.1 Grammar (`src/dsl/grammar/sql_create_table.rs`) - **Column constraints** — extend `COL_CONSTRAINT_CHOICES` with: - - `DEFAULT `: `Seq[ Word("default"), - Subgrammar(&sql_expr::SQL_OR_EXPR) ]`. + - `DEFAULT ` where `` is a **literal** (number / + string / `null` / `true` / `false`) **or a parenthesised** + `( )` — *not* a bare `sql_expr`. This matches standard + SQL (a complex default must be parenthesised) **and** resolves a + real ambiguity found in testing: a bare `sql_expr` greedily + consumes a following `NOT` (as `NOT IN`/`NOT LIKE`/`NOT BETWEEN`), + breaking the common `DEFAULT 0 NOT NULL`. The parens give the + expression a clean right edge. (See §6.3.) - `CHECK ( )`: `Seq[ Word("check"), Punct('('), - Subgrammar(&sql_expr::SQL_OR_EXPR), Punct(')') ]`. + Subgrammar(&sql_expr::SQL_OR_EXPR), Punct(')') ]` — already + paren-bounded, so no ambiguity. - **Table element** — extend `ELEMENT_CHOICES` with table-level `UNIQUE ( col, … )`: `Seq[ Word("unique"), Punct('('), Repeated(uniq_column, ',', 1), Punct(')') ]`. (Distinct ident role, @@ -146,9 +153,13 @@ execution; `finalize_persistence` writes yaml/CSV/journal. No new unknown-column `[ERR]`. If they do, the grammar must suppress schema-existence checks in the CREATE-TABLE CHECK context (as the simple-mode path effectively does). Resolve during step 1. -2. **DEFAULT expression boundary** — confirm the `sql_expr` match is - maximal enough that the raw-text slice for `DEFAULT ` ends - cleanly before the next element. Covered by builder tests. +2. **DEFAULT grammar — resolved during implementation (§4.1).** A bare + `DEFAULT ` greedily ate a following `NOT` (start of + `NOT IN`/`LIKE`/`BETWEEN`), so `DEFAULT 0 NOT NULL` failed to parse + (caught by a builder test). Fixed by restricting a bare default to a + **literal**, with complex defaults **parenthesised** (`DEFAULT + (expr)`) — standard SQL, and the parens bound the expression. The + raw-text capture keeps the parens so it re-emits as valid SQL. ## 7. Devil's Advocate review of this plan diff --git a/docs/requirements.md b/docs/requirements.md index 1d6c5a6..d423eff 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -215,11 +215,12 @@ handoff-14 cleanup; 449 after B2/C2.) the same completion / highlighting / hints as the DSL (ADR-0001's `sqlparser-rs` reservation is superseded). Implemented so far: full `SELECT` (ADR-0032), `INSERT` / `UPDATE` / `DELETE` (ADR-0033), and - `CREATE TABLE` (ADR-0035 sub-phase 4a, 2026-05-25 — columns + types + - `NOT NULL` / `UNIQUE` / `PRIMARY KEY` + `IF NOT EXISTS`, executed - structurally). Remaining DDL — `CREATE TABLE` constraints (4a.2), - FK (4b), `DROP TABLE` (4c), indexes (4d), `ALTER TABLE` (4e–4h) — is - phased per ADR-0035 §13.)* + `CREATE TABLE` (ADR-0035, 2026-05-25 — executed structurally: columns + + types + `NOT NULL`/`UNIQUE`/`PRIMARY KEY` + `IF NOT EXISTS` (4a), + then per-column `DEFAULT`/`CHECK` (raw `sql_expr` text) and composite + `UNIQUE(a,b)` (4a.2)). Remaining DDL — table-level/multi-column + `CHECK` (4a.3), FK (4b), `DROP TABLE` (4c), indexes (4d), + `ALTER TABLE` (4e–4h) — is phased per ADR-0035 §13.)* - [ ] **Q2** Non-standard syntax rejected with a clear message pointing at the supported subset. *(Design done — ADR-0030 §8: out-of-subset statements are diff --git a/src/db.rs b/src/db.rs index d0817af..2708604 100644 --- a/src/db.rs +++ b/src/db.rs @@ -465,6 +465,7 @@ enum Request { name: String, columns: Vec, primary_key: Vec, + unique_constraints: Vec>, if_not_exists: bool, source: Option, reply: oneshot::Sender>, @@ -833,6 +834,7 @@ impl Database { name: String, columns: Vec, primary_key: Vec, + unique_constraints: Vec>, if_not_exists: bool, source: Option, ) -> Result { @@ -841,6 +843,7 @@ impl Database { name, columns, primary_key, + unique_constraints, if_not_exists, source, reply, @@ -1687,12 +1690,14 @@ fn handle_request( &name, &columns, &primary_key, + &[], )); } Request::SqlCreateTable { name, columns, primary_key, + unique_constraints, if_not_exists, source, reply, @@ -1720,6 +1725,7 @@ fn handle_request( &name, &columns, &primary_key, + &unique_constraints, ) .map(CreateOutcome::Created) }); @@ -2317,6 +2323,7 @@ fn read_schema_snapshot(conn: &Connection) -> Result { name: name.clone(), primary_key: read.primary_key.clone(), columns, + unique_constraints: read.unique_constraints.clone(), }); } @@ -2493,7 +2500,14 @@ fn column_constraints_sql(spec: &ColumnSpec) -> Result { if spec.unique { sql.push_str(" UNIQUE"); } - if let Some(literal) = default_sql_literal(spec)? { + // Advanced-mode raw `DEFAULT ` (ADR-0035 §4a.2) takes + // precedence over a simple-mode typed default; SQLite stores the + // literal text and `PRAGMA table_info` reports it back for the + // round-trip (no metadata needed for DEFAULT). + if let Some(raw) = &spec.default_sql { + sql.push_str(" DEFAULT "); + sql.push_str(raw); + } else if let Some(literal) = default_sql_literal(spec)? { sql.push_str(" DEFAULT "); sql.push_str(&literal); } @@ -2554,6 +2568,7 @@ fn read_schema_for_specs(columns: &[ColumnSpec], primary_key: &[String]) -> Read .collect(), primary_key: primary_key.to_vec(), foreign_keys: Vec::new(), + unique_constraints: Vec::new(), } } @@ -2598,6 +2613,7 @@ fn do_create_table( name: &str, columns: &[ColumnSpec], primary_key: &[String], + unique_constraints: &[Vec], ) -> Result { if columns.is_empty() { // SQLite requires at least one column. The DSL grammar @@ -2629,9 +2645,16 @@ fn do_create_table( // by the DDL clause and the metadata insert below. The // minimal schema gives `compile_expr` the column types. let check_schema = read_schema_for_specs(columns, primary_key); + // Advanced-mode raw `CHECK` text (ADR-0035 §4a.2) wins over a + // compiled simple-mode `Expr`; both are stored verbatim in the + // column metadata and echoed by `schema_to_ddl` on rebuild. let check_sqls: Vec> = columns .iter() - .map(|c| c.check.as_ref().map(|e| compile_check_sql(e, &check_schema))) + .map(|c| { + c.check_sql + .clone() + .or_else(|| c.check.as_ref().map(|e| compile_check_sql(e, &check_schema))) + }) .collect(); let mut column_clauses: Vec = Vec::with_capacity(columns.len()); @@ -2669,6 +2692,16 @@ fn do_create_table( ddl.push(')'); } + // Composite (multi-column) UNIQUE constraints (ADR-0035 §4a.2). + // Single-column UNIQUE rides on the column's inline `UNIQUE`; these + // are the multi-column table-level constraints. + for cols in unique_constraints { + let idents: Vec = cols.iter().map(|n| quote_ident(n)).collect(); + ddl.push_str(", UNIQUE ("); + ddl.push_str(&idents.join(", ")); + ddl.push(')'); + } + ddl.push_str(") STRICT;"); debug!(ddl = %ddl, "create_table"); @@ -4685,6 +4718,10 @@ struct ReadSchema { columns: Vec, primary_key: Vec, foreign_keys: Vec, + /// Composite (multi-column) UNIQUE constraints (ADR-0035 §4a.2), + /// read from the UNIQUE-constraint indexes (`origin = 'u'`). + /// Single-column UNIQUE rides on `ReadColumn::unique` instead. + unique_constraints: Vec>, } #[derive(Debug, Clone)] @@ -4767,13 +4804,12 @@ fn read_schema(conn: &Connection, table: &str) -> Result { .map(|c| c.name.clone()) .collect(); - // Detect single-column UNIQUE constraints (ADR-0018 §4). + // Detect UNIQUE constraints (ADR-0018 §4, ADR-0035 §4a.2). // pragma_index_list returns one row per index; we filter to - // unique indexes whose origin is "u" (a UNIQUE constraint, - // as opposed to "pk" or "c"). For each, pragma_index_info - // gives the constituent column(s); we only mark single- - // column unique here. Compound UNIQUE is out of scope. - let unique_columns = read_unique_columns(conn, table)?; + // unique indexes whose origin is "u" (a UNIQUE constraint, as + // opposed to "pk" or "c"). Single-column → the column's `unique` + // flag; multi-column → a composite `unique_constraints` entry. + let (unique_columns, unique_constraints) = read_unique_constraints(conn, table)?; for col in &mut columns { if unique_columns.contains(&col.name) { col.unique = true; @@ -4810,6 +4846,7 @@ fn read_schema(conn: &Connection, table: &str) -> Result { columns, primary_key, foreign_keys, + unique_constraints, }) } @@ -4888,11 +4925,18 @@ fn parse_action_from_sqlite(s: &str) -> ReferentialAction { /// `"u"` (a UNIQUE constraint, not a PK-implied or CHECK /// auto-index) and which cover exactly one column. Compound /// UNIQUE is deferred to a future ADR (out of scope for ADR-0018). -fn read_unique_columns( +/// Read the table's `UNIQUE` constraints (ADR-0018 §4, ADR-0035 +/// §4a.2) from the constraint-backing indexes (`origin = 'u'`). +/// Returns `(single_column_names, composite_constraints)`: a +/// single-column UNIQUE rides on the column's `unique` flag, while a +/// multi-column UNIQUE becomes a `Vec` of its columns (in +/// index order). +fn read_unique_constraints( conn: &Connection, table: &str, -) -> Result, DbError> { - let mut out: std::collections::HashSet = std::collections::HashSet::new(); +) -> Result<(std::collections::HashSet, Vec>), DbError> { + let mut single: std::collections::HashSet = std::collections::HashSet::new(); + let mut composite: Vec> = Vec::new(); let mut idx_stmt = conn .prepare( "SELECT name, \"unique\", origin \ @@ -4916,18 +4960,22 @@ fn read_unique_columns( continue; } let mut info_stmt = conn - .prepare("SELECT name FROM pragma_index_info(?1);") + .prepare("SELECT name FROM pragma_index_info(?1) ORDER BY seqno;") .map_err(DbError::from_rusqlite)?; let cols: Vec = info_stmt .query_map([&idx_name], |row| row.get::<_, String>(0)) .map_err(DbError::from_rusqlite)? .collect::, _>>() .map_err(DbError::from_rusqlite)?; - if cols.len() == 1 { - out.insert(cols.into_iter().next().expect("len 1")); + match cols.len() { + 0 => {} + 1 => { + single.insert(cols.into_iter().next().expect("len 1")); + } + _ => composite.push(cols), } } - Ok(out) + Ok((single, composite)) } /// Generate the CREATE TABLE DDL from a `ReadSchema`. Used during @@ -4986,6 +5034,14 @@ fn schema_to_ddl(table: &str, schema: &ReadSchema) -> String { clauses.push(format!("PRIMARY KEY ({})", pk_idents.join(", "))); } + // Composite (multi-column) UNIQUE constraints (ADR-0035 §4a.2) — + // emitted identically to `do_create_table` so a created table and + // its rebuilt form match. + for cols in &schema.unique_constraints { + let idents: Vec = cols.iter().map(|n| quote_ident(n)).collect(); + clauses.push(format!("UNIQUE ({})", idents.join(", "))); + } + for fk in &schema.foreign_keys { clauses.push(format!( "FOREIGN KEY ({child}) REFERENCES {parent_table}({parent_col}) \ @@ -7447,6 +7503,7 @@ fn build_read_schema(table: &TableSchema, relationships: &[RelationshipSchema]) columns, primary_key: table.primary_key.clone(), foreign_keys, + unique_constraints: table.unique_constraints.clone(), } } @@ -10365,6 +10422,8 @@ mod tests { unique, default, check: None, + default_sql: None, + check_sql: None, } } @@ -11146,6 +11205,7 @@ mod tests { ], primary_key: vec!["id".to_string()], foreign_keys: vec![], + unique_constraints: Vec::new(), }; let ddl = schema_to_ddl("T", &schema); assert!( diff --git a/src/dsl/command.rs b/src/dsl/command.rs index 437001a..6b571fd 100644 --- a/src/dsl/command.rs +++ b/src/dsl/command.rs @@ -30,11 +30,20 @@ pub struct ColumnSpec { /// `UNIQUE` — non-`NULL` values must be distinct (ADR-0029). pub unique: bool, /// `DEFAULT ` — the value used when an `insert` - /// omits this column (ADR-0029). + /// omits this column (ADR-0029). Simple-mode form. pub default: Option, /// `CHECK ()` — every row must satisfy this boolean - /// expression (ADR-0029). + /// expression (ADR-0029). Simple-mode form (a typed `Expr`). pub check: Option, + /// Advanced-mode raw-SQL `DEFAULT` (ADR-0035 §4a.2): the + /// expression text captured from a SQL `CREATE TABLE`, since + /// `sql_expr` yields no `Expr`. When `Some`, it takes precedence + /// over `default` in DDL emission. `None` in simple mode. + pub default_sql: Option, + /// Advanced-mode raw-SQL `CHECK` (ADR-0035 §4a.2): the inner + /// expression text (without the `CHECK ( … )` wrapper). When + /// `Some`, it takes precedence over `check`. `None` in simple mode. + pub check_sql: Option, } impl ColumnSpec { @@ -49,6 +58,8 @@ impl ColumnSpec { unique: false, default: None, check: None, + default_sql: None, + check_sql: None, } } } @@ -133,6 +144,10 @@ pub enum Command { name: String, columns: Vec, primary_key: Vec, + /// Composite (multi-column) `UNIQUE (a, b)` table constraints + /// (ADR-0035 §4a.2). Single-column table-level `UNIQUE` is + /// folded into the column's `unique` flag instead. + unique_constraints: Vec>, if_not_exists: bool, }, /// Add a column to an existing table. The column carries diff --git a/src/dsl/grammar/ddl.rs b/src/dsl/grammar/ddl.rs index 3a6bb9d..033ea9d 100644 --- a/src/dsl/grammar/ddl.rs +++ b/src/dsl/grammar/ddl.rs @@ -1288,17 +1288,27 @@ pub static CREATE: CommandNode = CommandNode { help_id: Some("ddl.create"), usage_ids: &["parse.usage.create_table"],}; +/// The friendly error for a column type without a preceding name — +/// a structural impossibility given the grammar, defended anyway. +fn sql_col_type_without_name() -> ValidationError { + ValidationError { + message_key: "parse.error_wrapper", + args: vec![("detail", "column type without a name".to_string())], + } +} + /// Build a `Command::SqlCreateTable` from the advanced-mode SQL -/// `CREATE TABLE` shape (ADR-0035 §1, sub-phase 4a). Executes +/// `CREATE TABLE` shape (ADR-0035 §1, sub-phases 4a + 4a.2). Executes /// structurally — extracts the same `ColumnSpec`/`primary_key` the /// simple-mode builder produces so the worker reuses `do_create_table`. /// -/// 4a surface: columns + types (the §3 alias map incl. `double -/// precision`) + `NOT NULL` / `UNIQUE` / column- and table-level -/// `PRIMARY KEY` + `IF NOT EXISTS`. `DEFAULT` / `CHECK` / -/// table-level `UNIQUE` are absent from the grammar (4a.2), so they -/// never reach this builder. -fn build_sql_create_table(path: &MatchedPath, _source: &str) -> Result { +/// Surface: columns and types (the §3 alias map incl. `double +/// precision`), `NOT NULL` / `UNIQUE` / column- and table-level +/// `PRIMARY KEY`, and `IF NOT EXISTS` (4a); per-column `DEFAULT` and +/// `CHECK` (raw `sql_expr` text captured by byte span — `sql_expr` +/// builds no AST) and composite `UNIQUE (a, b)` (4a.2). Table-level +/// multi-column `CHECK` and FK are absent from the grammar (4a.3 / 4b). +fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result { let name = require_ident(path, "table_name")?; // `if` only appears in the `IF NOT EXISTS` prefix (the `not` of // `NOT NULL` never carries an `if`), so its presence is the flag. @@ -1309,6 +1319,7 @@ fn build_sql_create_table(path: &MatchedPath, _source: &str) -> Result = Vec::new(); let mut primary_key: Vec = Vec::new(); + let mut unique_constraints: Vec> = Vec::new(); let mut pending_name: Option = None; let mut items = path.items.iter().peekable(); while let Some(item) = items.next() { @@ -1323,10 +1334,7 @@ fn build_sql_create_table(path: &MatchedPath, _source: &str) -> Result Result Result { - if let Some(last) = columns.last_mut() { + if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { + items.next(); // consume '(' + let mut cols: Vec = Vec::new(); + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Ident { role: "unique_column", .. } => { + cols.push(it.text.clone()); + items.next(); + } + MatchedKind::Punct(',') => { + items.next(); + } + MatchedKind::Punct(')') => { + items.next(); + break; + } + _ => break, + } + } + // Single-column table-level UNIQUE folds into the + // column's flag (round-trips via the single-column + // path); composite (or a name not among the + // columns) becomes a constraint. + match columns.iter_mut().find(|c| cols.len() == 1 && c.name == cols[0]) { + Some(c) => c.unique = true, + None if !cols.is_empty() => unique_constraints.push(cols), + None => {} + } + } else if let Some(last) = columns.last_mut() { last.unique = true; } } @@ -1385,6 +1420,26 @@ fn build_sql_create_table(path: &MatchedPath, _source: &str) -> Result` — capture the expression's raw SQL text + // by byte span (`sql_expr` builds no AST). The match is + // maximal, so the expression runs until a depth-0 element + // boundary (`,` / `)`) or the next constraint keyword. + MatchedKind::Word("default") => { + if let Some((s, e)) = capture_expr_span(&mut items) + && let Some(last) = columns.last_mut() + { + last.default_sql = Some(source[s..e].trim().to_string()); + } + } + // `check ( )` — capture the inner expression text + // (without the wrapping parens) by matching paren depth. + MatchedKind::Word("check") => { + if let Some((s, e)) = capture_parenthesised_span(&mut items) + && let Some(last) = columns.last_mut() + { + last.check_sql = Some(source[s..e].trim().to_string()); + } + } _ => {} } } @@ -1405,10 +1460,77 @@ fn build_sql_create_table(path: &MatchedPath, _source: &str) -> Result` expression from the +/// matched-item stream (ADR-0035 §4a.2). Consumes the expression's +/// terminals (tracking paren depth) and stops *without consuming* the +/// next depth-0 element boundary (`,` / `)`) or constraint keyword +/// (`not` / `unique` / `primary` / `check`) — those terminals were +/// matched by the following constraint/element, not by the expression. +/// Returns `(start, end)` byte offsets, or `None` if no expression +/// terminal followed. +fn capture_expr_span<'a, I>(items: &mut std::iter::Peekable) -> Option<(usize, usize)> +where + I: Iterator, +{ + let mut depth = 0usize; + let mut start: Option = None; + let mut end = 0usize; + while let Some(it) = items.peek() { + match &it.kind { + MatchedKind::Punct(',' | ')') if depth == 0 => break, + MatchedKind::Word("not" | "unique" | "primary" | "check") if depth == 0 => break, + _ => { + match &it.kind { + MatchedKind::Punct('(') => depth += 1, + MatchedKind::Punct(')') => depth = depth.saturating_sub(1), + _ => {} + } + start.get_or_insert(it.span.0); + end = it.span.1; + items.next(); + } + } + } + start.map(|s| (s, end)) +} + +/// Capture the byte span of the contents of a parenthesised group +/// (`CHECK ( )`) from the matched-item stream — the next item +/// must be the opening `(`. Consumes through the matching `)` (tracking +/// nested parens) and returns the `(start, end)` offsets of the text +/// *between* the parens, or `None` if no `(` follows. +fn capture_parenthesised_span<'a, I>(items: &mut std::iter::Peekable) -> Option<(usize, usize)> +where + I: Iterator, +{ + if !matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Punct('('))) { + return None; + } + let open = items.next()?; // '(' + let inner_start = open.span.1; + let mut depth = 1usize; + let mut inner_end = inner_start; + for it in items.by_ref() { + match &it.kind { + MatchedKind::Punct('(') => depth += 1, + MatchedKind::Punct(')') => { + depth -= 1; + if depth == 0 { + inner_end = it.span.0; + break; + } + } + _ => {} + } + } + Some((inner_start, inner_end)) +} + pub static SQL_CREATE_TABLE: CommandNode = CommandNode { entry: Word::keyword("create"), shape: Node::Subgrammar(&super::sql_create_table::SQL_CREATE_TABLE_SHAPE), diff --git a/src/dsl/grammar/sql_create_table.rs b/src/dsl/grammar/sql_create_table.rs index 7e11532..1cc4597 100644 --- a/src/dsl/grammar/sql_create_table.rs +++ b/src/dsl/grammar/sql_create_table.rs @@ -24,7 +24,7 @@ //! `sql_insert::SQL_INSERT_SHAPE`, which starts at `INTO`). use crate::dsl::grammar::sql_select::reject_internal_table; -use crate::dsl::grammar::{IdentSource, Node, ValidationError, Word}; +use crate::dsl::grammar::{IdentSource, Node, ValidationError, Word, sql_expr}; use crate::dsl::types::Type; static COMMA: Node = Node::Punct(','); @@ -107,13 +107,47 @@ static PRIMARY_KEY_NODES: &[Node] = &[ Node::Word(Word::keyword("primary")), Node::Word(Word::keyword("key")), ]; -// `NOT NULL` | `UNIQUE` | `PRIMARY KEY`. `DEFAULT` / `CHECK` are -// deliberately absent (4a.2): typing them is an ordinary parse error -// until the constraint slice lands. +// `DEFAULT ` / `CHECK ()` reuse the full ADR-0031 +// `sql_expr` surface (the same fragment `WHERE`/projections use). The +// fragment is validate-only (no AST), so the builder captures the +// matched text's **raw SQL** by byte span (ADR-0035 §4a.2). +// +// A bare `DEFAULT` value is a **literal** (or a *parenthesised* +// expression) — matching standard SQL, where a complex default must be +// `DEFAULT (expr)`. This is not just spec fidelity: a bare unbounded +// `sql_expr` greedily consumes a following `NOT` (as the start of +// `NOT IN`/`NOT LIKE`/`NOT BETWEEN`), which would break the common +// `DEFAULT 0 NOT NULL`. The parens give the expression a clean end. +static DEFAULT_PAREN_EXPR_NODES: &[Node] = &[ + Node::Punct('('), + Node::Subgrammar(&sql_expr::SQL_OR_EXPR), + Node::Punct(')'), +]; +static DEFAULT_VALUE_CHOICES: &[Node] = &[ + Node::Seq(DEFAULT_PAREN_EXPR_NODES), + Node::NumberLit { validator: None }, + Node::StringLit, + Node::Word(Word::keyword("null")), + Node::Word(Word::keyword("true")), + Node::Word(Word::keyword("false")), +]; +const DEFAULT_VALUE: Node = Node::Choice(DEFAULT_VALUE_CHOICES); +static DEFAULT_NODES: &[Node] = &[Node::Word(Word::keyword("default")), DEFAULT_VALUE]; +static CHECK_NODES: &[Node] = &[ + Node::Word(Word::keyword("check")), + Node::Punct('('), + Node::Subgrammar(&sql_expr::SQL_OR_EXPR), + Node::Punct(')'), +]; +// `NOT NULL` | `UNIQUE` | `PRIMARY KEY` | `DEFAULT ` | +// `CHECK ()`. Each branch starts on a distinct keyword, so the +// `Choice` never ambiguously commits. static COL_CONSTRAINT_CHOICES: &[Node] = &[ Node::Seq(NOT_NULL_NODES), Node::Word(Word::keyword("unique")), Node::Seq(PRIMARY_KEY_NODES), + Node::Seq(DEFAULT_NODES), + Node::Seq(CHECK_NODES), ]; const COL_CONSTRAINT: Node = Node::Choice(COL_CONSTRAINT_CHOICES); /// Zero-or-more column constraints after the type (`min: 0`). @@ -173,12 +207,41 @@ static TABLE_PK_NODES: &[Node] = &[ ]; 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 +// single-column path); two or more become a composite UNIQUE +// constraint (ADR-0035 §4a.2). Distinct ident role from `pk_column` +// so the builder routes them separately. +const UNIQUE_COLUMN_REF: Node = Node::Ident { + source: IdentSource::NewName, + role: "unique_column", + 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, +}; +static TABLE_UNIQUE_NODES: &[Node] = &[ + Node::Word(Word::keyword("unique")), + Node::Punct('('), + Node::Repeated { + inner: &UNIQUE_COLUMN_REF, + separator: Some(&COMMA), + min: 1, + }, + Node::Punct(')'), +]; +const TABLE_UNIQUE: Node = Node::Seq(TABLE_UNIQUE_NODES); + // One element of the column list: a table-level `PRIMARY KEY (…)` or a // column definition. `TABLE_PK` is tried first — it starts with the // keyword `primary`, which disambiguates it from a column name. (A // column literally named `primary` is therefore unavailable, the same // trade real SQL makes with its reserved words.) -static ELEMENT_CHOICES: &[Node] = &[TABLE_PK, COLUMN_DEF]; +static ELEMENT_CHOICES: &[Node] = &[TABLE_PK, TABLE_UNIQUE, COLUMN_DEF]; const ELEMENT: Node = Node::Choice(ELEMENT_CHOICES); static COLUMN_LIST_NODES: &[Node] = &[ @@ -364,14 +427,30 @@ mod tests { } #[test] - fn deferred_constraints_are_not_accepted_in_4a() { - // DEFAULT / CHECK / table-level UNIQUE belong to the 4a.2 - // constraint slice; their shapes are absent here, so they do - // not walk (surfacing as a parse error with the usage - // skeleton, which lists the supported surface). - bad("table t (id int default 0)"); - bad("table t (id int check (id > 0))"); - bad("table t (a int, b int, unique (a, b))"); + fn column_default_and_check_accepted() { + // 4a.2: DEFAULT / CHECK reuse the full sql_expr surface. + good("table t (id int, n int default 0)"); + good("table t (id int, name text default 'x')"); + good("table t (id int check (id > 0))"); + good("table t (id int check (id > 0 and id < 100))"); + good("table t (price real default 0.0 check (price >= 0.0))"); + } + + #[test] + fn table_level_unique_accepted() { + // 4a.2: composite + single-column table-level UNIQUE. + good("table t (a int, b int, unique (a, b))"); + good("table t (a int, b text, unique (b))"); + good("table t (id int primary key, email text, unique (email))"); + } + + #[test] + fn table_level_check_and_fk_still_rejected() { + // Table-level (multi-column) CHECK is 4a.3 (needs a metadata + // table); FK is 4b. Neither shape exists here yet. + bad("table t (a int, b int, check (a < b))"); + bad("table t (id int, ref int references other(id))"); + bad("table t (id int, foreign key (id) references other(id))"); } } @@ -383,7 +462,7 @@ mod tests { #[cfg(test)] mod builder_tests { - use crate::dsl::command::Command; + use crate::dsl::command::{ColumnSpec, Command}; use crate::dsl::parser::{parse_command, parse_command_in_mode}; use crate::dsl::types::Type; use crate::mode::Mode; @@ -396,6 +475,7 @@ mod builder_tests { columns, primary_key, if_not_exists, + .. } => ( name, columns.into_iter().map(|c| (c.name, c.ty)).collect(), @@ -540,4 +620,80 @@ mod builder_tests { "SQL CREATE TABLE must not parse in simple mode" ); } + + // --- 4a.2: CHECK / DEFAULT raw text + composite UNIQUE --- + + /// Parse and return the full `SqlCreateTable` columns + + /// composite-unique constraints. + fn parse_sct(input: &str) -> (Vec, Vec>) { + match parse_command(input).expect("should parse") { + Command::SqlCreateTable { + columns, + unique_constraints, + .. + } => (columns, unique_constraints), + other => panic!("expected SqlCreateTable, got {other:?}"), + } + } + + fn col<'a>(cols: &'a [ColumnSpec], name: &str) -> &'a ColumnSpec { + cols.iter().find(|c| c.name == name).expect("column") + } + + #[test] + fn check_captures_raw_inner_sql_text() { + let (cols, _) = parse_sct("create table t (id int check (id > 0))"); + assert_eq!(col(&cols, "id").check_sql.as_deref(), Some("id > 0")); + } + + #[test] + fn check_with_nested_parens_captures_balanced_text() { + let (cols, _) = parse_sct("create table t (a int, b int check ((a + b) > 0))"); + assert_eq!(col(&cols, "b").check_sql.as_deref(), Some("(a + b) > 0")); + } + + #[test] + fn default_captures_raw_sql_text() { + let (cols, _) = + parse_sct("create table t (id int primary key, n int default 42, s text default 'x')"); + assert_eq!(col(&cols, "n").default_sql.as_deref(), Some("42")); + assert_eq!(col(&cols, "s").default_sql.as_deref(), Some("'x'")); + } + + #[test] + fn default_expression_stops_before_following_constraint() { + // The boundary case: `default 0 not null` — the expr is just + // `0`; `not null` is the next constraint, not part of it. + let (cols, _) = parse_sct("create table t (id int, n int default 0 not null)"); + let n = col(&cols, "n"); + assert_eq!(n.default_sql.as_deref(), Some("0")); + assert!(n.not_null, "NOT NULL still recognised after the default"); + } + + #[test] + fn parenthesised_default_captures_expression_with_parens() { + // A complex (non-literal) default must be parenthesised + // (standard SQL); the captured text keeps the parens so it + // re-emits as valid `DEFAULT (…)`. + let (cols, _) = parse_sct("create table t (id int, n int default (1 + 2) not null)"); + let n = col(&cols, "n"); + assert_eq!(n.default_sql.as_deref(), Some("(1 + 2)")); + assert!(n.not_null); + } + + #[test] + fn composite_unique_collected_as_constraint() { + let (cols, uniq) = parse_sct("create table t (a int, b int, unique (a, b))"); + assert_eq!(uniq, vec![vec!["a".to_string(), "b".to_string()]]); + // The columns themselves are not individually unique. + assert!(!col(&cols, "a").unique && !col(&cols, "b").unique); + } + + #[test] + fn single_column_table_unique_folds_into_the_column() { + let (cols, uniq) = parse_sct("create table t (a int, b text, unique (b))"); + assert!(uniq.is_empty(), "single-column UNIQUE is not a composite"); + assert!(col(&cols, "b").unique, "it folds into the column's flag"); + assert!(!col(&cols, "a").unique); + } } diff --git a/src/persistence/mod.rs b/src/persistence/mod.rs index 5970748..51ee37e 100644 --- a/src/persistence/mod.rs +++ b/src/persistence/mod.rs @@ -140,6 +140,12 @@ pub struct TableSchema { pub name: String, pub primary_key: Vec, pub columns: Vec, + /// Composite (multi-column) `UNIQUE` constraints (ADR-0035 + /// §4a.2). Single-column UNIQUE is carried on the column's + /// `ColumnSchema::unique` flag instead. Empty for project files + /// written before composite UNIQUE existed — the YAML field is + /// optional on read. + pub unique_constraints: Vec>, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/src/persistence/yaml.rs b/src/persistence/yaml.rs index f9a8d12..845613b 100644 --- a/src/persistence/yaml.rs +++ b/src/persistence/yaml.rs @@ -90,6 +90,21 @@ fn write_table(out: &mut String, table: &TableSchema) { for col in &table.columns { write_column(out, col); } + // Composite (multi-column) UNIQUE constraints (ADR-0035 §4a.2) — + // emitted only when present so unconstrained tables stay compact. + if !table.unique_constraints.is_empty() { + let _ = writeln!(out, " unique_constraints:"); + for cols in &table.unique_constraints { + write!(out, " - [").unwrap(); + for (i, c) in cols.iter().enumerate() { + if i > 0 { + out.push_str(", "); + } + out.push_str("e_if_needed(c)); + } + let _ = writeln!(out, "]"); + } + } } /// Always render `s` as a double-quoted YAML string — used @@ -249,6 +264,7 @@ pub(crate) fn parse_schema(body: &str) -> Result { name: t.name, primary_key: t.primary_key, columns, + unique_constraints: t.unique_constraints, }); } let mut relationships: Vec = Vec::with_capacity(raw.relationships.len()); @@ -357,6 +373,10 @@ struct RawTable { name: String, primary_key: Vec, columns: Vec, + /// Composite (multi-column) UNIQUE constraints (ADR-0035 §4a.2). + /// Optional on read — older project files omit it. + #[serde(default)] + unique_constraints: Vec>, } #[derive(Deserialize)] @@ -418,6 +438,7 @@ mod tests { ColumnSchema { name: "id".to_string(), user_type: Type::Serial, unique: false, not_null: false, default: None, check: None }, ColumnSchema { name: "Name".to_string(), user_type: Type::Text, unique: false, not_null: false, default: None, check: None }, ], + unique_constraints: Vec::new(), }, TableSchema { name: "Orders".to_string(), @@ -426,6 +447,7 @@ mod tests { ColumnSchema { name: "id".to_string(), user_type: Type::Serial, unique: false, not_null: false, default: None, check: None }, ColumnSchema { name: "CustId".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, ], + unique_constraints: Vec::new(), }, ], relationships: vec![RelationshipSchema { @@ -494,6 +516,7 @@ mod tests { default: None, check: None, }], + unique_constraints: Vec::new(), }], relationships: vec![], indexes: vec![], @@ -551,6 +574,7 @@ mod tests { check: Some("\"stock\" >= 0".to_string()), }, ], + unique_constraints: Vec::new(), }], relationships: vec![], indexes: vec![], @@ -637,6 +661,7 @@ relationships: ColumnSchema { name: "a".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, ColumnSchema { name: "b".to_string(), user_type: Type::Int, unique: false, not_null: false, default: None, check: None }, ], + unique_constraints: Vec::new(), }], relationships: vec![], indexes: vec![], diff --git a/src/runtime.rs b/src/runtime.rs index 6b45500..909c140 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1924,9 +1924,10 @@ async fn execute_command_typed( name, columns, primary_key, + unique_constraints, if_not_exists, } => database - .sql_create_table(name, columns, primary_key, if_not_exists, src) + .sql_create_table(name, columns, primary_key, unique_constraints, if_not_exists, src) .await .map(|outcome| match outcome { CreateOutcome::Created(d) => CommandOutcome::Schema(Some(d)), @@ -1954,6 +1955,8 @@ async fn execute_command_typed( unique, default, check, + default_sql: None, + check_sql: None, }, src, ) diff --git a/tests/sql_create_table.rs b/tests/sql_create_table.rs index 8885c12..e0fca04 100644 --- a/tests/sql_create_table.rs +++ b/tests/sql_create_table.rs @@ -51,6 +51,7 @@ fn created_table_appears_with_playground_types() { ColumnSpec::new("name", Type::Text), ], vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table Widget (id int primary key, name text)".to_string()), )) @@ -87,6 +88,7 @@ fn integer_primary_key_is_plain_int() { "T".to_string(), vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (id integer primary key)".to_string()), )) @@ -111,6 +113,7 @@ fn serial_pk_autoincrements_in_multi_column_table() { ColumnSpec::new("name", Type::Text), ], vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (id serial primary key, name text)".to_string()), )) @@ -153,6 +156,7 @@ fn if_not_exists_is_a_noop_when_table_exists() { "T".to_string(), specs(), vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (id int)".to_string()), )) @@ -163,6 +167,7 @@ fn if_not_exists_is_a_noop_when_table_exists() { "T".to_string(), specs(), vec!["id".to_string()], + vec![], // no composite UNIQUE true, // IF NOT EXISTS Some("create table if not exists T (id int)".to_string()), )) @@ -188,6 +193,7 @@ fn table_without_primary_key_is_allowed() { "Notes".to_string(), vec![ColumnSpec::new("body", Type::Text)], vec![], // no primary key + vec![], // no composite UNIQUE false, Some("create table Notes (body text)".to_string()), )) @@ -207,6 +213,162 @@ fn table_without_primary_key_is_allowed() { assert_eq!(data.rows.len(), 1); } +/// A column carrying a raw-SQL `CHECK` (ADR-0035 §4a.2). +fn col_check(name: &str, ty: Type, check_sql: &str) -> ColumnSpec { + let mut c = ColumnSpec::new(name, ty); + c.check_sql = Some(check_sql.to_string()); + c +} + +/// A column carrying a raw-SQL `DEFAULT` (ADR-0035 §4a.2). +fn col_default(name: &str, ty: Type, default_sql: &str) -> ColumnSpec { + let mut c = ColumnSpec::new(name, ty); + c.default_sql = Some(default_sql.to_string()); + c +} + +#[test] +fn check_constraint_is_enforced() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ColumnSpec::new("id", Type::Serial), col_check("price", Type::Real, "price >= 0")], + vec!["id".to_string()], + vec![], + false, + Some("create table T (id serial primary key, price real check (price >= 0))".to_string()), + )) + .expect("create"); + // A satisfying row inserts; a violating one is rejected by the CHECK. + r.block_on(db.insert( + "T".to_string(), + Some(vec!["price".to_string()]), + vec![Value::Number("10".to_string())], + Some("insert".to_string()), + )) + .expect("price 10 satisfies the check"); + let bad = r.block_on(db.insert( + "T".to_string(), + Some(vec!["price".to_string()]), + vec![Value::Number("-5".to_string())], + Some("insert".to_string()), + )); + assert!(bad.is_err(), "CHECK (price >= 0) rejects -5"); +} + +#[test] +fn default_is_applied_when_column_omitted() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ + ColumnSpec::new("id", Type::Serial), + ColumnSpec::new("label", Type::Text), + col_default("n", Type::Int, "7"), + ], + vec!["id".to_string()], + vec![], + false, + Some("create table T (id serial primary key, label text, n int default 7)".to_string()), + )) + .expect("create"); + // Insert only `label`; `id` auto-fills and `n` takes its default. + r.block_on(db.insert( + "T".to_string(), + Some(vec!["label".to_string()]), + vec![Value::Text("x".to_string())], + Some("insert".to_string()), + )) + .expect("insert"); + let data = r + .block_on(db.query_data("T".to_string(), None, None, None)) + .expect("query"); + let n_idx = data.columns.iter().position(|c| c == "n").expect("n column"); + assert_eq!(data.rows[0][n_idx].as_deref(), Some("7"), "DEFAULT 7 applied"); +} + +#[test] +fn composite_unique_is_enforced() { + let (_p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ColumnSpec::new("a", Type::Int), ColumnSpec::new("b", Type::Int)], + vec![], + vec![vec!["a".to_string(), "b".to_string()]], + false, + Some("create table T (a int, b int, unique (a, b))".to_string()), + )) + .expect("create"); + let ins = |a: &str, b: &str| { + db.insert( + "T".to_string(), + None, + vec![Value::Number(a.to_string()), Value::Number(b.to_string())], + Some("insert".to_string()), + ) + }; + r.block_on(ins("1", "2")).expect("first (1,2)"); + assert!(r.block_on(ins("1", "2")).is_err(), "UNIQUE(a,b) rejects duplicate (1,2)"); + r.block_on(ins("1", "3")).expect("distinct (1,3) is allowed"); +} + +#[test] +fn check_default_and_composite_unique_survive_rebuild() { + // The part-D round-trip: CHECK (metadata), DEFAULT (PRAGMA), and + // composite UNIQUE (TableSchema + PRAGMA index_list origin 'u') + // must all be reconstructed from project.yaml on rebuild. + let (p, db, _d) = open(false); + let r = rt(); + r.block_on(db.sql_create_table( + "T".to_string(), + vec![ + ColumnSpec::new("a", Type::Int), + ColumnSpec::new("b", Type::Int), + col_check("price", Type::Real, "price >= 0"), + col_default("n", Type::Int, "7"), + ], + vec![], + vec![vec!["a".to_string(), "b".to_string()]], + false, + Some( + "create table T (a int, b int, price real check (price >= 0), \ + n int default 7, unique (a, b))" + .to_string(), + ), + )) + .expect("create"); + + r.block_on(db.rebuild_from_text(p.path().to_path_buf(), None)) + .expect("rebuild"); + + let ins = |a: &str, b: &str, price: &str| { + db.insert( + "T".to_string(), + Some(vec!["a".to_string(), "b".to_string(), "price".to_string()]), + vec![ + Value::Number(a.to_string()), + Value::Number(b.to_string()), + Value::Number(price.to_string()), + ], + Some("insert".to_string()), + ) + }; + // CHECK survived: a negative price is rejected. + assert!(r.block_on(ins("1", "1", "-1")).is_err(), "CHECK survived rebuild"); + // A valid row inserts; DEFAULT n=7 survived. + r.block_on(ins("1", "1", "5")).expect("valid row"); + let data = r + .block_on(db.query_data("T".to_string(), None, None, None)) + .expect("query"); + let n_idx = data.columns.iter().position(|c| c == "n").expect("n column"); + assert_eq!(data.rows[0][n_idx].as_deref(), Some("7"), "DEFAULT survived rebuild"); + // Composite UNIQUE survived: (1,1) again is rejected. + assert!(r.block_on(ins("1", "1", "5")).is_err(), "composite UNIQUE survived rebuild"); +} + #[test] fn if_not_exists_noop_is_journalled() { // A successful no-op is still a submission and belongs in the @@ -218,6 +380,7 @@ fn if_not_exists_noop_is_journalled() { "T".to_string(), vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (id int)".to_string()), )) @@ -228,6 +391,7 @@ fn if_not_exists_noop_is_journalled() { "T".to_string(), vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], + vec![], // no composite UNIQUE true, Some(noop.to_string()), )) @@ -246,6 +410,7 @@ fn plain_create_errors_when_table_exists() { "T".to_string(), specs(), vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (id int)".to_string()), )) @@ -255,6 +420,7 @@ fn plain_create_errors_when_table_exists() { "T".to_string(), specs(), vec!["id".to_string()], + vec![], // no composite UNIQUE false, // no IF NOT EXISTS Some("create table T (id int)".to_string()), )); @@ -269,6 +435,7 @@ fn sql_create_table_is_one_undo_step() { "T".to_string(), vec![ColumnSpec::new("id", Type::Int)], vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (id int)".to_string()), )) @@ -319,6 +486,7 @@ fn serial_pk_first_column_autoincrements_after_rebuild() { ColumnSpec::new("name", Type::Text), ], vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (id serial primary key, name text)".to_string()), )) @@ -350,6 +518,7 @@ fn serial_pk_non_first_column_autoincrements_after_rebuild() { ColumnSpec::new("id", Type::Serial), ], vec!["id".to_string()], + vec![], // no composite UNIQUE false, Some("create table T (name text, id serial primary key)".to_string()), ))