a049ff9aa0
ADR-0029 (column constraints — NOT NULL / UNIQUE / CHECK / DEFAULT) is fully implemented across the handoff-22 and handoff-23 sessions. Ticks requirement C3, and corrects ADR §10's CHECK-error wording to the compiled-SQL form per the §7 storage deviation.
171 lines
7.4 KiB
Markdown
171 lines
7.4 KiB
Markdown
# Session handoff — 2026-05-19 (23)
|
||
|
||
Twenty-third handover. This session **finished ADR-0029** —
|
||
column constraints (`NOT NULL` / `UNIQUE` / `CHECK` /
|
||
`DEFAULT`). Commits 5–6, planned in full in handoff-22 §4, are
|
||
implemented, tested and committed. **ADR-0029 is complete**;
|
||
requirement `C3` (schema constraints) is now ticked in
|
||
`docs/requirements.md`.
|
||
|
||
## §1. State at handoff
|
||
|
||
**Branch:** `main`. Working tree clean (after this docs
|
||
commit). **3 commits** since handoff-22 (`102dff0`) — two
|
||
feature commits plus this handoff — all local; push
|
||
asynchronously, not blocking.
|
||
|
||
```
|
||
<this file> docs: handoff 23 — ADR-0029 complete; tick C3
|
||
5e97f6a constraints: CHECK-violation friendly error + typing-surface matrix (ADR-0029 §10)
|
||
abce118 constraints: `add constraint` / `drop constraint` on existing columns (ADR-0029 §2.2)
|
||
```
|
||
|
||
**Tests:** **1240 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:
|
||
**188 cells** (was 174 — +14 for the constraint grammar).
|
||
|
||
**Clippy:** clean (`cargo clippy --all-targets -- -D
|
||
warnings`, nursery group).
|
||
|
||
## §2. What ADR-0029 delivered (full picture)
|
||
|
||
Commits 1–4 (handoff-22) landed the constraint *suffix* —
|
||
`not null` / `unique` / `default <lit>` / `check (<expr>)`
|
||
after a column's `(type)` group on `create table` and `add
|
||
column`, the db-layer DDL/round-trip, and the §6 `add column`
|
||
routing. This session added the rest:
|
||
|
||
**Commit 5 (`abce118`) — `add constraint` / `drop constraint`.**
|
||
The §2.2 surface for modifying an *existing* column's
|
||
constraints:
|
||
|
||
```
|
||
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 | unique | default | check) from <T>.<col>
|
||
```
|
||
|
||
- **Grammar** (`src/dsl/grammar/ddl.rs`): `ADD_CONSTRAINT` /
|
||
`DROP_CONSTRAINT` join the `add` / `drop` `Choice`s,
|
||
discriminated by the `constraint` form word. `add
|
||
constraint` reuses the §2.1 `COLUMN_CONSTRAINT` Choice;
|
||
`drop constraint` uses a payload-free `DROP_CONSTRAINT_KIND`
|
||
Choice. `CONSTRAINT_TARGET` is the dotted `<T>.<col>`
|
||
(`table_name` / `column_name` roles — distinct from the
|
||
CHECK expression's `expr_column`, so the target column
|
||
never collides with an expression column).
|
||
- **AST** (`src/dsl/command.rs`): `Command::AddConstraint` /
|
||
`DropConstraint`; `Constraint` (payload-carrying) /
|
||
`ConstraintKind` (payload-free) enums.
|
||
- **Worker** (`src/db.rs`): `do_add_constraint` /
|
||
`do_drop_constraint` apply the change through the
|
||
rebuild-table primitive. `do_add_constraint` runs the §5
|
||
**dry-run first** — `dry_run_not_null` / `dry_run_unique` /
|
||
`dry_run_check` scan the existing rows and, on a violation,
|
||
refuse *before any write* with a pretty-printed table of
|
||
offending rows (the `db.diagnostic.add_*_summary` catalog
|
||
keys + `render_diagnostic_table`, mirroring
|
||
`do_change_column_type`). §9 redundant-on-PK declarations
|
||
and §6 `default` on a `serial` / `shortid` column are
|
||
friendly refusals.
|
||
- Both return `TableDescription` → `CommandOutcome::Schema` →
|
||
the existing auto-show path (no new `AppEvent`).
|
||
|
||
**Commit 6 (`5e97f6a`) — CHECK friendly error + matrix.**
|
||
|
||
- **CHECK-violation friendly error (ADR §10).** The friendly
|
||
layer already had a `translate_check` + `error.check.*`
|
||
catalog entries (placeholder from an earlier session), but
|
||
nothing filled the column in. `enrich_dsl_failure`
|
||
(`src/runtime.rs`) gained a CHECK branch:
|
||
`enrich_check_violation` reads the column from the engine's
|
||
`CHECK constraint failed: <column>` message, then resolves
|
||
the table, the offending value, and the column's compiled
|
||
`CHECK` expression. `FailureContext` / `TranslateContext`
|
||
carry a new `check_rule` field; `translate_check` renders
|
||
"the value `<v>` breaks the rule `<rule>`" when the rule is
|
||
known, falling back to the plain hint otherwise.
|
||
- **Typing-surface matrix.** New `tests/typing_surface/
|
||
constraints.rs` — 14 cells covering the create-table /
|
||
add-column constraint suffix and `add` / `drop constraint`.
|
||
|
||
## §3. Decisions / deviations this session
|
||
|
||
1. **`schema_to_ddl` UNIQUE bug fixed** (`abce118`). It
|
||
suppressed `UNIQUE` for *every* PK column. A *compound*-PK
|
||
member is **not** individually unique, so an explicit
|
||
`UNIQUE` on it must survive the rebuild. The condition is
|
||
now "suppress only for a *single-column* PK". Caught by the
|
||
`add_constraint_unique_on_compound_pk_member_is_allowed`
|
||
test. Pre-existing latent bug.
|
||
2. **"Nothing to drop" is a friendly refusal** (`abce118`) —
|
||
`drop constraint` on a constraint the column never carried
|
||
refuses ("`T.col` has no UNIQUE constraint to drop")
|
||
instead of a silent no-op rebuild. This was *not* spelled
|
||
out in handoff-22 §4; it is consistent with ADR-0029 §9's
|
||
"clarity over permissiveness" and was an autonomous
|
||
call — flagged here and accepted by the user at commit
|
||
time.
|
||
3. **CHECK friendly error went full ADR §10** (`5e97f6a`) —
|
||
the user chose, when asked, to surface the rule + offending
|
||
value rather than the minimal handoff-§4 scope. ADR §10's
|
||
prose said "DSL-form expression"; deviation §7 had removed
|
||
the DSL-text form, so §10 was updated to say the rule is
|
||
shown in its **compiled-SQL form** — consistent with §7/§8.
|
||
|
||
## §4. Key reusable pieces (this session)
|
||
|
||
- `src/db.rs`: `do_add_constraint` / `do_drop_constraint`;
|
||
`read_constraint_dry_run_rows` + `dry_run_not_null` /
|
||
`dry_run_unique` / `dry_run_check`; `render_constraint_
|
||
dry_run` and the `dry_run_id_*` helpers; `value_to_default_
|
||
sql` (factored out of `default_sql_literal`). `DryRunRow`
|
||
type alias.
|
||
- `src/runtime.rs`: `enrich_check_violation`.
|
||
- `src/friendly/`: `FailureContext.check_rule` /
|
||
`TranslateContext.check_rule`; `error.check.*.hint_with_rule`
|
||
catalog keys.
|
||
- `Operation::AddConstraint` / `DropConstraint` in
|
||
`src/friendly/translate.rs`.
|
||
|
||
## §5. What's next
|
||
|
||
ADR-0029 closes requirement `C3`. No feature is in flight.
|
||
Open clusters (prioritisation is a **user decision — ask**):
|
||
|
||
- Snapshot / undo / replay `U`-series (designed in ADR-0006,
|
||
not yet implemented).
|
||
- m:n convenience `C4`; modify-relationship `C3a` (drop+add
|
||
covers it today).
|
||
- Project storage Iter 5–6: `export` / `import`, `--resume`,
|
||
persistent input history, migration-framework scaffold.
|
||
- SQL handling in advanced mode (`sqlparser-rs`, its own ADR).
|
||
- Friendly-error layer `H1` (the remaining SQL→English
|
||
sweep); strong syntax-help in parse errors `H1a`.
|
||
- Session-log / Markdown export `V4`; ER diagram export `V3`.
|
||
- CI workflow `TT5`; readline shortcuts `I1b`; multi-line
|
||
input `I1`; Tab completion polish `I3`.
|
||
|
||
## §6. How to take over
|
||
|
||
1. **Read this file**, then `CLAUDE.md` (working-style
|
||
rules), then `docs/requirements.md` (per-item progress —
|
||
`C3` now ticked).
|
||
2. **Run `cargo test`** — 1240 passing, 0 failing, 1 ignored.
|
||
3. **Run `cargo clippy --all-targets -- -D warnings`** —
|
||
clean.
|
||
4. Pick the next cluster *with the user* — §5 has no default.
|
||
|
||
### Note on the typing-surface matrix
|
||
|
||
`tests/typing_surface/` is **188 cells**. The matrix-snapshot
|
||
discipline stands: a failing cell with *correct* new
|
||
behaviour → update its snapshot; with *wrong* behaviour → the
|
||
cell earned its keep. `cargo insta` is not installed —
|
||
regenerate with `INSTA_UPDATE=always cargo test --test
|
||
typing_surface_matrix <filter>` and review the written
|
||
`.snap` files before committing.
|