test+docs: lock drop-PK-refused on advanced surface; document no-PK advanced mode (#19)
Dropping a PK column was already refused in both modes via the shared do_drop_column guard; this adds end-to-end coverage on the advanced ALTER surface (single-column + compound PK, asserting refusal for the right reason) and documents the asymmetry that advanced-mode SQL can create a PK-less table (SQLite's implicit rowid keys it) while simple mode forbids it. See issue #19 comment for the full assessment.
This commit is contained in:
@@ -41,6 +41,22 @@ entry names the ADR that drew the boundary.
|
|||||||
|
|
||||||
## Table creation (ADR-0029)
|
## Table creation (ADR-0029)
|
||||||
|
|
||||||
|
- **A simple-mode table always has a primary key; an advanced-mode
|
||||||
|
table need not.** `create table … with pk …` is mandatory in simple
|
||||||
|
mode (ADR-0029) — the bare `with pk` even defaults to `id serial`.
|
||||||
|
Advanced-mode SQL follows standard SQL and permits a *PK-less* table:
|
||||||
|
`create table t (a int)` declares no primary key. This is **not** a
|
||||||
|
storage problem — every ordinary table (STRICT included) carries
|
||||||
|
SQLite's implicit `rowid`, which keys it internally; only a
|
||||||
|
`WITHOUT ROWID` table (which this app never creates) would lack one.
|
||||||
|
So the simple-mode requirement is a *pedagogical* boundary (teach that
|
||||||
|
tables should have a key), not an engine constraint. Consequences in a
|
||||||
|
PK-less table, all handled: `show data … limit` falls back to rowid
|
||||||
|
order (no stable user-facing key to order by); `update` / `delete`
|
||||||
|
still target the affected rows by rowid; and there is no "PK column"
|
||||||
|
to drop — dropping a *declared* PK column is refused in **both** modes
|
||||||
|
(the shared `do_drop_column` guard: *"cannot drop primary-key column
|
||||||
|
…"*).
|
||||||
- **`create table` declares only primary-key columns.**
|
- **`create table` declares only primary-key columns.**
|
||||||
`create table T with pk …` makes every listed column part
|
`create table T with pk …` makes every listed column part
|
||||||
of the primary key; there is no simple-mode syntax for a
|
of the primary key; there is no simple-mode syntax for a
|
||||||
|
|||||||
@@ -65,6 +65,50 @@ fn replay_is_refused(script: &str) -> bool {
|
|||||||
matches!(events.last(), Some(AppEvent::ReplayFailed { .. }))
|
matches!(events.last(), Some(AppEvent::ReplayFailed { .. }))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Like [`replay_is_refused`] but returns the failure message, so a test
|
||||||
|
/// can assert the command was refused *for the expected reason* rather
|
||||||
|
/// than e.g. a parse error.
|
||||||
|
fn replay_failure_message(script: &str) -> Option<String> {
|
||||||
|
let (project, db, _d) = open();
|
||||||
|
let r = rt();
|
||||||
|
std::fs::write(project.path().join("conv.commands"), script).expect("write script");
|
||||||
|
let events = r.block_on(run_replay(&db, project.path(), "conv.commands"));
|
||||||
|
match events.last() {
|
||||||
|
Some(AppEvent::ReplayFailed { error, .. }) => Some(error.clone()),
|
||||||
|
_ => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn e2e_alter_drop_primary_key_column_is_refused() {
|
||||||
|
// Issue #19: dropping a PK column must be refused on the advanced
|
||||||
|
// ALTER surface too (it reaches the shared `do_drop_column` guard).
|
||||||
|
let msg = replay_failure_message(
|
||||||
|
"create table T (id int primary key, v text)\n\
|
||||||
|
alter table T drop column id\n",
|
||||||
|
)
|
||||||
|
.expect("dropping a PK column must be refused");
|
||||||
|
assert!(
|
||||||
|
msg.to_lowercase().contains("primary"),
|
||||||
|
"refused for the wrong reason: {msg}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn e2e_alter_drop_compound_primary_key_member_is_refused() {
|
||||||
|
// A member of a *compound* PK is still a PK column, so dropping it is
|
||||||
|
// refused identically (each member reports primary_key = true).
|
||||||
|
let msg = replay_failure_message(
|
||||||
|
"create table T (a int, b int, v text, primary key (a, b))\n\
|
||||||
|
alter table T drop column a\n",
|
||||||
|
)
|
||||||
|
.expect("dropping a compound-PK member must be refused");
|
||||||
|
assert!(
|
||||||
|
msg.to_lowercase().contains("primary"),
|
||||||
|
"refused for the wrong reason: {msg}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// The current user-facing type of column `name` in table `T`.
|
/// The current user-facing type of column `name` in table `T`.
|
||||||
fn col_type(db: &Database, r: &tokio::runtime::Runtime, name: &str) -> Option<Type> {
|
fn col_type(db: &Database, r: &tokio::runtime::Runtime, name: &str) -> Option<Type> {
|
||||||
r.block_on(db.describe_table("T".to_string(), None))
|
r.block_on(db.describe_table("T".to_string(), None))
|
||||||
|
|||||||
Reference in New Issue
Block a user