fix(fk): inline FK referencing a compound PK points at the table-level form
ADR-0043 D4 residual: an inline column-level FK (`<col> REFERENCES P(a,b)`)
is single-column by construction, so referencing a parent's compound PK
gave the generic arity error ("1 foreign-key column(s) on the child side,
but `P`'s key has 2..."). It now points the user at the table-level form:
"an inline column reference can only name one column ... Use the table-level
form instead: FOREIGN KEY (<columns>) REFERENCES P (a, b)".
- Adds `inline: bool` to SqlForeignKey, set by the grammar's single shared
builder consume_fk_reference (true for the inline path, false for the
table-level and ALTER paths).
- resolve_fk_parent_columns takes `inline` and tailors the arity-mismatch
message when an inline FK meets a compound key.
Tests: parse-layer (inline=true / table-level=false) + end-to-end worker
refusal wording. 2209 pass / 0 fail / 1 ignored. Clippy clean.
This commit is contained in:
@@ -7110,6 +7110,7 @@ fn resolve_fk_parent_columns(
|
|||||||
parent_pk: &[String],
|
parent_pk: &[String],
|
||||||
explicit: Option<&[String]>,
|
explicit: Option<&[String]>,
|
||||||
child_arity: usize,
|
child_arity: usize,
|
||||||
|
inline: bool,
|
||||||
) -> Result<Vec<String>, DbError> {
|
) -> Result<Vec<String>, DbError> {
|
||||||
if child_arity == 0 {
|
if child_arity == 0 {
|
||||||
return Err(DbError::Unsupported(
|
return Err(DbError::Unsupported(
|
||||||
@@ -7142,6 +7143,20 @@ fn resolve_fk_parent_columns(
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
if parent_columns.len() != child_arity {
|
if parent_columns.len() != child_arity {
|
||||||
|
// An inline column-level FK (`<col> REFERENCES …`) can only carry
|
||||||
|
// the one column it sits on, so it can never satisfy a compound
|
||||||
|
// key — point the user at the table-level form rather than the
|
||||||
|
// generic arity message (ADR-0043 D4).
|
||||||
|
if inline && parent_columns.len() > 1 {
|
||||||
|
return Err(DbError::Unsupported(format!(
|
||||||
|
"an inline column reference can only name one column, but \
|
||||||
|
`{parent_table}`'s key has {n}. Use the table-level form \
|
||||||
|
instead: `FOREIGN KEY (<columns>) REFERENCES \
|
||||||
|
{parent_table} ({pk})`.",
|
||||||
|
n = parent_columns.len(),
|
||||||
|
pk = parent_columns.join(", "),
|
||||||
|
)));
|
||||||
|
}
|
||||||
return Err(DbError::Unsupported(format!(
|
return Err(DbError::Unsupported(format!(
|
||||||
"{child_arity} foreign-key column(s) on the child side, but \
|
"{child_arity} foreign-key column(s) on the child side, but \
|
||||||
`{parent_table}`'s key has {n}. A foreign key references every \
|
`{parent_table}`'s key has {n}. A foreign key references every \
|
||||||
@@ -7210,6 +7225,7 @@ fn resolve_create_table_fks(
|
|||||||
&parent_pk,
|
&parent_pk,
|
||||||
fk.parent_columns.as_deref(),
|
fk.parent_columns.as_deref(),
|
||||||
fk.child_columns.len(),
|
fk.child_columns.len(),
|
||||||
|
fk.inline,
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
// Each child column must be one of the columns being defined,
|
// Each child column must be one of the columns being defined,
|
||||||
@@ -7295,6 +7311,7 @@ fn do_add_relationship(
|
|||||||
&parent_schema.primary_key,
|
&parent_schema.primary_key,
|
||||||
Some(parent_columns),
|
Some(parent_columns),
|
||||||
child_columns.len(),
|
child_columns.len(),
|
||||||
|
false, // DSL `add relationship` is never an inline column FK
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
// 2. Read child schema; refuse missing columns unless --create-fk.
|
// 2. Read child schema; refuse missing columns unless --create-fk.
|
||||||
@@ -7824,6 +7841,7 @@ fn do_alter_add_foreign_key(
|
|||||||
&parent_pk,
|
&parent_pk,
|
||||||
fk.parent_columns.as_deref(),
|
fk.parent_columns.as_deref(),
|
||||||
fk.child_columns.len(),
|
fk.child_columns.len(),
|
||||||
|
fk.inline, // false for `ALTER … ADD FOREIGN KEY` (table-level)
|
||||||
)?;
|
)?;
|
||||||
// Every child column must already exist for `ALTER … ADD FOREIGN
|
// Every child column must already exist for `ALTER … ADD FOREIGN
|
||||||
// KEY` — there is no SQL spelling to auto-create one (`--create-fk`
|
// KEY` — there is no SQL spelling to auto-create one (`--create-fk`
|
||||||
|
|||||||
@@ -45,6 +45,13 @@ pub struct SqlForeignKey {
|
|||||||
pub parent_columns: Option<Vec<String>>,
|
pub parent_columns: Option<Vec<String>>,
|
||||||
pub on_delete: ReferentialAction,
|
pub on_delete: ReferentialAction,
|
||||||
pub on_update: ReferentialAction,
|
pub on_update: ReferentialAction,
|
||||||
|
/// `true` for an inline column-level FK (`<col> REFERENCES …`),
|
||||||
|
/// `false` for the table-level `FOREIGN KEY (…)` and `ALTER …`
|
||||||
|
/// forms. An inline FK is single-column by construction, so when
|
||||||
|
/// it references a compound key the resolver points the user at
|
||||||
|
/// the table-level form rather than emitting the generic arity
|
||||||
|
/// error (ADR-0043 D4).
|
||||||
|
pub inline: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A column at table-creation time: a name, a user-facing
|
/// A column at table-creation time: a name, a user-facing
|
||||||
|
|||||||
@@ -1557,7 +1557,7 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result<Command, V
|
|||||||
// Inline FK is single-column (the column it sits on);
|
// Inline FK is single-column (the column it sits on);
|
||||||
// a compound FK uses the table-level form (ADR-0043 D4).
|
// a compound FK uses the table-level form (ADR-0043 D4).
|
||||||
let child_column = columns.last().map_or_else(String::new, |c| c.name.clone());
|
let child_column = columns.last().map_or_else(String::new, |c| c.name.clone());
|
||||||
foreign_keys.push(consume_fk_reference(&mut items, None, vec![child_column]));
|
foreign_keys.push(consume_fk_reference(&mut items, None, vec![child_column], true));
|
||||||
}
|
}
|
||||||
// Table-level `[constraint <name>] foreign key (<col>)
|
// Table-level `[constraint <name>] foreign key (<col>)
|
||||||
// references <parent> [(<col>)] [on …]` (ADR-0035 §5, 4b).
|
// references <parent> [(<col>)] [on …]` (ADR-0035 §5, 4b).
|
||||||
@@ -1587,7 +1587,8 @@ fn build_sql_create_table(path: &MatchedPath, source: &str) -> Result<Command, V
|
|||||||
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) {
|
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) {
|
||||||
items.next();
|
items.next();
|
||||||
}
|
}
|
||||||
let fk = consume_fk_reference(&mut items, pending_fk_name.take(), child_columns);
|
let fk =
|
||||||
|
consume_fk_reference(&mut items, pending_fk_name.take(), child_columns, false);
|
||||||
foreign_keys.push(fk);
|
foreign_keys.push(fk);
|
||||||
}
|
}
|
||||||
// Track paren depth for element-boundary detection. The
|
// Track paren depth for element-boundary detection. The
|
||||||
@@ -1704,6 +1705,7 @@ fn consume_fk_reference<'a, I>(
|
|||||||
items: &mut std::iter::Peekable<I>,
|
items: &mut std::iter::Peekable<I>,
|
||||||
name: Option<String>,
|
name: Option<String>,
|
||||||
child_columns: Vec<String>,
|
child_columns: Vec<String>,
|
||||||
|
inline: bool,
|
||||||
) -> SqlForeignKey
|
) -> SqlForeignKey
|
||||||
where
|
where
|
||||||
I: Iterator<Item = &'a crate::dsl::walker::outcome::MatchedItem>,
|
I: Iterator<Item = &'a crate::dsl::walker::outcome::MatchedItem>,
|
||||||
@@ -1752,6 +1754,7 @@ where
|
|||||||
parent_columns,
|
parent_columns,
|
||||||
on_delete,
|
on_delete,
|
||||||
on_update,
|
on_update,
|
||||||
|
inline,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2454,7 +2457,8 @@ fn build_alter_fk(path: &MatchedPath) -> SqlForeignKey {
|
|||||||
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) {
|
if matches!(items.peek().map(|i| &i.kind), Some(MatchedKind::Word("references"))) {
|
||||||
items.next();
|
items.next();
|
||||||
}
|
}
|
||||||
consume_fk_reference(&mut items, None, child_columns)
|
// `ALTER TABLE … ADD FOREIGN KEY (…)` is the table-level form.
|
||||||
|
consume_fk_reference(&mut items, None, child_columns, false)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub static SQL_ALTER_TABLE: CommandNode = CommandNode {
|
pub static SQL_ALTER_TABLE: CommandNode = CommandNode {
|
||||||
|
|||||||
@@ -1004,6 +1004,16 @@ mod builder_tests {
|
|||||||
assert_eq!(fk.parent_columns, Some(vec!["id".to_string()]));
|
assert_eq!(fk.parent_columns, Some(vec!["id".to_string()]));
|
||||||
assert_eq!(fk.on_delete, ReferentialAction::NoAction);
|
assert_eq!(fk.on_delete, ReferentialAction::NoAction);
|
||||||
assert_eq!(fk.on_update, ReferentialAction::NoAction);
|
assert_eq!(fk.on_update, ReferentialAction::NoAction);
|
||||||
|
assert!(fk.inline, "a column-level `references` is an inline FK (ADR-0043 D4)");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn table_level_fk_is_not_inline() {
|
||||||
|
// The table-level `FOREIGN KEY (...)` form is not inline, so it can
|
||||||
|
// carry a multi-column reference and never triggers the inline
|
||||||
|
// "use the table-level form" hint (ADR-0043 D4).
|
||||||
|
let fks = parse_sct_fks("create table t (id int, pid int, foreign key (pid) references parent(id))");
|
||||||
|
assert!(!fks[0].inline, "table-level FOREIGN KEY is not inline");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -137,6 +137,7 @@ fn sql_create_table_compound_fk_executes_and_enforces() {
|
|||||||
parent_columns: Some(vec!["country".to_string(), "code".to_string()]),
|
parent_columns: Some(vec!["country".to_string(), "code".to_string()]),
|
||||||
on_delete: ReferentialAction::NoAction,
|
on_delete: ReferentialAction::NoAction,
|
||||||
on_update: ReferentialAction::NoAction,
|
on_update: ReferentialAction::NoAction,
|
||||||
|
inline: false,
|
||||||
}],
|
}],
|
||||||
false,
|
false,
|
||||||
None,
|
None,
|
||||||
@@ -363,6 +364,65 @@ fn compound_fk_arity_mismatch_is_refused() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn inline_fk_referencing_compound_pk_points_at_table_level_form() {
|
||||||
|
// ADR-0043 D4 residual: an *inline* single-column FK cannot express a
|
||||||
|
// multi-column reference, so referencing a parent's compound PK must
|
||||||
|
// refuse with a pointer to the table-level `FOREIGN KEY (...)` form —
|
||||||
|
// not the generic arity message. The grammar marks the FK `inline`.
|
||||||
|
let (_p, db, _dir) = open_project_db();
|
||||||
|
let rt = rt();
|
||||||
|
rt.block_on(async {
|
||||||
|
db.create_table(
|
||||||
|
"Region".to_string(),
|
||||||
|
vec![
|
||||||
|
ColumnSpec::new("country", Type::Int),
|
||||||
|
ColumnSpec::new("code", Type::Int),
|
||||||
|
],
|
||||||
|
vec!["country".to_string(), "code".to_string()],
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.expect("create Region");
|
||||||
|
|
||||||
|
// Parse the inline form so the `inline` flag is set by the grammar.
|
||||||
|
let cmd = parse_command(
|
||||||
|
"create table City (country int references Region(country, code))",
|
||||||
|
)
|
||||||
|
.expect("parses");
|
||||||
|
let Command::SqlCreateTable {
|
||||||
|
name,
|
||||||
|
columns,
|
||||||
|
primary_key,
|
||||||
|
unique_constraints,
|
||||||
|
check_constraints,
|
||||||
|
foreign_keys,
|
||||||
|
if_not_exists,
|
||||||
|
} = cmd
|
||||||
|
else {
|
||||||
|
panic!("expected SqlCreateTable");
|
||||||
|
};
|
||||||
|
let err = db
|
||||||
|
.sql_create_table(
|
||||||
|
name,
|
||||||
|
columns,
|
||||||
|
primary_key,
|
||||||
|
unique_constraints,
|
||||||
|
check_constraints,
|
||||||
|
foreign_keys,
|
||||||
|
if_not_exists,
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.expect_err("inline FK referencing a compound PK must be refused");
|
||||||
|
let msg = format!("{err}");
|
||||||
|
assert!(
|
||||||
|
msg.contains("FOREIGN KEY"),
|
||||||
|
"expected a pointer to the table-level `FOREIGN KEY (...)` form, got: {msg}"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn compound_fk_type_mismatch_per_pair_is_refused() {
|
fn compound_fk_type_mismatch_per_pair_is_refused() {
|
||||||
let (_p, db, _dir) = open_project_db();
|
let (_p, db, _dir) = open_project_db();
|
||||||
|
|||||||
@@ -839,6 +839,7 @@ fn fk(child_column: &str, parent_table: &str, parent_column: Option<&str>) -> Sq
|
|||||||
parent_columns: parent_column.map(|c| vec![c.to_string()]),
|
parent_columns: parent_column.map(|c| vec![c.to_string()]),
|
||||||
on_delete: ReferentialAction::NoAction,
|
on_delete: ReferentialAction::NoAction,
|
||||||
on_update: ReferentialAction::NoAction,
|
on_update: ReferentialAction::NoAction,
|
||||||
|
inline: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -109,6 +109,7 @@ fn dropping_a_referenced_parent_is_refused() {
|
|||||||
parent_columns: Some(vec!["id".to_string()]),
|
parent_columns: Some(vec!["id".to_string()]),
|
||||||
on_delete: rdbms_playground::dsl::ReferentialAction::NoAction,
|
on_delete: rdbms_playground::dsl::ReferentialAction::NoAction,
|
||||||
on_update: rdbms_playground::dsl::ReferentialAction::NoAction,
|
on_update: rdbms_playground::dsl::ReferentialAction::NoAction,
|
||||||
|
inline: true,
|
||||||
}],
|
}],
|
||||||
false,
|
false,
|
||||||
Some("create table child (id serial primary key, pid int references parent(id))".to_string()),
|
Some("create table child (id serial primary key, pid int references parent(id))".to_string()),
|
||||||
|
|||||||
Reference in New Issue
Block a user