docs: ADR-0035 4a — refine scope (CHECK/DEFAULT to constraint slice; double-precision; serial-inline)
Three design questions settled during 4a implementation (plan + ADR §13 + README in lockstep): - CHECK/DEFAULT defer to the 4a.2 constraint slice: sql_expr is validate-only (no Expr AST), so they need raw-SQL-text storage on a separate path, not do_create_table's Expr->compile reuse. 4a.2 now also covers composite UNIQUE / multi-column table CHECK. - double precision (the lone two-word alias) handled via a keyword-pair branch; single-word aliases + discarded (len) cover the rest. - serial sole-PK in a multi-column table must inline PRIMARY KEY to keep autoincrement (worker-step do_create_table extension). 4a core narrows to columns + types + NOT NULL/UNIQUE/PRIMARY KEY + IF NOT EXISTS; everything else errors "not yet supported".
This commit is contained in:
@@ -54,17 +54,16 @@ From the ADR-0035 design (handoff 36 §3) and the pre-implementation
|
||||
**advanced mode** and produces `Command::SqlCreateTable`.
|
||||
- [ ] Simple-mode `create table T with pk …` is **unchanged** and still
|
||||
parses as `Command::CreateTable` (dispatch fallback verified).
|
||||
- [ ] Column element: `<name> <type> [constraints…]` where constraints
|
||||
are `NOT NULL`, `UNIQUE`, `PRIMARY KEY`, `DEFAULT <expr>`,
|
||||
`CHECK (<expr>)` — the per-column ADR-0029 set spelled in SQL.
|
||||
- [ ] Type slot: ten keywords + the §2.5 alias map; length/precision
|
||||
arg accepted-and-ignored.
|
||||
- [ ] `INTEGER PRIMARY KEY` → plain `int` PK (no auto-increment).
|
||||
- [ ] Column element: `<name> <type> [NOT NULL] [UNIQUE]
|
||||
[PRIMARY KEY]` — the clean-reuse constraints only. `DEFAULT` /
|
||||
`CHECK` are **not** in 4a (→ 4a.2, §6.1).
|
||||
- [ ] Type slot: ten keywords + the §2.5 alias map (incl. the two-word
|
||||
`double precision`, §6.3); length/precision arg accepted-and-ignored.
|
||||
- [ ] `INTEGER PRIMARY KEY` → plain `int` PK (no auto-increment);
|
||||
`serial` PK autoincrements even in a multi-column table (§6.4).
|
||||
- [ ] Table-level `PRIMARY KEY (<col>, …)` — **single and compound**.
|
||||
- [ ] Single-column table-level `UNIQUE(a)` / `CHECK(expr-over-a)`
|
||||
accepted by **normalizing into the column spec**; composite
|
||||
`UNIQUE(a,b)` / multi-column table `CHECK` rejected "not yet
|
||||
supported" (→ 4a.2, §6).
|
||||
- [ ] `DEFAULT`, `CHECK`, and any table-level `UNIQUE`/`CHECK` rejected
|
||||
"not yet supported" (→ 4a.2, §6.1).
|
||||
- [ ] `IF NOT EXISTS` → no-op-with-note on an existing table.
|
||||
- [ ] Structural execution: reuses `do_create_table`; the new object is
|
||||
a first-class playground table — `__rdbms_playground_columns`
|
||||
@@ -126,18 +125,17 @@ Mirror `src/dsl/grammar/sql_insert.rs`:
|
||||
- After `table` keyword: an optional `IF NOT EXISTS` keyword run that
|
||||
sets a flag in the builder.
|
||||
- Column list: a `Repeated` of column-or-table elements inside `( … )`.
|
||||
- Column element: name slot (`IdentSource::NewName`) + type slot
|
||||
(`IdentSource::Types`) + a constraint walk. The type slot consumes
|
||||
an optional parenthesised number/number-pair and discards it
|
||||
(length/precision ignored).
|
||||
- Table element (4a): `PRIMARY KEY ( col, … )`. **No** `FOREIGN KEY`,
|
||||
no inline `REFERENCES` in 4a (those parse-error for now; 4b adds
|
||||
them).
|
||||
- `CHECK (<expr>)` / `DEFAULT <expr>` reuse the ADR-0031 fragment via
|
||||
`Node::Subgrammar(&sql_expr::SQL_OR_EXPR)` (the same mechanism
|
||||
`sql_insert.rs` uses for value expressions). The matched expression
|
||||
is compiled to storable SQL via the existing `compile_check_sql`
|
||||
path used by `do_create_table`.
|
||||
- Column element: name slot (`IdentSource::NewName`) + type slot + a
|
||||
constraint walk admitting only `NOT NULL` / `UNIQUE` /
|
||||
`PRIMARY KEY` (column-level). `DEFAULT` / `CHECK` are **not** in 4a
|
||||
(→ 4a.2); typing them is "not yet supported".
|
||||
- Type slot: `Choice[ Seq[Word("double"), Word("precision")],
|
||||
Seq[ Ident{source: Types, validator: SQL_TYPE_VALIDATOR},
|
||||
Optional<( number [, number] )> ] ]` (§6.3). The validator uses
|
||||
`Type::from_sql_name`; the length arg is discarded.
|
||||
- Table element (4a): `PRIMARY KEY ( col, … )` only. **No**
|
||||
`FOREIGN KEY` / inline `REFERENCES` (4b), **no** table-level
|
||||
`UNIQUE`/`CHECK` (4a.2) — those parse-error / "not yet supported".
|
||||
|
||||
Register in `REGISTRY` (`src/dsl/grammar/mod.rs`, Advanced group):
|
||||
`(&data::SQL_CREATE_TABLE, CommandCategory::Advanced)` alongside the
|
||||
@@ -233,53 +231,86 @@ enforces it.
|
||||
- `DROP TABLE [IF EXISTS]` → 4c. (`IF EXISTS` no-op-with-note reuses the
|
||||
4.4 mechanism.)
|
||||
- Indexes, `ALTER TABLE`, `ALTER COLUMN TYPE`, table rename → 4d–4h.
|
||||
- **Composite `UNIQUE(a,b)` / multi-column table `CHECK` → 4a.2** (a
|
||||
persistence-model extension; see §6). 4a accepts **single-column**
|
||||
table-level `UNIQUE(a)` / `CHECK(expr-over-one-col)` by normalizing
|
||||
them into the column spec (free — `ColumnSchema` already carries
|
||||
per-column `unique`/`check`), and rejects the composite/multi-column
|
||||
forms with a "not yet supported" message until 4a.2 lands.
|
||||
- **`CHECK`, `DEFAULT`, and all table-level `UNIQUE`/`CHECK` → 4a.2**
|
||||
(the constraint slice; see §6.1). 4a rejects them with a
|
||||
"not yet supported" message. 4a keeps only the constraints that are a
|
||||
clean `do_create_table` reuse: column-level `NOT NULL`, `UNIQUE`, and
|
||||
`PRIMARY KEY`, plus table-level `PRIMARY KEY (cols)`.
|
||||
- `CREATE TABLE … AS SELECT` (CTAS): **not in ADR-0035's surface** at
|
||||
all — an ordinary parse error, not a deferral.
|
||||
|
||||
## 6. Decisions settled this round (user-confirmed) + the 4a/4a.2 split
|
||||
## 6. Decisions settled this round (all user-confirmed unless noted)
|
||||
|
||||
1. **4a/4a.2 split (user-confirmed).** Composite `UNIQUE(a,b)` and
|
||||
multi-column table `CHECK` are **deferred to a dedicated slice
|
||||
4a.2**. Rationale (the general rule): a DDL feature needs
|
||||
data-model work exactly when it introduces a **structure simple
|
||||
mode could never produce** — not merely new syntax. Almost all of
|
||||
advanced DDL (per-column constraints, compound PK, FK, indexes,
|
||||
drop, alter-column) maps onto structures the model already persists
|
||||
(`ColumnSchema`, `TableSchema.primary_key`, `RelationshipSchema`,
|
||||
`IndexSchema`), so it is syntax-only + reuse. Composite UNIQUE /
|
||||
table CHECK are the **first structures with no slot in
|
||||
`TableSchema`**, so they need a model + round-trip extension and
|
||||
earn their own slice. **Same class, already on the radar:**
|
||||
`CREATE UNIQUE INDEX` (4d — `IndexSchema` has no `unique` field;
|
||||
ADR-0025 deferred it) and table rename (4h — a new low-level op the
|
||||
ADR already flags).
|
||||
**The general rule (the litmus test).** A DDL feature needs data-model
|
||||
or new-execution work exactly when it introduces a **structure simple
|
||||
mode could never produce, or an expression representation the structural
|
||||
helper can't consume** — not merely new syntax. Almost all of advanced
|
||||
DDL (per-column NOT NULL/UNIQUE, compound PK, FK, indexes, drop,
|
||||
alter-column) maps onto structures the model already persists
|
||||
(`ColumnSchema`, `TableSchema.primary_key`, `RelationshipSchema`,
|
||||
`IndexSchema`) and feeds the existing helpers, so it is syntax-only +
|
||||
reuse. The exceptions earn their own slice. **Same class, already on the
|
||||
radar:** `CREATE UNIQUE INDEX` (4d — `IndexSchema` has no `unique`
|
||||
field; ADR-0025 deferred it) and table rename (4h — a new low-level op
|
||||
the ADR flags).
|
||||
|
||||
**4a.2 scope** (its own test-first slice, after 4a core):
|
||||
- Extend `TableSchema` (`src/persistence/mod.rs`) with table-level
|
||||
constraint slots — e.g. `unique_constraints: Vec<Vec<String>>` and
|
||||
`check_constraints: Vec<String>`.
|
||||
- Extend the YAML writer/parser + `RawTable`
|
||||
(`src/persistence/yaml.rs`) — additive, backward-compatible,
|
||||
optional-on-read (the pattern used when `unique`/`not_null`/
|
||||
`check` were added; **no migration needed**).
|
||||
- Extend `read_schema_snapshot` (`src/db.rs` ~2225) to **detect**
|
||||
composite UNIQUE / table CHECK from the database.
|
||||
- Extend `do_create_table` to **emit** them in the DDL.
|
||||
- Extend the `SqlCreateTable` command shape + grammar to carry/parse
|
||||
them (lifting the 4a "not yet supported" rejection).
|
||||
- **Round-trip tests** (save → load → rebuild) proving they survive,
|
||||
on top of parse/execute/undo coverage.
|
||||
1. **The constraint slice 4a.2 (user-confirmed).** A dedicated
|
||||
test-first slice, after 4a core, gathering every constraint that is
|
||||
*not* a clean reuse:
|
||||
- **`CHECK` / `DEFAULT`** — via the full ADR-0031 `sql_expr` surface,
|
||||
captured as **raw SQL text** and stored directly (`ColumnSchema.
|
||||
check` / `.default` are already `String`/`Option<String>`). Needed
|
||||
because `sql_expr` is **validate-only** — it builds no `Expr` AST,
|
||||
so it cannot feed `compile_check_sql(expr: &Expr)` /
|
||||
`ColumnSpec.check: Option<Expr>`. This is a **separate execution
|
||||
path** (raw-text, not the `Expr`→compile path), threaded through
|
||||
`SqlCreateTable` + a `do_create_table` variant — hence its own
|
||||
slice rather than 4a.
|
||||
- **Composite `UNIQUE(a,b)` and multi-column table `CHECK`** — the
|
||||
first structures with no slot in `TableSchema`; need the
|
||||
model + round-trip extension: extend `TableSchema`
|
||||
(`src/persistence/mod.rs`, e.g. `unique_constraints:
|
||||
Vec<Vec<String>>`, `check_constraints: Vec<String>`); extend the
|
||||
YAML writer/parser + `RawTable` (`src/persistence/yaml.rs`,
|
||||
additive/backward-compatible/optional-on-read — the
|
||||
`unique`/`not_null`/`check` pattern, **no migration**); extend
|
||||
`read_schema_snapshot` (`src/db.rs` ~2225) to **detect** them;
|
||||
extend `do_create_table` to **emit** them; **round-trip tests**
|
||||
(save → load → rebuild).
|
||||
- Until 4a.2: `CREATE TABLE` with `CHECK`, `DEFAULT`, or any
|
||||
table-level `UNIQUE`/`CHECK` errors **"not yet supported"** (same
|
||||
treatment as composite UNIQUE) — not a confusing parse error.
|
||||
|
||||
2. **No-op-with-note plumbing — mechanism (a)** (the `CreateOutcome`
|
||||
enum, §4.4). Explicit and reused by `DROP TABLE IF EXISTS` (4c);
|
||||
chosen now so the worker reply type is designed once.
|
||||
|
||||
3. **`double precision` (implementer call).** The lone two-word alias is
|
||||
handled by a dedicated keyword-pair branch in the type slot
|
||||
(`Choice[ Seq[Word("double"), Word("precision")], <type-ident +
|
||||
optional (len)> ]`); the builder maps the word-pair to `Type::Real`.
|
||||
Single-word aliases + an optional discarded `(len[,len])` cover the
|
||||
rest. Delivers the ADR §3 item without bending it.
|
||||
|
||||
4. **`serial` PK inline emission (implementer call, step 3/worker).**
|
||||
`do_create_table` today inlines `PRIMARY KEY` (the rowid-alias that
|
||||
makes `serial` autoincrement) **only when the table has one column**.
|
||||
SQL mode allows `CREATE TABLE t (id serial primary key, name text)` —
|
||||
a `serial` sole-PK in a multi-column table — which would otherwise
|
||||
get a table-level PK and **lose autoincrement**. Step 3 extends the
|
||||
inline condition to "sole-PK column whose type is `serial` → inline
|
||||
`PRIMARY KEY` on that column," leaving the simple-mode paths
|
||||
(single-column, or compound) unchanged. Covered by a worker test.
|
||||
|
||||
5. **Redundant PK constraints (implementer call).** SQL mode is
|
||||
**lenient** like real engines: `id int primary key not null` /
|
||||
`… unique` is accepted, and the builder silently de-dups the
|
||||
redundant `not_null`/`unique` flag off a sole-PK column (so the
|
||||
emitted DDL/metadata stay clean for `do_create_table`). Simple
|
||||
mode's ADR-0029 §9 *rejection* is unchanged. (Diverges from simple
|
||||
mode deliberately, matching the advanced-mode "trust the user like
|
||||
SQL" posture, ADR-0035 §7.)
|
||||
|
||||
## 7. Devil's Advocate review of this plan
|
||||
|
||||
- **Does it reuse rather than fork execution?** Yes — `do_create_table`
|
||||
@@ -307,18 +338,22 @@ enforces it.
|
||||
(canonical + aliases + length-ignored + unknown→None) → red → add
|
||||
`resolve_type_name` → green.
|
||||
2. **Command + parser** — write Tier-1 `ast_builder` tests (valid
|
||||
columns/types/constraints, compound PK, single-column table-level
|
||||
`UNIQUE`/`CHECK` normalized, `INTEGER PRIMARY KEY`→int,
|
||||
`IF NOT EXISTS` flag; FK-shape and composite `UNIQUE(a,b)` /
|
||||
multi-column `CHECK` rejected "not yet supported") → red → add
|
||||
`Command::SqlCreateTable`, `sql_create_table.rs`, REGISTRY entry →
|
||||
columns/types incl. aliases + `double precision` + discarded
|
||||
`(len)`, column-level `NOT NULL`/`UNIQUE`/`PRIMARY KEY`, compound
|
||||
table-level PK, `INTEGER PRIMARY KEY`→int, `IF NOT EXISTS` flag;
|
||||
`DEFAULT`/`CHECK`/table-level `UNIQUE`/FK-shape rejected "not yet
|
||||
supported") → red → add `Command::SqlCreateTable`,
|
||||
`sql_create_table.rs`, REGISTRY entry, exhaustive-match stubs →
|
||||
green. Include a fallback test: simple `create … with pk` still
|
||||
parses in advanced mode.
|
||||
parses in advanced mode. *(Compiles only once the worker dispatch
|
||||
handles the new `Command`, so steps 2–3 land together.)*
|
||||
3. **Worker** — write Tier-3 `tests/sql_create_table.rs` (metadata, CSV,
|
||||
describe; alias end-to-end; `IF NOT EXISTS` no-op-with-note;
|
||||
duplicate-table plain error) → red → add `Request::SqlCreateTable` +
|
||||
`snapshot_then` arm + the §4.4 outcome + reuse `do_create_table` →
|
||||
green.
|
||||
describe; alias end-to-end; `serial` PK autoincrements in a
|
||||
multi-column table, §6.4; `IF NOT EXISTS` no-op-with-note;
|
||||
duplicate-table plain error; redundant PK constraint de-duped, §6.5)
|
||||
→ red → add `Request::SqlCreateTable` + `snapshot_then` arm + the
|
||||
§4.4 `CreateOutcome` + the §6.4 `do_create_table` serial-inline
|
||||
extension + reuse `do_create_table` → green.
|
||||
4. **Undo** — write the undo Tier-3 test (one step; undo/redo) → red →
|
||||
confirm it passes (the `snapshot_then` wrap should make it green with
|
||||
no extra code; if not, the wrap is wrong) → green.
|
||||
|
||||
Reference in New Issue
Block a user