docs: handoff 22 — ADR-0029 through commit 4
ADR-0028 complete (per handoff-21); ADR-0029 (column constraints) written, accepted, and implemented through commit 4 of 6 — NOT NULL / UNIQUE / DEFAULT / CHECK at `create table` and `add column`. Commits 5 (`add constraint` / `drop constraint` + the §5 dry-run) and 6 (friendly errors + typing-surface matrix) are planned in full in §4.
This commit is contained in:
claude@clouddev1
@@ -0,0 +1,259 @@
|
|||||||
|
# Session handoff — 2026-05-19 (22)
|
||||||
|
|
||||||
|
Twenty-second handover. **Interim** handoff: this session
|
||||||
|
finished **ADR-0028** (handed off in handoff-21), then started
|
||||||
|
**ADR-0029** — column constraints (`NOT NULL` / `UNIQUE` /
|
||||||
|
`CHECK` / `DEFAULT`). ADR-0029 is **written, accepted, and
|
||||||
|
implemented through commit 4 of 6**; commits 5–6 are planned
|
||||||
|
in full detail in §4 and not yet built. A fresh session can
|
||||||
|
implement them from this file + the ADR.
|
||||||
|
|
||||||
|
## §1. State at handoff
|
||||||
|
|
||||||
|
**Branch:** `main`. Working tree clean. **8 commits** since
|
||||||
|
handoff-21 (`02234e6`), all local — push asynchronously, not
|
||||||
|
blocking.
|
||||||
|
|
||||||
|
```
|
||||||
|
<this file> docs: handoff 22 — ADR-0029 through commit 4
|
||||||
|
942222b constraints: CHECK — check (<expr>) at create/add (ADR-0029)
|
||||||
|
58d8958 add column: column constraints — NOT NULL/UNIQUE/DEFAULT (§6)
|
||||||
|
12395a9 create table: column constraints — NOT NULL/UNIQUE/DEFAULT grammar
|
||||||
|
a60e879 db: column-constraint infrastructure — NOT NULL/UNIQUE/DEFAULT
|
||||||
|
eff2ee8 refactor: ColumnSpec / AddColumn carry constraint fields (scaffolding)
|
||||||
|
7bfd213 docs: ADR-0029 — column constraints
|
||||||
|
59351e0 docs: move indexes/query-plans out of the deferred list
|
||||||
|
02234e6 docs: handoff 21 — ADR-0028 complete
|
||||||
|
```
|
||||||
|
|
||||||
|
**Tests:** **1201 passing, 0 failing, 1 ignored** (`cargo
|
||||||
|
test`). The ignored test is the long-standing `` ```ignore ``
|
||||||
|
doc-test in `src/friendly/mod.rs`. Typing-surface matrix:
|
||||||
|
**174 cells**, unchanged this session.
|
||||||
|
|
||||||
|
**Clippy:** clean (`cargo clippy --all-targets -- -D
|
||||||
|
warnings`, nursery group).
|
||||||
|
|
||||||
|
## §2. Done before ADR-0029
|
||||||
|
|
||||||
|
- **ADR-0028 (query plans / `explain`)** — finished and fully
|
||||||
|
documented in **handoff-21**. Read that for context.
|
||||||
|
- **`docs: move indexes/query-plans out of the deferred
|
||||||
|
list`** (`59351e0`) — a CLAUDE.md tidy-up only.
|
||||||
|
|
||||||
|
## §3. ADR-0029 — what it is, what's done
|
||||||
|
|
||||||
|
Read **`docs/adr/0029-column-constraints.md`** — it is the
|
||||||
|
spec, and it has been kept current (the two deviations below
|
||||||
|
are already folded in). The feature: the four column-level
|
||||||
|
constraints, declared in a suffix after a column's `(type)`
|
||||||
|
group and — for commit 5 — modifiable on existing columns.
|
||||||
|
|
||||||
|
**Commits 1–4 (done):**
|
||||||
|
|
||||||
|
1. `eff2ee8` — `ColumnSpec` / `Command::AddColumn` gained the
|
||||||
|
four constraint fields (`not_null` / `unique` /
|
||||||
|
`default: Option<Value>` / `check: Option<Expr>`), all
|
||||||
|
defaulting off; `Database::add_column` now takes a
|
||||||
|
`ColumnSpec`. `ColumnSpec::new(name, ty)` is the
|
||||||
|
unconstrained constructor. 110 call sites updated.
|
||||||
|
2. `a60e879` — the db layer honours the `ColumnSpec`
|
||||||
|
constraints: `column_constraints_sql`, `do_create_table`
|
||||||
|
emits `NOT NULL`/`UNIQUE`/`DEFAULT`; `ReadColumn` gained
|
||||||
|
`default_sql`; `schema_to_ddl` emits `DEFAULT` (so the
|
||||||
|
rebuild primitive preserves it); `ColumnDescription` +
|
||||||
|
`do_describe_table` (now sourced from `read_schema`) +
|
||||||
|
`constraints_display`.
|
||||||
|
3. `12395a9` — the `create table` constraint-suffix grammar
|
||||||
|
(`not null` / `unique` / `default <literal>`);
|
||||||
|
`build_create_table` collects per-column constraints; §9
|
||||||
|
redundancy check; `project.yaml` round-trip
|
||||||
|
(`ColumnSchema` gained `not_null` / `default`).
|
||||||
|
4. `58d8958` — `add column` with the same suffix; the §6
|
||||||
|
routing (`unique` / `not_null`-without-default route
|
||||||
|
through the rebuild primitive via
|
||||||
|
`do_add_constrained_column_via_rebuild`); §6 pre-flight
|
||||||
|
refusals.
|
||||||
|
5. `942222b` — `CHECK`. `check ( <expr> )` reuses the
|
||||||
|
ADR-0026 expression grammar via `Subgrammar`. The parsed
|
||||||
|
`Expr` is compiled once to inline SQL (`compile_check_sql`)
|
||||||
|
and stored in that form everywhere — a `check_expr` column
|
||||||
|
in `__rdbms_playground_columns`, `ColumnSchema.check`, the
|
||||||
|
column DDL.
|
||||||
|
|
||||||
|
### Three decisions / discoveries this session (the ADR
|
||||||
|
### already reflects all three)
|
||||||
|
|
||||||
|
1. **`create table … with pk …` makes *every* listed column
|
||||||
|
a primary-key column** — there is no simple-mode syntax
|
||||||
|
for a non-PK column at create time (recorded in
|
||||||
|
`docs/simple-mode-limitations.md`). Consequence: the
|
||||||
|
`create table` constraint suffix is realistically only
|
||||||
|
useful for `default` / `check`; `not null` / `unique`
|
||||||
|
there are §9 redundancy errors. The real home for
|
||||||
|
`not null` / `unique` is `add column` and (commit 5)
|
||||||
|
`add constraint`.
|
||||||
|
2. **`CHECK` is stored as compiled SQL, not DSL text**
|
||||||
|
(user-ratified; ADR §7/§8 updated). Double-quoted
|
||||||
|
identifiers, consistent with ADR-0028's `explain` display
|
||||||
|
SQL. No `Expr`→text renderer, no re-parser.
|
||||||
|
3. **A `CHECK` on a `serial`/`shortid` column is
|
||||||
|
create-table-only** — `add column … (serial) … check` is
|
||||||
|
refused with a friendly message (the auto-fill rebuild
|
||||||
|
path does not thread it).
|
||||||
|
|
||||||
|
### Key reusable pieces (commits 1–4 built these)
|
||||||
|
|
||||||
|
All in `src/dsl/grammar/ddl.rs` unless noted:
|
||||||
|
|
||||||
|
- `COLUMN_CONSTRAINT` — the `Choice` over the four individual
|
||||||
|
constraint nodes (`NOT_NULL_CONSTRAINT`, `UNIQUE_CONSTRAINT`,
|
||||||
|
`DEFAULT_CONSTRAINT`, `CHECK_CONSTRAINT`).
|
||||||
|
`COLUMN_CONSTRAINT_SUFFIX` is `Repeated{min:0}` of it. The
|
||||||
|
`add constraint` command (commit 5) reuses the individual
|
||||||
|
nodes.
|
||||||
|
- `collect_column_constraints(path)` → `(bool, bool,
|
||||||
|
Option<Value>, Option<Expr>)` — scans a single-column
|
||||||
|
path's constraint keywords. `consume_check_expr` — extracts
|
||||||
|
a `check ( … )` expression (paren-depth aware).
|
||||||
|
- `redundant_pk_constraint(column, constraint)` — the
|
||||||
|
ADR-0029 §9 friendly error.
|
||||||
|
- `src/db.rs`: `column_constraints_sql`, `default_sql_literal`,
|
||||||
|
`compile_check_sql`, `insert_column_metadata`,
|
||||||
|
`read_schema_for_specs`, `do_add_constrained_column_via_rebuild`.
|
||||||
|
`ReadColumn` carries `notnull` / `unique` / `default_sql` /
|
||||||
|
`check`; `schema_to_ddl` emits all four — **so the rebuild
|
||||||
|
primitive already round-trips every constraint.**
|
||||||
|
|
||||||
|
## §4. ADR-0029 commits 5–6 — the plan
|
||||||
|
|
||||||
|
### Commit 5 — `add constraint` / `drop constraint` (ADR §2.2, §5)
|
||||||
|
|
||||||
|
The surface (the user chose `add constraint <kind>` over a
|
||||||
|
bare `add <kind>`, for grammar-hierarchy uniformity):
|
||||||
|
|
||||||
|
```
|
||||||
|
add constraint not null to <T>.<col>
|
||||||
|
add constraint unique to <T>.<col>
|
||||||
|
add constraint default <literal> to <T>.<col>
|
||||||
|
add constraint check ( <expr> ) to <T>.<col>
|
||||||
|
drop constraint not null from <T>.<col>
|
||||||
|
drop constraint unique from <T>.<col>
|
||||||
|
drop constraint default from <T>.<col>
|
||||||
|
drop constraint check from <T>.<col>
|
||||||
|
```
|
||||||
|
|
||||||
|
**Grammar (`src/dsl/grammar/ddl.rs`):** add a `constraint`
|
||||||
|
form to the `ADD` and `DROP` command `Choice`s. The word
|
||||||
|
`constraint` after `add` / `drop` discriminates it (distinct
|
||||||
|
from `column` / `index` / `table` / `relationship` / `1`).
|
||||||
|
After `add constraint`, reuse the existing `COLUMN_CONSTRAINT`
|
||||||
|
Choice for the `<constraint>`; after `drop constraint`, a
|
||||||
|
`Choice` of `not null` / `unique` / `default` / `check`
|
||||||
|
keyword-only nodes (no payload). The dotted `<T>.<col>` —
|
||||||
|
reuse the `Ident '.' Ident` shape from `add 1:n relationship
|
||||||
|
from <P>.<col>` (same file). `build_add` / `build_drop`
|
||||||
|
discriminate forms by the word after the entry verb — add a
|
||||||
|
`Some("constraint")` arm to each.
|
||||||
|
|
||||||
|
**AST (`src/dsl/command.rs`):**
|
||||||
|
```rust
|
||||||
|
Command::AddConstraint { table: String, column: String, constraint: Constraint }
|
||||||
|
Command::DropConstraint { table: String, column: String, kind: ConstraintKind }
|
||||||
|
pub enum Constraint { NotNull, Unique, Default(Value), Check(Expr) }
|
||||||
|
pub enum ConstraintKind { NotNull, Unique, Default, Check }
|
||||||
|
```
|
||||||
|
|
||||||
|
**GOTCHA — the two new `Command` variants break every
|
||||||
|
exhaustive `match Command`** (exactly ADR-0028's gotcha 3).
|
||||||
|
Add arms to: `Command::verb()` and `target_table()`
|
||||||
|
(`command.rs`); `execute_command_typed` (`runtime.rs`);
|
||||||
|
`build_translate_context` (`app.rs` — map to an `Operation`,
|
||||||
|
e.g. a new `Operation::AddConstraint` or reuse an existing
|
||||||
|
one); `command_kind_label` (`tests/typing_surface/mod.rs`).
|
||||||
|
Land grammar + AST + worker in **one commit** (a green
|
||||||
|
intermediate is impossible otherwise).
|
||||||
|
|
||||||
|
**DB worker (`src/db.rs`):** `Request::AddConstraint` /
|
||||||
|
`DropConstraint`; `Database::add_constraint` /
|
||||||
|
`drop_constraint`; `do_add_constraint` / `do_drop_constraint`.
|
||||||
|
Both return `TableDescription` (the post-rebuild structure) →
|
||||||
|
route as `CommandOutcome::Schema(Some(desc))` →
|
||||||
|
`AppEvent::DslSucceeded` (the existing auto-show path; **no
|
||||||
|
new AppEvent needed**).
|
||||||
|
|
||||||
|
`do_add_constraint` steps:
|
||||||
|
1. `read_schema(conn, table)`; find the target column.
|
||||||
|
2. **§9 redundancy** (execution-time, since the parser has no
|
||||||
|
schema): if the column is a PK column, reject `not null`
|
||||||
|
(always) and `unique` (single-column PK only) — see ADR §9.
|
||||||
|
3. **§5 dry-run** for `not null` / `unique` / `check` (skip
|
||||||
|
for `default`): scan the existing rows; refuse with a
|
||||||
|
learner-friendly pretty-table of offending rows if any.
|
||||||
|
*Mirror how `do_change_column_type` surfaces its ADR-0017
|
||||||
|
dry-run refusal table* — that is the existing precedent
|
||||||
|
for "scan data, refuse with a table." `add not null` →
|
||||||
|
rows `WHERE col IS NULL`; `add unique` → non-NULL
|
||||||
|
duplicate groups; `add check` → rows `WHERE NOT (<sql>)`.
|
||||||
|
4. Build `new_schema` = the read schema with the target
|
||||||
|
`ReadColumn`'s field set (`notnull` / `unique` /
|
||||||
|
`default_sql` via `default_sql_literal` / `check` via
|
||||||
|
`compile_check_sql`), then `rebuild_table(...)`. The
|
||||||
|
`metadata_updates` closure updates the `check_expr`
|
||||||
|
metadata column when the constraint is a `CHECK` (the
|
||||||
|
other three are pragma-recoverable and need no metadata).
|
||||||
|
|
||||||
|
`do_drop_constraint`: §9 check (cannot drop a PK-implied
|
||||||
|
`not null`, or `unique` from a single-column PK — friendly
|
||||||
|
error); build `new_schema` with the field cleared; rebuild.
|
||||||
|
No dry-run (removing a constraint cannot violate data).
|
||||||
|
`drop check` also NULLs the `check_expr` metadata.
|
||||||
|
|
||||||
|
**Catalog:** `parse.usage.add_constraint` /
|
||||||
|
`parse.usage.drop_constraint` in `en-US.yaml` *and*
|
||||||
|
`keys.rs` (the `keys_validate_against_catalog` test enforces
|
||||||
|
the pair). The §5 refusal messages can be `DbError::Unsupported`
|
||||||
|
detail strings for now (consistent with §6's commit-4
|
||||||
|
messages) or dedicated catalog keys.
|
||||||
|
|
||||||
|
### Commit 6 — friendly errors + matrix + verification
|
||||||
|
|
||||||
|
- **CHECK-violation friendly error** (ADR §10): an `insert` /
|
||||||
|
`update` that violates a `CHECK` makes SQLite report
|
||||||
|
`CHECK constraint failed`; add a `friendly`-layer catalog
|
||||||
|
entry translating it (`NOT NULL` / `UNIQUE` are already
|
||||||
|
enriched — see `runtime.rs` `enrich_dsl_failure`).
|
||||||
|
- **Typing-surface matrix cells** in `tests/typing_surface/`
|
||||||
|
for the constraint grammar — `create table … not null`,
|
||||||
|
`add column … check (…)`, `add constraint … to T.col`,
|
||||||
|
etc. Follow the matrix-snapshot discipline (handoff-17→21):
|
||||||
|
a new cell's snapshot is created with
|
||||||
|
`INSTA_UPDATE=always cargo test --test typing_surface_matrix
|
||||||
|
<filter>` then reviewed before committing (`cargo insta` is
|
||||||
|
not installed on this machine).
|
||||||
|
- Full suite green vs. the 1201 baseline; clippy clean.
|
||||||
|
- Final handoff (handoff-23) and tick `C3` in
|
||||||
|
`docs/requirements.md`.
|
||||||
|
|
||||||
|
## §5. How to take over
|
||||||
|
|
||||||
|
1. **Read this file, then `docs/adr/0029-column-constraints.md`**
|
||||||
|
(the spec — current), then **handoff-21** (ADR-0028).
|
||||||
|
2. **Read `CLAUDE.md`** — working-style rules.
|
||||||
|
3. **Run `cargo test`** — 1201 passing, 0 failing, 1 ignored.
|
||||||
|
4. **Run `cargo clippy --all-targets -- -D warnings`** — clean.
|
||||||
|
5. **Implement ADR-0029 commit 5** per §4 — grammar + AST +
|
||||||
|
worker in one commit (the `match Command` breakage forces
|
||||||
|
it). Then commit 6. The §4 anchors and gotchas are this
|
||||||
|
session's; the dry-run precedent to copy is
|
||||||
|
`do_change_column_type`.
|
||||||
|
|
||||||
|
### Note on mechanical churn
|
||||||
|
|
||||||
|
Adding a field to `ColumnSpec` / `ColumnDescription` /
|
||||||
|
`ReadColumn` / `ColumnSchema` breaks 20–110 construction
|
||||||
|
sites, almost all in test code. This session delegated those
|
||||||
|
purely-mechanical sweeps to a general-purpose sub-agent
|
||||||
|
(`mode: bypassPermissions`) twice — it is the right tool for
|
||||||
|
that drudgery. Commit 5 adds no such field, so it should not
|
||||||
|
recur; but if commit 6 does, delegate it.
|
||||||
Reference in New Issue
Block a user