fix: ADR-0036 — name the offending value for natural-order SQL INSERTs
A no-column-list (natural-order / Form B) SQL INSERT that hit a UNIQUE/ CHECK violation degraded to the neutral "that value" because user_value_for_column only resolved the offending value for the explicit- column-list form. Extend user_value_for_column_with_schema to map each VALUES position to the schema's columns in declaration order (ALL columns — advanced-mode Form B auto-fills nothing, so every column has a value), single-row only (multi-row stays ambiguous). Closes the Phase 1 carryover gap so the error names the real value either way. Tests: 1948 passing (+1), 0 failed, 0 skipped, 1 ignored; clippy clean.
This commit is contained in:
+28
-3
@@ -1572,9 +1572,11 @@ fn user_value_for_column(command: &Command, column: &str) -> Option<crate::dsl::
|
||||
.map(|(_, v)| v.clone()),
|
||||
// ADR-0036 Phase 1: a single-row literal SQL INSERT retains its
|
||||
// captured literals, so a constraint error can name the real
|
||||
// value. Explicit column list only here (natural order needs the
|
||||
// schema); multi-row is skipped (which row conflicted is
|
||||
// ambiguous) — both degrade to the neutral "that value".
|
||||
// value. Explicit column list resolves here (sync, no schema);
|
||||
// the natural-order (no-column-list) form is resolved in
|
||||
// `user_value_for_column_with_schema` (it needs the schema to map
|
||||
// position → column). Multi-row is skipped (which row conflicted
|
||||
// is ambiguous) — it degrades to the neutral "that value".
|
||||
Command::SqlInsert {
|
||||
listed_columns,
|
||||
literal_rows,
|
||||
@@ -1639,6 +1641,29 @@ async fn user_value_for_column_with_schema(
|
||||
let idx = natural_cols.iter().position(|c| *c == column)?;
|
||||
return values.get(idx).cloned();
|
||||
}
|
||||
// ADR-0036 Phase 1 follow-up: a no-column-list (natural-order) SQL
|
||||
// INSERT also names the offending value. Each VALUES position maps to
|
||||
// the schema's columns in declaration order — and unlike the DSL
|
||||
// `Insert` short form above, ALL columns are mapped (advanced-mode
|
||||
// Form B auto-fills nothing, so the user supplies a value for every
|
||||
// column; `do_sql_insert` validates against the same full mapping).
|
||||
// Single-row only — which row of a multi-row insert conflicted is
|
||||
// ambiguous, so it degrades to the neutral "that value".
|
||||
if let Command::SqlInsert {
|
||||
listed_columns,
|
||||
literal_rows,
|
||||
..
|
||||
} = command
|
||||
&& listed_columns.is_empty()
|
||||
&& literal_rows.len() == 1
|
||||
{
|
||||
let desc = database
|
||||
.describe_table(table.to_string(), None)
|
||||
.await
|
||||
.ok()?;
|
||||
let idx = desc.columns.iter().position(|c| c.name == column)?;
|
||||
return literal_rows[0].get(idx).cloned().flatten();
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
|
||||
@@ -146,6 +146,77 @@ fn enrich_unique_insert_natural_order_short_form_resolves_value_via_schema() {
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enrich_unique_sql_insert_natural_order_resolves_value_via_schema() {
|
||||
// ADR-0036 Phase 1 follow-up: a no-column-list (natural-order) SQL
|
||||
// INSERT also names the offending value in a constraint error. The
|
||||
// schema maps each VALUES position to its column, in declaration
|
||||
// order — ALL columns (advanced-mode Form B auto-fills nothing, so
|
||||
// the user supplies a value for every column).
|
||||
let db = db();
|
||||
rt().block_on(async {
|
||||
db.create_table(
|
||||
"Customers".to_string(),
|
||||
vec![
|
||||
ColumnSpec::new("id".to_string(), Type::Int),
|
||||
ColumnSpec::new("name".to_string(), Type::Text),
|
||||
],
|
||||
vec!["id".to_string()],
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
db.insert(
|
||||
"Customers".to_string(),
|
||||
None,
|
||||
vec![Value::Number("5".to_string()), Value::Text("Alice".to_string())],
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Natural-order SQL insert (no column list) collides on id=5.
|
||||
let input = "insert into Customers values (5, 'Bob')";
|
||||
let cmd = parse_command(input).expect("parses as advanced-mode SQL insert");
|
||||
let Command::SqlInsert {
|
||||
sql,
|
||||
target_table,
|
||||
listed_columns,
|
||||
row_source,
|
||||
returning,
|
||||
literal_rows,
|
||||
} = cmd.clone()
|
||||
else {
|
||||
panic!("expected Command::SqlInsert, got {cmd:?}");
|
||||
};
|
||||
assert!(listed_columns.is_empty(), "natural-order form has no column list");
|
||||
let err = db
|
||||
.run_sql_insert_with_literals(
|
||||
sql,
|
||||
None,
|
||||
target_table,
|
||||
listed_columns,
|
||||
row_source,
|
||||
returning,
|
||||
literal_rows,
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
DbError::Sqlite { kind: SqliteErrorKind::UniqueViolation, .. }
|
||||
));
|
||||
|
||||
let facts = enrich_dsl_failure(&db, &cmd, &err).await;
|
||||
assert_eq!(facts.column.as_deref(), Some("id"));
|
||||
assert_eq!(
|
||||
facts.value.as_deref(),
|
||||
Some("5"),
|
||||
"the offending value is named even without an explicit column list",
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enrich_unique_update_resolves_value_from_assignments() {
|
||||
let db = db();
|
||||
|
||||
Reference in New Issue
Block a user