Skip to content

Commit b8617d2

Browse files
joaoh82claude
andauthored
fix(sql): error on unknown columns in single-table scope (SQLR-2) (#166)
SingleTableScope::lookup silently returned Value::Null for any column not in the schema (legacy eval_expr behavior), while JoinedScope errors. Net effect: a typo'd column in WHERE silently matched every row — and on UPDATE/DELETE, mutated or deleted the entire table. Tighten SingleTableScope::lookup to error on unknown columns, matching JoinedScope. Schema columns whose cells were never written still read as NULL (omitted-from-INSERT semantics unchanged). Audit found no existing test pinning the lenient behavior. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 961cefa commit b8617d2

2 files changed

Lines changed: 91 additions & 6 deletions

File tree

docs/smoke-test.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ These should each print a clean `An error occured: …` message and leave the RE
168168
INSERT INTO users (email, dept) VALUES ('e', 'x', 999); -- 3 values for 2 columns
169169
SELECT * FROM nope; -- unknown table
170170
SELECT height FROM users; -- unknown column
171+
SELECT * FROM users WHERE height IS NULL; -- unknown column in WHERE (SQLR-2)
172+
DELETE FROM users WHERE height IS NULL; -- unknown column in WHERE — must not delete anything (SQLR-2)
171173
CREATE TABLE sqlrite_master (x INTEGER); -- reserved name
172174
SELECT * FROM users WHERE hired / 0 > 0; -- division by zero
173175
```

src/sql/executor.rs

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ use crate::sql::parser::select::{
4444
// The trait stays small on purpose:
4545
//
4646
// - `lookup` resolves a column reference (`col` or `t.col`) to a
47-
// `Value`. NULL-padded joined rows yield `Value::Null` for any
48-
// column from their side. Ambiguous unqualified references in
49-
// joined scope error.
47+
// `Value`. Unknown columns error in both scopes (SQLR-2).
48+
// NULL-padded joined rows yield `Value::Null` for any column
49+
// from their side. Ambiguous unqualified references in joined
50+
// scope error.
5051
//
5152
// - `single_table_view` lets index-probing helpers (FTS, HNSW,
5253
// vec_distance) bail out cleanly when invoked over a join — they
@@ -79,10 +80,19 @@ impl<'a> SingleTableScope<'a> {
7980
impl RowScope for SingleTableScope<'_> {
8081
fn lookup(&self, qualifier: Option<&str>, col: &str) -> Result<Value> {
8182
// The qualifier (if any) is ignored — we only have one table
82-
// in scope, so `t.col` resolves the same as `col`. This
83-
// matches the historical single-table path which did the
84-
// same thing in `eval_expr`.
83+
// in scope, so `t.col` resolves the same as `col`. Validating
84+
// it against the table name/alias requires plumbing the FROM
85+
// alias through every `eval_expr` callsite (SQLR-2 follow-up).
8586
let _ = qualifier;
87+
// SQLR-2 — unknown columns error, matching `JoinedScope`. A
88+
// schema column whose cell was never written (omitted from the
89+
// INSERT column list) still reads as NULL.
90+
if !self.table.contains_column(col.to_string()) {
91+
return Err(SQLRiteError::Internal(format!(
92+
"Column '{col}' does not exist on table '{}'",
93+
self.table.tb_name
94+
)));
95+
}
8696
Ok(self.table.get_value(col, self.rowid).unwrap_or(Value::Null))
8797
}
8898

@@ -4292,6 +4302,79 @@ mod tests {
42924302
);
42934303
}
42944304

4305+
// ---------------------------------------------------------------------
4306+
// SQLR-2 — unknown columns error in single-table scope, matching
4307+
// JoinedScope. Before the fix, lookup silently returned NULL, so a
4308+
// typo'd WHERE matched every row (catastrophic for UPDATE/DELETE).
4309+
// ---------------------------------------------------------------------
4310+
4311+
/// Seed a two-row table the SQLR-2 tests share.
4312+
fn seed_sqlr2() -> Database {
4313+
let mut db = Database::new("t".to_string());
4314+
crate::sql::process_command(
4315+
"CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT);",
4316+
&mut db,
4317+
)
4318+
.unwrap();
4319+
crate::sql::process_command("INSERT INTO t (id, name) VALUES (1, 'alice');", &mut db)
4320+
.unwrap();
4321+
crate::sql::process_command("INSERT INTO t (id, name) VALUES (2, 'bob');", &mut db)
4322+
.unwrap();
4323+
db
4324+
}
4325+
4326+
#[test]
4327+
fn where_unknown_column_errors_single_table() {
4328+
let mut db = seed_sqlr2();
4329+
let res = crate::sql::process_command("SELECT id FROM t WHERE typo IS NULL;", &mut db);
4330+
let err = res.expect_err("WHERE on an unknown column must error, not match via NULL");
4331+
assert!(
4332+
err.to_string().contains("does not exist"),
4333+
"expected unknown-column error, got: {err}"
4334+
);
4335+
}
4336+
4337+
#[test]
4338+
fn order_by_unknown_column_errors_single_table() {
4339+
let mut db = seed_sqlr2();
4340+
let res = crate::sql::process_command("SELECT id FROM t ORDER BY typo;", &mut db);
4341+
assert!(
4342+
res.is_err(),
4343+
"ORDER BY on an unknown column must error, not sort by NULL"
4344+
);
4345+
}
4346+
4347+
#[test]
4348+
fn update_with_unknown_column_in_where_errors_and_mutates_nothing() {
4349+
let mut db = seed_sqlr2();
4350+
let res =
4351+
crate::sql::process_command("UPDATE t SET name = 'x' WHERE typo IS NULL;", &mut db);
4352+
assert!(
4353+
res.is_err(),
4354+
"UPDATE with a typo'd WHERE column must error, not update every row"
4355+
);
4356+
let rows = run_select(&mut db, "SELECT id FROM t WHERE name = 'x';");
4357+
assert!(
4358+
rows.contains("0 rows returned"),
4359+
"no row may be updated when the WHERE errors, got: {rows}"
4360+
);
4361+
}
4362+
4363+
#[test]
4364+
fn delete_with_unknown_column_in_where_errors_and_deletes_nothing() {
4365+
let mut db = seed_sqlr2();
4366+
let res = crate::sql::process_command("DELETE FROM t WHERE typo IS NULL;", &mut db);
4367+
assert!(
4368+
res.is_err(),
4369+
"DELETE with a typo'd WHERE column must error, not delete every row"
4370+
);
4371+
let rows = run_select(&mut db, "SELECT id FROM t;");
4372+
assert!(
4373+
rows.contains("2 rows returned"),
4374+
"no row may be deleted when the WHERE errors, got: {rows}"
4375+
);
4376+
}
4377+
42954378
#[test]
42964379
fn where_is_null_combines_with_and_or() {
42974380
// Sanity check that the new arms compose with the existing

0 commit comments

Comments
 (0)