Needs check: can we drop the PK from a table? #19

New Issue

2026-06-10T13:50:12+01:00

oli commented

2026-06-10 13:50:12 +01:00

We don't support creating a table with no PK, so an attempt to drop it should be met with a friendly error message.

claude-clouddev1 commented

2026-06-10 14:05:24 +01:00

Checked end-to-end. Dropping a PK column is already refused, in both modes — so the core concern here is satisfied.

The guard lives in the shared do_drop_column executor (src/db.rs): it pre-checks col_info.primary_key and returns "cannot drop primary-key column T.col. Drop the table or change the primary key first."
Both surfaces reach it: simple drop column T: id → Command::DropColumn, and advanced ALTER TABLE T DROP COLUMN id → AlterTableAction::DropColumn → database.drop_column(...) (runtime.rs). Same executor, same guard.
Now locked with end-to-end tests on the advanced surface (single-column and compound PK), asserting refusal for the right reason: e2e_alter_drop_primary_key_column_is_refused / e2e_alter_drop_compound_primary_key_member_is_refused in tests/it/sql_alter_table.rs. The single-column worker path was already covered by drop_column_refuses_primary_key + the message is locked in engine_vocabulary_audit.rs.

One premise correction: the issue says "we don't support creating a table with no PK" — that's true for simple mode only. The advanced SQL surface follows standard SQL and does allow a PK-less table (create table t (a int) — verified end-to-end). This is fine: SQLite keys every ordinary table by its implicit rowid, so a PK-less table is valid and fully handled (show data … limit falls back to rowid order; update/delete target rows by rowid). A PK-less table simply has no "PK column" to drop, so the guard is moot there.

This advanced-mode difference is now documented in docs/simple-mode-limitations.md (Table creation section).

Recommendation: close — the drop-PK guard is in place, refused in both modes, and now covered by tests; the no-PK-in-advanced-mode capability is intentional and documented.

Checked end-to-end. **Dropping a PK column is already refused, in both modes** — so the core concern here is satisfied. - The guard lives in the shared `do_drop_column` executor (`src/db.rs`): it pre-checks `col_info.primary_key` and returns *"cannot drop primary-key column `T.col`. Drop the table or change the primary key first."* - Both surfaces reach it: simple `drop column T: id` → `Command::DropColumn`, and advanced `ALTER TABLE T DROP COLUMN id` → `AlterTableAction::DropColumn` → `database.drop_column(...)` (`runtime.rs`). Same executor, same guard. - Now locked with end-to-end tests on the advanced surface (single-column **and** compound PK), asserting refusal *for the right reason*: `e2e_alter_drop_primary_key_column_is_refused` / `e2e_alter_drop_compound_primary_key_member_is_refused` in `tests/it/sql_alter_table.rs`. The single-column worker path was already covered by `drop_column_refuses_primary_key` + the message is locked in `engine_vocabulary_audit.rs`. **One premise correction:** the issue says *"we don't support creating a table with no PK"* — that's true for **simple mode** only. The **advanced SQL** surface follows standard SQL and *does* allow a PK-less table (`create table t (a int)` — verified end-to-end). This is fine: SQLite keys every ordinary table by its implicit `rowid`, so a PK-less table is valid and fully handled (`show data … limit` falls back to rowid order; `update`/`delete` target rows by rowid). A PK-less table simply has no "PK column" to drop, so the guard is moot there. This advanced-mode difference is now documented in `docs/simple-mode-limitations.md` (Table creation section). **Recommendation: close** — the drop-PK guard is in place, refused in both modes, and now covered by tests; the no-PK-in-advanced-mode capability is intentional and documented.

claude-clouddev1 closed this issue

2026-06-10 14:17:31 +01:00

oli referenced this issue from a commit

2026-06-10 14:58:15 +01:00

test+docs: lock drop-PK-refused on advanced surface; document no-PK advanced mode (#19)

oli referenced this issue from a commit

2026-06-10 19:14:29 +01:00

docs: session handoff 62 — C4 m:n convenience command + issue #19

Sign in to join this conversation.