feat: ADR-0035 Amendment 1 — drop composite UNIQUE; friendlier drop-column + generic-error wording
F1/F2/F3 from the whole-Phase-4 /runda (handoff-42 §3):
- F3: drop an anonymous composite UNIQUE via a derived, engine-neutral
name `unique_<cols>` — recomputed live, nothing persisted, reusing the
existing `DROP CONSTRAINT <name>` grammar (no new syntax/metadata, the
§4g anonymity decision intact). A name matching more than one UNIQUE is
refused as ambiguous, never guessed. One undo step. `describe`
annotates each composite UNIQUE with its name.
- F1: dropping a column a composite UNIQUE covers is refused up-front
with the derived name + the actionable drop command (was an unhelpful
generic engine refusal).
- F2: contextless friendly_message() no longer leaks a literal `{table}`
in the generic hint (new `error.generic.hint_no_table`, selected when
no table is in context). The table-ful path is unchanged.
Docs: ADR-0035 Amendment 1 + Status + README index + plan
docs/plans/20260526-adr-0035-composite-unique-drop-f1f2f3.md.
Tests: +5 (drop-by-name, ambiguous-refused, one-undo-step, F1 guard,
F2 no-leak) + a describe-render assertion. 1922 pass / 0 fail / 0 skip;
clippy clean.
This commit is contained in:
@@ -4359,6 +4359,26 @@ fn do_drop_column(
|
||||
)));
|
||||
}
|
||||
|
||||
// A composite UNIQUE covering this column (ADR-0035 Amendment 1): the
|
||||
// engine refuses to drop a column a UNIQUE constraint spans, so refuse
|
||||
// up-front with the constraint's derived name and the actionable drop
|
||||
// command. Single-column UNIQUEs ride on the column `unique` flag (the
|
||||
// engine drops their auto-index with the column), not
|
||||
// `unique_constraints`, so they do not reach here.
|
||||
if let Some(cols) = schema
|
||||
.unique_constraints
|
||||
.iter()
|
||||
.find(|cols| cols.iter().any(|c| c == column))
|
||||
{
|
||||
let cname = unique_constraint_name(cols);
|
||||
return Err(DbError::Unsupported(format!(
|
||||
"cannot drop `{table}.{column}` — it is part of the UNIQUE \
|
||||
constraint `{cname}` ({}); drop that constraint first \
|
||||
(`alter table {table} drop constraint {cname}`).",
|
||||
cols.join(", "),
|
||||
)));
|
||||
}
|
||||
|
||||
// A CHECK (table-level, or a *different* column's column-level CHECK)
|
||||
// that references this column (ADR-0035 §4e, the 4a.3 deferral): a
|
||||
// deliberate up-front refusal — dropping the column would break that
|
||||
@@ -6121,6 +6141,16 @@ fn read_unique_constraints(
|
||||
Ok((single, composite))
|
||||
}
|
||||
|
||||
/// Engine-neutral display/address name for an anonymous composite UNIQUE
|
||||
/// constraint (ADR-0035 Amendment 1): `unique_<col1>_<col2>…`. A pure
|
||||
/// function of the column list — recomputed wherever the name is shown
|
||||
/// (`describe`) or matched (`ALTER TABLE … DROP CONSTRAINT <name>`), so
|
||||
/// nothing is persisted; the constraint stays a bare column-list in our
|
||||
/// model and the §4g anonymity decision is intact.
|
||||
pub(crate) fn unique_constraint_name(cols: &[String]) -> String {
|
||||
format!("unique_{}", cols.join("_"))
|
||||
}
|
||||
|
||||
/// Generate the CREATE TABLE DDL from a `ReadSchema`. Used during
|
||||
/// the rebuild dance.
|
||||
fn schema_to_ddl(table: &str, schema: &ReadSchema) -> String {
|
||||
@@ -7073,7 +7103,46 @@ fn do_drop_constraint_by_name(
|
||||
);
|
||||
}
|
||||
|
||||
// 3. Not a known named constraint on this table.
|
||||
// 3. A composite UNIQUE whose derived name (ADR-0035 Amendment 1,
|
||||
// `unique_<cols>`) matches? The constraint is anonymous in our
|
||||
// model, so we recompute each composite UNIQUE's name and match.
|
||||
// Order matters: a named CHECK/FK above shadows a derived UNIQUE
|
||||
// name (the distinctive `unique_` prefix makes a clash unlikely).
|
||||
let schema = read_schema(conn, table)?;
|
||||
let matched_cols: Vec<Vec<String>> = schema
|
||||
.unique_constraints
|
||||
.iter()
|
||||
.filter(|cols| unique_constraint_name(cols) == name)
|
||||
.cloned()
|
||||
.collect();
|
||||
if matched_cols.len() > 1 {
|
||||
// Two distinct UNIQUEs can derive the same name (e.g. a column
|
||||
// literally named `b_c` vs `UNIQUE (b, c)`). Refuse rather than
|
||||
// guess which to drop.
|
||||
return Err(DbError::Unsupported(format!(
|
||||
"the constraint name `{name}` is ambiguous on `{table}` — it \
|
||||
matches more than one UNIQUE constraint; recreate the table \
|
||||
to change them."
|
||||
)));
|
||||
}
|
||||
if let Some(cols) = matched_cols.first() {
|
||||
let old_schema = schema.clone();
|
||||
let mut new_schema = schema;
|
||||
new_schema.unique_constraints.retain(|c| c != cols);
|
||||
let table_owned = table.to_string();
|
||||
rebuild_table(conn, table, &old_schema, &new_schema, |tx| {
|
||||
let changes = Changes {
|
||||
schema_dirty: true,
|
||||
rewritten_tables: vec![table_owned.clone()],
|
||||
..Changes::default()
|
||||
};
|
||||
finalize_persistence(tx, persistence, source, &changes)?;
|
||||
Ok(())
|
||||
})?;
|
||||
return Ok(Some(do_describe_table(conn, table)?));
|
||||
}
|
||||
|
||||
// 4. Not a known named constraint on this table.
|
||||
Err(DbError::Sqlite {
|
||||
message: format!("no such constraint: {name} on {table}"),
|
||||
kind: SqliteErrorKind::Other,
|
||||
|
||||
@@ -109,6 +109,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
||||
// ---- Generic engine refusal ----
|
||||
("error.generic.headline", &["operation"]),
|
||||
("error.generic.hint", &["table"]),
|
||||
("error.generic.hint_no_table", &[]),
|
||||
// ---- Invalid-value errors (pre-engine, single-line) ----
|
||||
(
|
||||
"error.invalid_value.arity.headline",
|
||||
|
||||
@@ -142,6 +142,10 @@ error:
|
||||
generic:
|
||||
headline: "the database refused this `{operation}`."
|
||||
hint: "The operation could not be completed against the current state of `{table}`."
|
||||
# Used when no table is in context (e.g. contextless `friendly_message()`
|
||||
# callsites: replay, undo, rebuild, export) so the hint never leaks a
|
||||
# literal `{table}` placeholder.
|
||||
hint_no_table: "The operation could not be completed against the current database state."
|
||||
|
||||
# Errors that are specifically about value validation
|
||||
# (DbError::InvalidValue) — wrong arity, wrong literal
|
||||
|
||||
@@ -659,10 +659,17 @@ fn translate_generic(message: &str, ctx: &TranslateContext) -> FriendlyError {
|
||||
let operation = ctx
|
||||
.operation
|
||||
.map_or("operation", Operation::keyword);
|
||||
let table = ctx_table(ctx);
|
||||
// F2 (ADR-0035 Amendment 1): when no table is in context, use the
|
||||
// table-less hint so a contextless `friendly_message()` (replay, undo,
|
||||
// rebuild, export) never renders a literal `{table}` placeholder.
|
||||
let hint = if ctx.table.is_some() {
|
||||
t!("error.generic.hint", table = ctx_table(ctx))
|
||||
} else {
|
||||
t!("error.generic.hint_no_table")
|
||||
};
|
||||
fe(
|
||||
t!("error.generic.headline", operation = operation),
|
||||
verbose_hint(ctx, t!("error.generic.hint", table = table)),
|
||||
verbose_hint(ctx, hint),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1034,6 +1041,28 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generic_hint_has_no_unsubstituted_table_without_context() {
|
||||
// F2 (ADR-0035 Amendment 1 plan): `friendly_message()` renders
|
||||
// with a default, table-less context at the default Verbose
|
||||
// verbosity, so a generic-bucket error must not leak a literal
|
||||
// `{table}` in its hint.
|
||||
let err = sqlite("some unclassified engine failure", SqliteErrorKind::Other);
|
||||
let rendered = translate(&err, &TranslateContext::default()).render();
|
||||
assert!(
|
||||
!rendered.contains("{table}"),
|
||||
"no unsubstituted placeholder in the table-less generic hint; got:\n{rendered}"
|
||||
);
|
||||
// The table-ful path is unchanged: a table in context still names it.
|
||||
let mut ctx = TranslateContext::for_op(Operation::Delete);
|
||||
ctx.table = Some("Orders".to_string());
|
||||
let with_table = translate(&err, &ctx).render();
|
||||
assert!(
|
||||
with_table.contains("Orders"),
|
||||
"the table-ful generic hint still names the table; got:\n{with_table}"
|
||||
);
|
||||
}
|
||||
|
||||
// ---- passthrough variants ----
|
||||
|
||||
#[test]
|
||||
|
||||
+10
-1
@@ -160,7 +160,13 @@ pub fn render_structure(desc: &TableDescription) -> Vec<String> {
|
||||
if !desc.unique_constraints.is_empty() || !desc.check_constraints.is_empty() {
|
||||
out.push("Table constraints:".to_string());
|
||||
for cols in &desc.unique_constraints {
|
||||
out.push(format!(" unique ({})", cols.join(", ")));
|
||||
// Annotate with the derived, addressable name (ADR-0035
|
||||
// Amendment 1) so the user can `drop constraint <name>`.
|
||||
out.push(format!(
|
||||
" {}: unique ({})",
|
||||
crate::db::unique_constraint_name(cols),
|
||||
cols.join(", ")
|
||||
));
|
||||
}
|
||||
for chk in &desc.check_constraints {
|
||||
match &chk.name {
|
||||
@@ -880,6 +886,9 @@ mod tests {
|
||||
let out = render_structure(&desc).join("\n");
|
||||
assert!(out.contains("Table constraints:"), "got:\n{out}");
|
||||
assert!(out.contains("unique (a, b)"), "got:\n{out}");
|
||||
// The composite UNIQUE shows its derived, addressable name
|
||||
// (ADR-0035 Amendment 1) so the user can `drop constraint <name>`.
|
||||
assert!(out.contains("unique_a_b: unique (a, b)"), "got:\n{out}");
|
||||
assert!(out.contains("check (a < b)"), "unnamed check; got:\n{out}");
|
||||
assert!(out.contains("check a_lt_b (a <> b)"), "named check shows its name; got:\n{out}");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user