diff --git a/src/app.rs b/src/app.rs index 5cf5521..64e1e0a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1331,7 +1331,7 @@ impl App { C::Insert { table, .. } => (Operation::Insert, Some(table.as_str()), None), C::Update { table, .. } => (Operation::Update, Some(table.as_str()), None), C::Delete { table, .. } => (Operation::Delete, Some(table.as_str()), None), - C::ShowData { name } | C::ShowTable { name } => { + C::ShowData { name, .. } | C::ShowTable { name } => { (Operation::Query, Some(name.as_str()), None) } C::Replay { .. } => (Operation::Replay, None, None), @@ -2502,10 +2502,10 @@ mod tests { "id".to_string(), crate::dsl::Value::Number("7".to_string()), )], - filter: crate::dsl::RowFilter::Where { - column: "name".to_string(), - value: crate::dsl::Value::Text("Bob".to_string()), - }, + filter: crate::dsl::RowFilter::eq( + "name", + crate::dsl::Value::Text("Bob".to_string()), + ), }; let err = crate::db::DbError::Sqlite { message: "UNIQUE constraint failed: Customers.id".to_string(), diff --git a/src/completion.rs b/src/completion.rs index 2d1dbae..f9a325e 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -267,9 +267,20 @@ pub fn candidates_at_cursor_with( Expectation::Ident { source, .. } if source.completes_from_schema() ) }); + // A slot the grammar explicitly marked `ProseOnly` + // (`Node::Hinted`) suppresses its keyword candidates + // regardless of `has_schema_ident`. The WHERE-expression + // operand is exactly this case: it accepts a column + // reference *and* a value literal, so the signature + // heuristic alone would surface the misleading + // `null`/`true`/`false` trio (ADR-0026 §8). + let prose_only_slot = matches!( + probe.pending_hint_mode, + Some(crate::dsl::grammar::HintMode::ProseOnly(_)) + ); if partial_prefix.is_empty() - && is_value_literal_signature(&expected) - && !has_schema_ident + && (prose_only_slot + || (is_value_literal_signature(&expected) && !has_schema_ident)) { return None; } @@ -676,6 +687,15 @@ pub fn invalid_ident_at_cursor( return None; } let partial = &input[start..cursor]; + // A token that starts with a digit cannot be an identifier + // (the identifier shape requires a letter or `_` first) — + // it is a numeric literal, never an "invalid column". + // Without this guard a literal like `1` at a slot that + // *also* accepts a column reference — the WHERE-expression + // operand (ADR-0026) — is mis-flagged as an unknown column. + if partial.starts_with(|c: char| c.is_ascii_digit()) { + return None; + } let leading = &input[..start]; let expected = expected_at(leading); if expected.is_empty() { @@ -1163,6 +1183,21 @@ mod tests { assert!(!cs.contains(&"OrderTotal".to_string()), "got {cs:?}"); } + #[test] + fn numeric_literal_in_where_is_not_flagged_as_invalid_column() { + use crate::dsl::types::Type; + // ADR-0026: the WHERE-expression operand accepts a + // column reference *or* a literal. A number literal + // (`1`) sits at a slot that also expects a column — + // it must not be mis-flagged as an unknown column. + let cache = schema_with_table("Customers", &[("id", Type::Int)]); + let input = "delete from Customers where id=1"; + assert!( + invalid_ident_at_cursor(input, input.len(), &cache).is_none(), + "a numeric literal must not be reported as an invalid column", + ); + } + #[test] fn insert_into_open_paren_offers_current_table_columns() { use crate::dsl::types::Type; diff --git a/src/db.rs b/src/db.rs index 832b1c4..7dad36c 100644 --- a/src/db.rs +++ b/src/db.rs @@ -31,7 +31,10 @@ use tokio::sync::{mpsc, oneshot}; use tracing::{debug, info, warn}; use crate::dsl::action::ReferentialAction; -use crate::dsl::command::{ChangeColumnMode, IndexSelector, RelationshipSelector, RowFilter}; +use crate::dsl::command::{ + ChangeColumnMode, CompareOp, Expr, IndexSelector, Operand, Predicate, + RelationshipSelector, RowFilter, +}; use crate::dsl::ColumnSpec; use crate::dsl::shortid; use crate::dsl::types::Type; @@ -507,6 +510,8 @@ enum Request { }, QueryData { table: String, + filter: Option, + limit: Option, source: Option, reply: oneshot::Sender>, }, @@ -888,11 +893,15 @@ impl Database { pub async fn query_data( &self, table: String, + filter: Option, + limit: Option, source: Option, ) -> Result { let (reply, recv) = oneshot::channel(); self.send(Request::QueryData { table, + filter, + limit, source, reply, }) @@ -1278,6 +1287,8 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req } Request::QueryData { table, + filter, + limit, source, reply, } => { @@ -1286,6 +1297,8 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req persistence, source.as_deref(), &table, + filter.as_ref(), + limit, )); } Request::RebuildFromText { @@ -4128,6 +4141,182 @@ fn bound_to_sqlite_value(b: &Bound) -> rusqlite::types::Value { } } +// ================================================================= +// WHERE-expression → parameterised SQL (ADR-0026 §6) +// ================================================================= + +/// Compile an `Expr` to a parameterised SQL boolean expression. +/// +/// Every literal becomes a `?` placeholder pushed onto `params` +/// (1-based, continuing from the current `params.len()` — so +/// the caller can pre-load SET-clause params); identifiers are +/// `quote_ident`-quoted. The raw user text is never spliced +/// into the SQL. Connectives, `NOT`, and parentheses come from +/// the tree structure; the database re-derives precedence from +/// the emitted operators. +fn compile_expr( + expr: &Expr, + schema: &ReadSchema, + params: &mut Vec, +) -> String { + match expr { + Expr::Or(terms) => join_expr(terms, "OR", schema, params), + Expr::And(terms) => join_expr(terms, "AND", schema, params), + Expr::Not(inner) => { + format!("(NOT {})", compile_expr(inner, schema, params)) + } + Expr::Predicate(predicate) => compile_predicate(predicate, schema, params), + } +} + +fn join_expr( + terms: &[Expr], + op: &str, + schema: &ReadSchema, + params: &mut Vec, +) -> String { + let parts: Vec = terms + .iter() + .map(|t| compile_expr(t, schema, params)) + .collect(); + format!("({})", parts.join(&format!(" {op} "))) +} + +fn compile_predicate( + predicate: &Predicate, + schema: &ReadSchema, + params: &mut Vec, +) -> String { + match predicate { + Predicate::Compare { left, op, right } => { + // A literal on one side binds against the column on + // the other side, where there is one. + let left_ty = operand_column_type(left, schema); + let right_ty = operand_column_type(right, schema); + let lhs = compile_operand(left, right_ty, params); + let rhs = compile_operand(right, left_ty, params); + format!("{lhs} {} {rhs}", compare_op_sql(*op)) + } + Predicate::Like { + target, + pattern, + negated, + } => { + let t = compile_operand(target, None, params); + let p = compile_operand(pattern, None, params); + let not = if *negated { "NOT " } else { "" }; + format!("{t} {not}LIKE {p}") + } + Predicate::Between { + target, + low, + high, + negated, + } => { + let ty = operand_column_type(target, schema); + let t = compile_operand(target, None, params); + let lo = compile_operand(low, ty, params); + let hi = compile_operand(high, ty, params); + let not = if *negated { "NOT " } else { "" }; + format!("{t} {not}BETWEEN {lo} AND {hi}") + } + Predicate::In { + target, + items, + negated, + } => { + let ty = operand_column_type(target, schema); + let t = compile_operand(target, None, params); + let rendered: Vec = items + .iter() + .map(|item| compile_operand(item, ty, params)) + .collect(); + let not = if *negated { "NOT " } else { "" }; + format!("{t} {not}IN ({})", rendered.join(", ")) + } + Predicate::IsNull { target, negated } => { + let t = compile_operand(target, None, params); + let not = if *negated { "NOT " } else { "" }; + format!("{t} IS {not}NULL") + } + } +} + +/// Render an operand. A column becomes a quoted identifier; a +/// literal becomes a `?` placeholder bound against `against`'s +/// type when one is supplied. +fn compile_operand( + operand: &Operand, + against: Option, + params: &mut Vec, +) -> String { + match operand { + Operand::Column(name) => quote_ident(name), + Operand::Literal(value) => { + params.push(bind_where_literal(value, against)); + format!("?{}", params.len()) + } + } +} + +/// The user-facing type of a column operand, if the operand is +/// a column the schema knows. Literals and unknown columns +/// yield `None`. +fn operand_column_type(operand: &Operand, schema: &ReadSchema) -> Option { + match operand { + Operand::Column(name) => schema + .columns + .iter() + .find(|c| c.name.eq_ignore_ascii_case(name)) + .and_then(|c| c.user_type), + Operand::Literal(_) => None, + } +} + +/// Bind a WHERE-clause literal. When a target column type is +/// known and the literal converts cleanly, bind through it; +/// otherwise bind by the literal's own syntactic shape — a +/// type-mismatched comparison is flagged in the editor but +/// still runs (ADR-0026 §7). +fn bind_where_literal(value: &Value, against: Option) -> rusqlite::types::Value { + if let Some(ty) = against + && let Ok(bound) = value.bind_for_column("", ty) + { + return bound_to_sqlite_value(&bound); + } + bound_to_sqlite_value(&syntactic_bound(value)) +} + +/// Bind a literal by the shape it was written in, ignoring any +/// column type — the permissive fallback for type-mismatched +/// comparisons and the literal-vs-literal case. +fn syntactic_bound(value: &Value) -> Bound { + match value { + Value::Null => Bound::Null, + Value::Bool(b) => Bound::Integer(i64::from(*b)), + Value::Text(s) => Bound::Text(s.clone()), + Value::Number(s) => s.parse::().map_or_else( + |_| { + s.parse::() + .map_or_else(|_| Bound::Text(s.clone()), Bound::Real) + }, + Bound::Integer, + ), + } +} + +const fn compare_op_sql(op: CompareOp) -> &'static str { + match op { + CompareOp::Eq => "=", + // `<>` is standard SQL for inequality (ADR-0026 §6). + CompareOp::NotEq => "<>", + CompareOp::Lt => "<", + CompareOp::LtEq => "<=", + CompareOp::Gt => ">", + CompareOp::GtEq => ">=", + } +} + /// Execute an INSERT/UPDATE/DELETE and convert any rusqlite /// failure into a `DbError`. Wraps the raw `conn.execute` so the /// three callers (insert, update, delete) have a single hook for @@ -4366,18 +4555,19 @@ fn do_update( // the updated rows even if the UPDATE changed the WHERE column. let rowids = match filter { RowFilter::AllRows => select_all_rowids(conn, table)?, - RowFilter::Where { column, value } => { - let bound = impl_value_for(&schema, column, value)?; + RowFilter::Where(expr) => { + let mut where_params: Vec = Vec::new(); + let clause = compile_expr(expr, &schema, &mut where_params); let mut stmt = conn .prepare(&format!( - "SELECT rowid FROM {ident} WHERE {col} = ?1;", + "SELECT rowid FROM {ident} WHERE {clause};", ident = quote_ident(table), - col = quote_ident(column), )) .map_err(DbError::from_rusqlite)?; - let bound_param = bound_to_sqlite_value(&bound); let rows = stmt - .query_map([&bound_param], |row| row.get::<_, i64>(0)) + .query_map(rusqlite::params_from_iter(where_params.iter()), |row| { + row.get::<_, i64>(0) + }) .map_err(DbError::from_rusqlite)?; let mut ids = Vec::new(); for r in rows { @@ -4401,14 +4591,10 @@ fn do_update( let where_sql = match filter { RowFilter::AllRows => String::new(), - RowFilter::Where { column, value } => { - let bound = impl_value_for(&schema, column, value)?; - params.push(bound_to_sqlite_value(&bound)); - format!( - " WHERE {col} = ?{n}", - col = quote_ident(column), - n = params.len() - ) + RowFilter::Where(expr) => { + // `compile_expr` continues the `?N` numbering from + // the SET params already in `params`. + format!(" WHERE {}", compile_expr(expr, &schema, &mut params)) } }; @@ -4476,14 +4662,8 @@ fn do_delete( let mut params: Vec = Vec::new(); let where_sql = match filter { RowFilter::AllRows => String::new(), - RowFilter::Where { column, value } => { - let bound = impl_value_for(&schema, column, value)?; - params.push(bound_to_sqlite_value(&bound)); - format!( - " WHERE {col} = ?{n}", - col = quote_ident(column), - n = params.len() - ) + RowFilter::Where(expr) => { + format!(" WHERE {}", compile_expr(expr, &schema, &mut params)) } }; let sql = format!( @@ -4536,8 +4716,10 @@ fn do_query_data_request( persistence: Option<&Persistence>, source: Option<&str>, table: &str, + filter: Option<&Expr>, + limit: Option, ) -> Result { - let data = do_query_data(conn, table)?; + let data = do_query_data(conn, table, filter, limit)?; if let (Some(p), Some(text)) = (persistence, source) { p.append_history(text) .map_err(DbError::from_persistence)?; @@ -4545,7 +4727,12 @@ fn do_query_data_request( Ok(data) } -fn do_query_data(conn: &Connection, table: &str) -> Result { +fn do_query_data( + conn: &Connection, + table: &str, + filter: Option<&Expr>, + limit: Option, +) -> Result { let schema = read_schema(conn, table)?; let column_names: Vec = schema.columns.iter().map(|c| c.name.clone()).collect(); let column_types: Vec> = @@ -4556,15 +4743,43 @@ fn do_query_data(conn: &Connection, table: &str) -> Result .map(|c| quote_ident(c)) .collect::>() .join(", "); + + // WHERE / LIMIT (ADR-0026 §5–§6). A `limit` implies a + // stable primary-key ORDER BY so `limit n` is "first n by + // primary key" rather than an arbitrary subset. + let mut params: Vec = Vec::new(); + let where_sql = filter.map_or_else(String::new, |expr| { + format!(" WHERE {}", compile_expr(expr, &schema, &mut params)) + }); + let (order_sql, limit_sql) = match limit { + Some(n) => { + let order = if schema.primary_key.is_empty() { + String::new() + } else { + let pk = schema + .primary_key + .iter() + .map(|c| quote_ident(c)) + .collect::>() + .join(", "); + format!(" ORDER BY {pk}") + }; + params.push(rusqlite::types::Value::Integer( + i64::try_from(n).unwrap_or(i64::MAX), + )); + (order, format!(" LIMIT ?{}", params.len())) + } + None => (String::new(), String::new()), + }; let sql = format!( - "SELECT {cols} FROM {ident};", + "SELECT {cols} FROM {ident}{where_sql}{order_sql}{limit_sql};", cols = cols_csv, ident = quote_ident(table), ); debug!(sql = %sql, "query_data"); let mut stmt = conn.prepare(&sql).map_err(DbError::from_rusqlite)?; let rows_iter = stmt - .query_map([], |row| { + .query_map(rusqlite::params_from_iter(params.iter()), |row| { let mut cells: Vec = Vec::with_capacity(column_names.len()); for i in 0..column_names.len() { let v: rusqlite::types::Value = row.get(i)?; @@ -5300,7 +5515,7 @@ mod tests { result.client_side_notes ); // Verify the column is populated 1..3. - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); let seq_idx = data.columns.iter().position(|c| c == "seq").unwrap(); let mut filled: Vec = data .rows @@ -5333,7 +5548,7 @@ mod tests { result.client_side_notes ); // Verify each row has a non-null shortid value. - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); let tag_idx = data.columns.iter().position(|c| c == "tag").unwrap(); for row in &data.rows { let v = row[tag_idx].as_ref().expect("non-null shortid auto-filled"); @@ -5366,7 +5581,7 @@ mod tests { .await .unwrap(); } - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); let seq_idx = data.columns.iter().position(|c| c == "seq").unwrap(); let mut values: Vec = data .rows @@ -5405,7 +5620,7 @@ mod tests { ) .await .unwrap(); - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); let seq_idx = data.columns.iter().position(|c| c == "seq").unwrap(); let mut values: Vec = data .rows @@ -5520,7 +5735,7 @@ mod tests { // Row data still accessible (id was preserved); the // dropped column is gone from the projection. - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); assert_eq!(data.columns, vec!["id".to_string()]); assert_eq!(data.rows.len(), 1); } @@ -6044,7 +6259,7 @@ mod tests { assert_eq!(note.transformed, 3); assert_eq!(note.lossy, 0); // Data preserved via the per-cell transformer. - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); assert_eq!(data.rows.len(), 3); } @@ -6335,7 +6550,7 @@ mod tests { result.client_side ); // Data preserved. - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); assert_eq!(data.rows.len(), 3); } @@ -6750,7 +6965,7 @@ mod tests { assert_eq!(note.auto_fill_kind, Some(AutoFillKind::Serial)); // Confirm the filled values: existing 5, fills are 6 // and 7 (continue sequence from MAX+1). - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); let code_idx = data.columns.iter().position(|c| c == "code").unwrap(); let mut values: Vec = data .rows @@ -6808,7 +7023,7 @@ mod tests { assert_eq!(note.auto_filled, 2); assert_eq!(note.auto_fill_kind, Some(AutoFillKind::ShortId)); // All three rows now have valid shortids. - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); let tag_idx = data.columns.iter().position(|c| c == "tag").unwrap(); for row in &data.rows { let v = row[tag_idx].as_ref().expect("non-null shortid after fill"); @@ -7334,7 +7549,7 @@ mod tests { // The InsertResult itself carries the just-inserted row. assert_eq!(result.data.rows.len(), 1); assert_eq!(result.data.rows[0][1], Some("Alice".to_string())); - let data = db.query_data("Customers".to_string(), None).await.unwrap(); + let data = db.query_data("Customers".to_string(), None, None, None).await.unwrap(); assert_eq!(data.columns, vec!["id".to_string(), "Name".to_string()]); assert_eq!(data.rows.len(), 1); assert_eq!(data.rows[0][1], Some("Alice".to_string())); @@ -7359,7 +7574,7 @@ mod tests { None) .await .unwrap(); - let data = db.query_data("Tags".to_string(), None).await.unwrap(); + let data = db.query_data("Tags".to_string(), None, None, None).await.unwrap(); let id = data.rows[0][0].as_ref().expect("auto-generated id"); assert!( id.len() >= 10 && id.len() <= 12, @@ -7378,7 +7593,7 @@ mod tests { None) .await .unwrap(); - let data = db.query_data("Customers".to_string(), None).await.unwrap(); + let data = db.query_data("Customers".to_string(), None, None, None).await.unwrap(); assert_eq!(data.rows[0][0], Some("99".to_string())); assert_eq!(data.rows[0][1], Some("Bob".to_string())); } @@ -7416,10 +7631,7 @@ mod tests { .update( "Customers".to_string(), vec![("Name".to_string(), Value::Text("Alicia".to_string()))], - RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + RowFilter::eq("id", Value::Number("1".to_string())), None) .await .unwrap(); @@ -7427,11 +7639,252 @@ mod tests { // The UpdateResult contains only the updated rows. assert_eq!(result.data.rows.len(), 1); assert_eq!(result.data.rows[0][1], Some("Alicia".to_string())); - let data = db.query_data("Customers".to_string(), None).await.unwrap(); + let data = db.query_data("Customers".to_string(), None, None, None).await.unwrap(); assert_eq!(data.rows[0][1], Some("Alicia".to_string())); assert_eq!(data.rows[1][1], Some("Bob".to_string())); } + // ---- Complex WHERE expressions (ADR-0026) ---------------- + + /// `People(id serial pk, Name text, Age int, Active bool)` + /// seeded with four rows: Alice/25/true, Bob/35/false, + /// Carol/45/true, Dave/35/true (ids 1..4). + async fn people_table(db: &Database) { + db.create_table( + "People".to_string(), + vec![ + col("id", Type::Serial), + col("Name", Type::Text), + col("Age", Type::Int), + col("Active", Type::Bool), + ], + vec!["id".to_string()], + None, + ) + .await + .unwrap(); + for (name, age, active) in [ + ("Alice", 25, true), + ("Bob", 35, false), + ("Carol", 45, true), + ("Dave", 35, true), + ] { + db.insert( + "People".to_string(), + None, + vec![ + Value::Text(name.to_string()), + Value::Number(age.to_string()), + Value::Bool(active), + ], + None, + ) + .await + .unwrap(); + } + } + + /// Pull the `RowFilter` out of an `update` / `delete` parsed + /// from DSL — the readable way to build a complex `Expr`. + fn parse_filter(dsl: &str) -> RowFilter { + match crate::dsl::parser::parse_command(dsl).expect("filter parse") { + crate::dsl::command::Command::Update { filter, .. } + | crate::dsl::command::Command::Delete { filter, .. } => filter, + other => panic!("expected update/delete, got {other:?}"), + } + } + + /// Pull the optional filter + limit out of a parsed + /// `show data` command. + fn parse_show(dsl: &str) -> (Option, Option) { + match crate::dsl::parser::parse_command(dsl).expect("show parse") { + crate::dsl::command::Command::ShowData { filter, limit, .. } => { + (filter, limit) + } + other => panic!("expected show data, got {other:?}"), + } + } + + /// The `Name` column of every remaining row, in row order. + fn names(data: &DataResult) -> Vec { + data.rows + .iter() + .map(|r| r[1].clone().unwrap_or_default()) + .collect() + } + + #[tokio::test] + async fn delete_with_and_expression_filters_rows() { + let db = db(); + people_table(&db).await; + let result = db + .delete( + "People".to_string(), + parse_filter( + "delete from People where Age >= 35 and Active = true", + ), + None, + ) + .await + .unwrap(); + // Carol (45/true) and Dave (35/true) match; Bob (35) is + // inactive, Alice (25) is too young. + assert_eq!(result.rows_affected, 2); + let data = db.query_data("People".to_string(), None, None, None).await.unwrap(); + assert_eq!(names(&data), vec!["Alice", "Bob"]); + } + + #[tokio::test] + async fn delete_with_or_expression_filters_rows() { + let db = db(); + people_table(&db).await; + let result = db + .delete( + "People".to_string(), + parse_filter("delete from People where id = 1 or id = 3"), + None, + ) + .await + .unwrap(); + assert_eq!(result.rows_affected, 2); + let data = db.query_data("People".to_string(), None, None, None).await.unwrap(); + assert_eq!(names(&data), vec!["Bob", "Dave"]); + } + + #[tokio::test] + async fn delete_with_not_expression_filters_rows() { + let db = db(); + people_table(&db).await; + // `not Age = 35` keeps Bob and Dave (both 35). + let result = db + .delete( + "People".to_string(), + parse_filter("delete from People where not Age = 35"), + None, + ) + .await + .unwrap(); + assert_eq!(result.rows_affected, 2); + let data = db.query_data("People".to_string(), None, None, None).await.unwrap(); + assert_eq!(names(&data), vec!["Bob", "Dave"]); + } + + #[tokio::test] + async fn delete_with_between_filters_rows() { + let db = db(); + people_table(&db).await; + let result = db + .delete( + "People".to_string(), + parse_filter("delete from People where Age between 30 and 40"), + None, + ) + .await + .unwrap(); + // Bob (35) and Dave (35) are in range. + assert_eq!(result.rows_affected, 2); + let data = db.query_data("People".to_string(), None, None, None).await.unwrap(); + assert_eq!(names(&data), vec!["Alice", "Carol"]); + } + + #[tokio::test] + async fn delete_with_in_filters_rows() { + let db = db(); + people_table(&db).await; + let result = db + .delete( + "People".to_string(), + parse_filter( + "delete from People where Name in ('Alice', 'Carol')", + ), + None, + ) + .await + .unwrap(); + assert_eq!(result.rows_affected, 2); + let data = db.query_data("People".to_string(), None, None, None).await.unwrap(); + assert_eq!(names(&data), vec!["Bob", "Dave"]); + } + + #[tokio::test] + async fn delete_with_like_filters_rows() { + let db = db(); + people_table(&db).await; + let result = db + .delete( + "People".to_string(), + parse_filter("delete from People where Name like 'A%'"), + None, + ) + .await + .unwrap(); + assert_eq!(result.rows_affected, 1, "only Alice matches `A%`"); + let data = db.query_data("People".to_string(), None, None, None).await.unwrap(); + assert_eq!(names(&data), vec!["Bob", "Carol", "Dave"]); + } + + #[tokio::test] + async fn update_with_complex_where_updates_only_matching_rows() { + let db = db(); + people_table(&db).await; + // Deactivate everyone over 30 who is still active. + let result = db + .update( + "People".to_string(), + vec![("Active".to_string(), Value::Bool(false))], + parse_filter( + "update People set Active=false \ + where Age > 30 and Active = true", + ), + None, + ) + .await + .unwrap(); + // Carol (45) and Dave (35) — Bob is already inactive. + assert_eq!(result.rows_affected, 2); + } + + #[tokio::test] + async fn query_data_with_where_filters_rows() { + let db = db(); + people_table(&db).await; + let (filter, limit) = parse_show("show data People where Active = true"); + let data = db + .query_data("People".to_string(), filter, limit, None) + .await + .unwrap(); + assert_eq!(names(&data), vec!["Alice", "Carol", "Dave"]); + } + + #[tokio::test] + async fn query_data_with_limit_caps_rows_by_primary_key() { + let db = db(); + people_table(&db).await; + let (filter, limit) = parse_show("show data People limit 2"); + let data = db + .query_data("People".to_string(), filter, limit, None) + .await + .unwrap(); + // `limit` implies an ORDER BY the primary key, so this + // is a stable "first 2 by id". + assert_eq!(names(&data), vec!["Alice", "Bob"]); + } + + #[tokio::test] + async fn query_data_with_where_and_limit_combines_both() { + let db = db(); + people_table(&db).await; + let (filter, limit) = + parse_show("show data People where Age >= 35 limit 1"); + let data = db + .query_data("People".to_string(), filter, limit, None) + .await + .unwrap(); + // Three rows match `Age >= 35` (Bob, Carol, Dave); the + // limit keeps the first by primary key — Bob (id 2). + assert_eq!(names(&data), vec!["Bob"]); + } + #[tokio::test] async fn update_with_all_rows_affects_everything() { let db = db(); @@ -7473,16 +7926,13 @@ mod tests { let result = db .delete( "Customers".to_string(), - RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + RowFilter::eq("id", Value::Number("1".to_string())), None) .await .unwrap(); assert_eq!(result.rows_affected, 1); assert!(result.cascade.is_empty(), "no children to cascade to"); - let data = db.query_data("Customers".to_string(), None).await.unwrap(); + let data = db.query_data("Customers".to_string(), None, None, None).await.unwrap(); assert_eq!(data.rows.len(), 1); assert_eq!(data.rows[0][1], Some("Bob".to_string())); } @@ -7585,14 +8035,11 @@ mod tests { // Delete Alice — cascades to Orders. db.delete( "Customers".to_string(), - RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + RowFilter::eq("id", Value::Number("1".to_string())), None) .await .unwrap(); - let orders = db.query_data("Orders".to_string(), None).await.unwrap(); + let orders = db.query_data("Orders".to_string(), None, None, None).await.unwrap(); assert!(orders.rows.is_empty(), "child rows should be cascaded"); } @@ -7620,7 +8067,7 @@ mod tests { None) .await .unwrap(); - let data = db.query_data("Flags".to_string(), None).await.unwrap(); + let data = db.query_data("Flags".to_string(), None, None, None).await.unwrap(); assert_eq!(data.rows[0][1], Some("true".to_string())); assert_eq!(data.rows[1][1], Some("false".to_string())); } @@ -7642,7 +8089,7 @@ mod tests { None) .await .unwrap(); - let data = db.query_data("T".to_string(), None).await.unwrap(); + let data = db.query_data("T".to_string(), None, None, None).await.unwrap(); assert_eq!(data.rows[0][1], None); } diff --git a/src/dsl/command.rs b/src/dsl/command.rs index 4ba4827..2e1ccc8 100644 --- a/src/dsl/command.rs +++ b/src/dsl/command.rs @@ -139,8 +139,15 @@ pub enum Command { filter: RowFilter, }, /// Render the rows of a table as a data view in the output. + /// An optional `where` filters rows; an optional `limit` + /// caps the row count (ADR-0026 §5). When `limit` is set the + /// query is implicitly ordered by the table's primary key, + /// so `limit n` is a stable "first n by primary key" rather + /// than an arbitrary subset. ShowData { name: String, + filter: Option, + limit: Option, }, /// Replay a sequence of DSL commands from a file. Each line /// is parsed and dispatched through the same pipeline as @@ -235,10 +242,28 @@ pub enum ChangeColumnMode { /// `--all-rows` flag opt-in for unfiltered operations. #[derive(Debug, Clone, PartialEq, Eq)] pub enum RowFilter { - Where { column: String, value: Value }, + /// Operate on rows matching this WHERE expression + /// (ADR-0026 — a full boolean expression, not just a single + /// equality). + Where(Expr), AllRows, } +impl RowFilter { + /// Build a `Where` filter for a single `column = value` + /// equality. The pre-ADR-0026 grammar produced exactly this + /// shape; the constructor stays as a convenience for + /// callers and tests that only need simple equality. + #[must_use] + pub fn eq(column: impl Into, value: Value) -> Self { + Self::Where(Expr::Predicate(Predicate::Compare { + left: Operand::Column(column.into()), + op: CompareOp::Eq, + right: Operand::Literal(value), + })) + } +} + /// A complex WHERE expression (ADR-0026 §4). /// /// Built by `grammar::expr::build_expr` from the flat @@ -414,7 +439,7 @@ impl Command { Self::CreateTable { name, .. } | Self::DropTable { name } | Self::ShowTable { name } - | Self::ShowData { name } => name, + | Self::ShowData { name, .. } => name, Self::AddColumn { table, .. } | Self::DropColumn { table, .. } | Self::RenameColumn { table, .. } diff --git a/src/dsl/grammar/data.rs b/src/dsl/grammar/data.rs index a350710..a2eef8d 100644 --- a/src/dsl/grammar/data.rs +++ b/src/dsl/grammar/data.rs @@ -16,9 +16,9 @@ //! ready to consume them when the schema reference flows //! through `parse_command`. -use crate::dsl::command::{Command, RowFilter}; +use crate::dsl::command::{Command, Expr, RowFilter}; use crate::dsl::grammar::{ - CommandNode, IdentSource, Node, ValidationError, Word, + CommandNode, IdentSource, Node, NumberValidator, ValidationError, Word, expr, shared::{column_value_list, current_column_value}, }; use crate::dsl::walker::context::WalkContext; @@ -59,7 +59,11 @@ const TABLE_NAME_INSERT: Node = Node::Ident { const SHOW_DATA_NODES: &[Node] = &[ Node::Word(Word::keyword("data")), - TABLE_NAME_EXISTING, + // `writes_table` so the optional `where` expression's + // column slots resolve against this table for completion. + TABLE_NAME_WRITES, + Node::Optional(&WHERE_CLAUSE), + Node::Optional(&LIMIT_CLAUSE), ]; const SHOW_DATA: Node = Node::Seq(SHOW_DATA_NODES); @@ -231,18 +235,6 @@ const SET_COLUMN: Node = Node::Ident { writes_user_listed_column: false, }; -/// Column-name slot in `where col = …` — same writes-column -/// semantics as SET_COLUMN, distinct role for the AST builder. -const FILTER_COLUMN: Node = Node::Ident { - source: IdentSource::Columns, - role: "filter_column", - validator: None, - highlight_override: None, - writes_table: false, - writes_column: true, - writes_user_listed_column: false, -}; - /// Value slot resolved at walk time from /// `WalkContext::current_column`. Falls back to the schemaless /// value-literal choice when no current_column is bound. @@ -260,17 +252,45 @@ const UPDATE_ASSIGNMENTS: Node = Node::Repeated { min: 1, }; +/// `where ` — the complex WHERE-expression fragment +/// (ADR-0026). The grammar tier is defined once in +/// `grammar::expr` and reached here through `Subgrammar`. const WHERE_CLAUSE_NODES: &[Node] = &[ Node::Word(Word::keyword("where")), - FILTER_COLUMN, - Node::Punct('='), - PER_COLUMN_VALUE, + Node::Subgrammar(&expr::OR_EXPR), ]; const WHERE_CLAUSE: Node = Node::Seq(WHERE_CLAUSE_NODES); const FILTER_CHOICES: &[Node] = &[WHERE_CLAUSE, Node::Flag("all-rows")]; const FILTER_CLAUSE: Node = Node::Choice(FILTER_CHOICES); +/// `limit ` — `` is a non-negative integer; the +/// validator rejects fractional / negative literals at parse +/// time (ADR-0026 §5). +fn validate_limit_count(value: &str) -> Result<(), ValidationError> { + if value.parse::().is_ok() { + Ok(()) + } else { + Err(ValidationError { + message_key: "parse.custom.bind_type_mismatch", + args: vec![ + ("found", value.to_string()), + ("expected", "non-negative integer".to_string()), + ], + }) + } +} +const LIMIT_VALIDATOR: NumberValidator = validate_limit_count; + +/// `limit ` clause, optional on `show data` (ADR-0026 §5). +const LIMIT_CLAUSE_NODES: &[Node] = &[ + Node::Word(Word::keyword("limit")), + Node::NumberLit { + validator: Some(LIMIT_VALIDATOR), + }, +]; +const LIMIT_CLAUSE: Node = Node::Seq(LIMIT_CLAUSE_NODES); + const UPDATE_NODES: &[Node] = &[ TABLE_NAME_WRITES, Node::Word(Word::keyword("set")), @@ -335,7 +355,11 @@ fn build_show(path: &MatchedPath) -> Result { .nth(1); let name = require_ident(path, "table_name")?; match sub { - Some("data") => Ok(Command::ShowData { name }), + Some("data") => Ok(Command::ShowData { + name, + filter: build_show_filter(path)?, + limit: build_show_limit(path)?, + }), Some("table") => Ok(Command::ShowTable { name }), _ => Err(ValidationError { message_key: "parse.error_wrapper", @@ -344,6 +368,59 @@ fn build_show(path: &MatchedPath) -> Result { } } +/// The optional `where ` of a `show data`. The expression +/// terminals run from just past `Word("where")` to the start of +/// the `limit` clause (or the end of the path) — neither the +/// `limit` keyword nor any expression keyword collide, so the +/// slice is exact. +fn build_show_filter(path: &MatchedPath) -> Result, ValidationError> { + let Some(where_idx) = path + .items + .iter() + .position(|i| matches!(&i.kind, MatchedKind::Word("where"))) + else { + return Ok(None); + }; + let end = path + .items + .iter() + .position(|i| matches!(&i.kind, MatchedKind::Word("limit"))) + .unwrap_or(path.items.len()); + Ok(Some(expr::build_expr(&path.items[where_idx + 1..end])?)) +} + +/// The optional `limit ` of a `show data`. The grammar's +/// `LIMIT_VALIDATOR` already constrained `` to a +/// non-negative integer, so the parse here cannot realistically +/// fail. +fn build_show_limit(path: &MatchedPath) -> Result, ValidationError> { + let Some(limit_idx) = path + .items + .iter() + .position(|i| matches!(&i.kind, MatchedKind::Word("limit"))) + else { + return Ok(None); + }; + let count = path + .items + .get(limit_idx + 1) + .ok_or_else(|| ValidationError { + message_key: "parse.error_wrapper", + args: vec![("detail", "missing limit count".to_string())], + })?; + count + .text + .parse::() + .map(Some) + .map_err(|_| ValidationError { + message_key: "parse.custom.bind_type_mismatch", + args: vec![ + ("found", count.text.clone()), + ("expected", "non-negative integer".to_string()), + ], + }) +} + fn build_insert(path: &MatchedPath) -> Result { let table = require_ident(path, "table_name")?; @@ -575,37 +652,19 @@ fn collect_filter(path: &MatchedPath) -> Result { { return Ok(RowFilter::AllRows); } - // Walk for filter_column ident, then `=`, then value. - let mut iter = path.items.iter(); - while let Some(item) = iter.next() { - if matches!( - item.kind, - MatchedKind::Ident { - role: "filter_column" - } - ) { - let column = item.text.clone(); - // Skip until `=`. - for next in iter.by_ref() { - if matches!(next.kind, MatchedKind::Punct('=')) { - break; - } - } - let value_item = iter.next().ok_or_else(|| ValidationError { - message_key: "parse.error_wrapper", - args: vec![("detail", "missing where value".to_string())], - })?; - let value = item_to_value(value_item).ok_or_else(|| ValidationError { - message_key: "parse.error_wrapper", - args: vec![("detail", "expected value literal".to_string())], - })?; - return Ok(RowFilter::Where { column, value }); - } - } - Err(ValidationError { - message_key: "parse.error_wrapper", - args: vec![("detail", "missing where or --all-rows".to_string())], - }) + let where_idx = path + .items + .iter() + .position(|i| matches!(&i.kind, MatchedKind::Word("where"))) + .ok_or_else(|| ValidationError { + message_key: "parse.error_wrapper", + args: vec![("detail", "missing where or --all-rows".to_string())], + })?; + // `where` is the last clause of update / delete, so every + // terminal after it belongs to the expression. + Ok(RowFilter::Where(expr::build_expr( + &path.items[where_idx + 1..], + )?)) } fn build_delete(path: &MatchedPath) -> Result { diff --git a/src/dsl/grammar/expr.rs b/src/dsl/grammar/expr.rs index e16c6be..b458abe 100644 --- a/src/dsl/grammar/expr.rs +++ b/src/dsl/grammar/expr.rs @@ -53,8 +53,9 @@ //! `build_expr` owns the tree shape. use crate::dsl::command::{CompareOp, Expr, Operand, Predicate}; -use crate::dsl::grammar::{IdentSource, Node, ValidationError, Word}; +use crate::dsl::grammar::{HintMode, IdentSource, Node, ValidationError, Word}; use crate::dsl::value::Value; +use crate::dsl::walker::context::WalkContext; use crate::dsl::walker::outcome::{MatchedItem, MatchedKind}; // ================================================================= @@ -64,19 +65,27 @@ use crate::dsl::walker::outcome::{MatchedItem, MatchedKind}; /// A column reference inside an expression. The `expr_column` /// role lets [`build_expr`] (and the command AST builders) tell /// an expression column apart from other identifier slots. +/// +/// `writes_column` so that, once a predicate's left operand +/// resolves to a known column, the walker records it in +/// `WalkContext::current_column` — the right operand's +/// schema-aware slot (`where_rhs_operand`) reads it to narrow +/// the hint panel to that column's type (ADR-0026 §8). const EXPR_COLUMN: Node = Node::Ident { source: IdentSource::Columns, role: "expr_column", validator: None, highlight_override: None, writes_table: false, - writes_column: false, + writes_column: true, writes_user_listed_column: false, }; /// Operand alternatives. The literal keywords (`null` / `true` /// / `false`) come before the column slot so they parse as -/// literals; any other identifier is a column reference. +/// literals; any other identifier is a column reference. No +/// validators — a type-mismatched literal in a comparison is +/// flagged in the editor but still parses (ADR-0026 §7). static OPERAND_CHOICES: &[Node] = &[ Node::Word(Word::keyword("null")), Node::Word(Word::keyword("true")), @@ -86,6 +95,45 @@ static OPERAND_CHOICES: &[Node] = &[ EXPR_COLUMN, ]; +/// The operand alternatives as a single node — the permissive +/// inner of the schema-aware right-hand operand slot. +static OPERAND_NODE: Node = Node::Choice(OPERAND_CHOICES); + +/// The right-hand operand of a predicate — the comparison RHS, +/// a `LIKE` pattern, the `BETWEEN` bounds, the `IN` items — +/// resolved at walk time (ADR-0026 §8). +/// +/// When the predicate's left operand resolved to a known +/// column, this wraps the operand in a `TypedValueSlot` keyed +/// on that column's user-facing type, so the hint panel +/// narrows to the column's type and names the column. +/// Otherwise a generic value-literal `Hinted` slot. Either way +/// the inner grammar is the *permissive* operand choice +/// (`OPERAND_NODE`) — a type-mismatched literal still matches +/// (ADR-0026 §7); the mismatch is an editor flag, never a +/// parse failure. +fn where_rhs_operand(ctx: &WalkContext) -> Node { + ctx.current_column.as_ref().map_or_else( + || Node::Hinted { + mode: HintMode::ProseOnly("hint.value_literal_slot"), + inner: &OPERAND_NODE, + }, + |col| { + // `Box::leak` mirrors `shared::slot_for_column` — + // the leak is per distinct column (the walker + // memoizes `DynamicSubgrammar` resolution on + // `current_column`), not per keystroke. + let leaked: &'static str = + Box::leak(col.name.clone().into_boxed_str()); + Node::TypedValueSlot { + ty: col.user_type, + column_name: Some(leaked), + inner: &OPERAND_NODE, + } + }, + ) +} + // ================================================================= // cmp_op := <= | <> | >= | != | < | > | = // ================================================================= @@ -110,10 +158,12 @@ static CMP_OP_CHOICES: &[Node] = &[ // predicate_tail branches // ================================================================= -/// `cmp_op operand`. +/// `cmp_op operand`. The right operand is the schema-aware +/// `where_rhs_operand` so the hint panel can narrow to the +/// left column's type. static COMPARE_FORM_NODES: &[Node] = &[ Node::Choice(CMP_OP_CHOICES), - Node::Choice(OPERAND_CHOICES), + Node::DynamicSubgrammar(where_rhs_operand), ]; /// `IS [NOT] NULL`. @@ -126,7 +176,7 @@ static IS_NULL_NODES: &[Node] = &[ /// `LIKE operand`. static LIKE_FORM_NODES: &[Node] = &[ Node::Word(Word::keyword("like")), - Node::Choice(OPERAND_CHOICES), + Node::DynamicSubgrammar(where_rhs_operand), ]; /// `BETWEEN operand AND operand`. The inner `and` is consumed @@ -134,9 +184,9 @@ static LIKE_FORM_NODES: &[Node] = &[ /// connective. static BETWEEN_FORM_NODES: &[Node] = &[ Node::Word(Word::keyword("between")), - Node::Choice(OPERAND_CHOICES), + Node::DynamicSubgrammar(where_rhs_operand), Node::Word(Word::keyword("and")), - Node::Choice(OPERAND_CHOICES), + Node::DynamicSubgrammar(where_rhs_operand), ]; /// `IN ( operand [, operand]* )`. @@ -144,7 +194,7 @@ static IN_FORM_NODES: &[Node] = &[ Node::Word(Word::keyword("in")), Node::Punct('('), Node::Repeated { - inner: &Node::Choice(OPERAND_CHOICES), + inner: &Node::DynamicSubgrammar(where_rhs_operand), separator: Some(&Node::Punct(',')), min: 1, }, diff --git a/src/dsl/mod.rs b/src/dsl/mod.rs index 7f9c916..5812b4a 100644 --- a/src/dsl/mod.rs +++ b/src/dsl/mod.rs @@ -20,8 +20,8 @@ pub mod walker; pub use action::ReferentialAction; pub use command::{ - AppCommand, ChangeColumnMode, ColumnSpec, Command, IndexSelector, MessagesValue, - ModeValue, RelationshipSelector, RowFilter, + AppCommand, ChangeColumnMode, ColumnSpec, Command, CompareOp, Expr, IndexSelector, + MessagesValue, ModeValue, Operand, Predicate, RelationshipSelector, RowFilter, }; pub use parser::{ParseError, parse_command}; pub use types::Type; diff --git a/src/dsl/parser.rs b/src/dsl/parser.rs index 8d68fea..2fc0bf0 100644 --- a/src/dsl/parser.rs +++ b/src/dsl/parser.rs @@ -1039,10 +1039,7 @@ mod tests { Command::Update { table: "Customers".to_string(), assignments: vec![("Name".to_string(), Value::Text("Alice".to_string()))], - filter: RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + filter: RowFilter::eq("id", Value::Number("1".to_string())), } ); } @@ -1057,10 +1054,7 @@ mod tests { ("Name".to_string(), Value::Text("Alice".to_string())), ("Email".to_string(), Value::Text("a@b.com".to_string())), ], - filter: RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + filter: RowFilter::eq("id", Value::Number("1".to_string())), } ); } @@ -1089,10 +1083,7 @@ mod tests { ok("delete from Customers where id=1"), Command::Delete { table: "Customers".to_string(), - filter: RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + filter: RowFilter::eq("id", Value::Number("1".to_string())), } ); } @@ -1119,7 +1110,9 @@ mod tests { assert_eq!( ok("show data Customers"), Command::ShowData { - name: "Customers".to_string() + name: "Customers".to_string(), + filter: None, + limit: None, } ); } diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index ae92774..8184f60 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -229,6 +229,15 @@ pub struct CompletionProbe { /// earlier in the walk). `None` when the walker is /// schemaless or the table didn't resolve. pub current_table_columns: Option>, + /// The grammar-declared `HintMode` at the cursor's slot + /// (`Node::Hinted`), if any. A `ProseOnly` slot tells the + /// completion engine to suppress its keyword candidates — + /// the node-attached signal that supersedes the + /// expected-set signature heuristic where the grammar + /// explicitly marks a slot prose-only (e.g. the + /// WHERE-expression operand, which also accepts a column + /// reference — ADR-0026 §8). + pub pending_hint_mode: Option, } /// Run a schema-aware walk and report the completion-engine's @@ -247,6 +256,7 @@ pub fn completion_probe( .map(|c| outcome::Expectation::Word(c.entry.primary)) .collect(), current_table_columns: None, + pending_hint_mode: None, }; } let mut ctx = context::WalkContext::with_schema(schema); @@ -258,6 +268,7 @@ pub fn completion_probe( .map(|c| outcome::Expectation::Word(c.entry.primary)) .collect(), current_table_columns: None, + pending_hint_mode: None, }; }; let expected = match result.outcome { @@ -276,6 +287,7 @@ pub fn completion_probe( CompletionProbe { expected, current_table_columns: ctx.current_table_columns, + pending_hint_mode: ctx.pending_hint_mode, } } @@ -1155,7 +1167,9 @@ mod tests { assert_eq!( parse("show data Customers").unwrap(), Command::ShowData { - name: "Customers".to_string() + name: "Customers".to_string(), + filter: None, + limit: None, } ); } @@ -1170,6 +1184,58 @@ mod tests { ); } + #[test] + fn walker_parses_show_data_with_where_and_limit() { + // ADR-0026 §5: `show data` gains an optional `where` + // and an optional `limit `. + match parse("show data Customers where id=1 limit 10").unwrap() { + Command::ShowData { + name, + filter: Some(_), + limit: Some(10), + } => assert_eq!(name, "Customers"), + other => panic!("expected ShowData with filter + limit, got {other:?}"), + } + } + + #[test] + fn walker_parses_show_data_with_limit_only() { + assert!(matches!( + parse("show data Customers limit 5").unwrap(), + Command::ShowData { + filter: None, + limit: Some(5), + .. + } + )); + } + + #[test] + fn walker_parses_update_with_complex_where() { + // The WHERE is a full boolean expression, not a single + // equality (ADR-0026). + match parse("update T set Active=true where Age>30 and Name like 'A%'") + .unwrap() + { + Command::Update { + filter: RowFilter::Where(crate::dsl::Expr::And(terms)), + .. + } => assert_eq!(terms.len(), 2, "two AND-ed predicates"), + other => panic!("expected Update with And-expression filter, got {other:?}"), + } + } + + #[test] + fn walker_parses_delete_with_or_where() { + assert!(matches!( + parse("delete from T where id=1 or id=2").unwrap(), + Command::Delete { + filter: RowFilter::Where(crate::dsl::Expr::Or(_)), + .. + } + )); + } + #[test] fn walker_parses_insert_with_explicit_column_list() { assert_eq!( @@ -1233,10 +1299,7 @@ mod tests { Command::Update { table: "Customers".to_string(), assignments: vec![("Email".to_string(), Value::Text("new@b.c".to_string()))], - filter: RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + filter: RowFilter::eq("id", Value::Number("1".to_string())), } ); } @@ -1251,10 +1314,7 @@ mod tests { ("Email".to_string(), Value::Text("a@b.c".to_string())), ("Name".to_string(), Value::Text("Alice".to_string())), ], - filter: RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + filter: RowFilter::eq("id", Value::Number("1".to_string())), } ); } @@ -1277,10 +1337,7 @@ mod tests { parse("delete from Customers where id=42").unwrap(), Command::Delete { table: "Customers".to_string(), - filter: RowFilter::Where { - column: "id".to_string(), - value: Value::Number("42".to_string()), - }, + filter: RowFilter::eq("id", Value::Number("42".to_string())), } ); } @@ -1708,20 +1765,15 @@ mod tests { } #[test] - fn phase_d_delete_where_rejects_decimal_at_int_column() { - // `where id=3.14` — id is Int; the typed slot rejects. + fn phase_d_delete_where_permits_decimal_at_int_column() { + // ADR-0026 §7: a type-mismatched WHERE comparison is + // flagged in the editor but never blocks. `id` is Int + // and `3.14` is not — yet the command still parses and + // would run (this relaxes the pre-ADR-0026 rejection). let schema = schema_with("T", &[("id", Type::Int)]); - let err = parse_command_with_schema("delete from T where id=3.14", &schema) - .expect_err("should reject"); - match err { - crate::dsl::ParseError::Invalid { message, .. } => { - assert!( - message.contains("integer") || message.contains("3.14"), - "got: {message}" - ); - } - other => panic!("expected Invalid, got {other:?}"), - } + let cmd = parse_command_with_schema("delete from T where id=3.14", &schema) + .expect("type-mismatched WHERE comparisons are permissive"); + assert!(matches!(cmd, crate::dsl::Command::Delete { .. }), "got {cmd:?}"); } // ---- Typed-slot HintMode (Phase D + HintMode dispatch) ---- diff --git a/src/runtime.rs b/src/runtime.rs index 90ce016..d8a3dfe 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1674,8 +1674,12 @@ async fn execute_command_typed( .delete(table, filter, src) .await .map(CommandOutcome::Delete), - Command::ShowData { name } => database - .query_data(name, src) + Command::ShowData { + name, + filter, + limit, + } => database + .query_data(name, filter, limit, src) .await .map(CommandOutcome::Query), // `replay` is parsed as a DSL command but routed by diff --git a/tests/friendly_enrichment.rs b/tests/friendly_enrichment.rs index 7d7366b..f7f1a83 100644 --- a/tests/friendly_enrichment.rs +++ b/tests/friendly_enrichment.rs @@ -181,19 +181,13 @@ fn enrich_unique_update_resolves_value_from_assignments() { let cmd = Command::Update { table: "Customers".to_string(), assignments: vec![("id".to_string(), Value::Number("1".to_string()))], - filter: RowFilter::Where { - column: "name".to_string(), - value: Value::Text("Bob".to_string()), - }, + filter: RowFilter::eq("name", Value::Text("Bob".to_string())), }; let err = db .update( "Customers".to_string(), vec![("id".to_string(), Value::Number("1".to_string()))], - RowFilter::Where { - column: "name".to_string(), - value: Value::Text("Bob".to_string()), - }, + RowFilter::eq("name", Value::Text("Bob".to_string())), None, ) .await @@ -464,18 +458,12 @@ fn enrich_fk_delete_resolves_child_table() { // Delete the parent that has children — engine refuses. let cmd = Command::Delete { table: "Customers".to_string(), - filter: RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + filter: RowFilter::eq("id", Value::Number("1".to_string())), }; let err = db .delete( "Customers".to_string(), - RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + RowFilter::eq("id", Value::Number("1".to_string())), None, ) .await diff --git a/tests/iteration2_persistence.rs b/tests/iteration2_persistence.rs index 8f5c0da..d56e46a 100644 --- a/tests/iteration2_persistence.rs +++ b/tests/iteration2_persistence.rs @@ -229,10 +229,7 @@ fn delete_with_cascade_rewrites_both_csvs() { let result = db .delete( "Customers".to_string(), - RowFilter::Where { - column: "id".to_string(), - value: Value::Number("1".to_string()), - }, + RowFilter::eq("id", Value::Number("1".to_string())), Some("delete from Customers where id=1".to_string()), ) .await diff --git a/tests/iteration3_rebuild.rs b/tests/iteration3_rebuild.rs index d7fab69..18c11b4 100644 --- a/tests/iteration3_rebuild.rs +++ b/tests/iteration3_rebuild.rs @@ -143,7 +143,7 @@ fn rebuild_restores_rows_from_csv() { }); let rows = rt() - .block_on(async { db.query_data("Customers".to_string(), None).await }) + .block_on(async { db.query_data("Customers".to_string(), None, None, None).await }) .expect("query_data"); assert_eq!(rows.rows.len(), 2); let names: Vec> = rows.rows.iter().map(|r| r[1].clone()).collect(); diff --git a/tests/iteration4a_rebuild_command.rs b/tests/iteration4a_rebuild_command.rs index 094d363..7670006 100644 --- a/tests/iteration4a_rebuild_command.rs +++ b/tests/iteration4a_rebuild_command.rs @@ -173,7 +173,7 @@ fn rebuild_against_populated_db_wipes_and_reloads() { .expect("rebuild"); }); let rows = rt() - .block_on(async { db.query_data("Customers".to_string(), None).await }) + .block_on(async { db.query_data("Customers".to_string(), None, None, None).await }) .unwrap(); assert_eq!(rows.rows.len(), 1); assert_eq!(rows.rows[0][1].as_deref(), Some("Edna")); diff --git a/tests/iteration5_export_import.rs b/tests/iteration5_export_import.rs index d4af456..84bc7dd 100644 --- a/tests/iteration5_export_import.rs +++ b/tests/iteration5_export_import.rs @@ -357,7 +357,7 @@ fn end_to_end_export_then_import_real_project() { // Round-trip: the inserted row is back. let data_view = rt() - .block_on(async { imported_db.query_data("Customers".to_string(), None).await }) + .block_on(async { imported_db.query_data("Customers".to_string(), None, None, None).await }) .expect("query data"); assert_eq!(data_view.rows.len(), 1); // Serial id auto-filled to 1; Name was the inserted value. diff --git a/tests/replay_command.rs b/tests/replay_command.rs index 72f910a..e621974 100644 --- a/tests/replay_command.rs +++ b/tests/replay_command.rs @@ -107,7 +107,7 @@ fn replay_three_lines_dispatches_three_commands() { // The dispatched commands actually mutated state. let data_result = rt() - .block_on(async { db.query_data("T".to_string(), None).await }) + .block_on(async { db.query_data("T".to_string(), None, None, None).await }) .expect("query_data"); assert_eq!(data_result.rows.len(), 1, "row inserted"); assert_eq!(data_result.rows[0][1].as_deref(), Some("Alice")); @@ -219,7 +219,7 @@ fn replay_aborts_on_first_parse_failure_and_reports_line() { "earlier add column should have stayed applied" ); let data_result = rt() - .block_on(async { db.query_data("T".to_string(), None).await }) + .block_on(async { db.query_data("T".to_string(), None, None, None).await }) .expect("query_data"); assert!( data_result.rows.is_empty(), @@ -268,7 +268,7 @@ fn replay_rejects_typed_slot_violation_at_parse_time() { // The earlier two lines stayed applied; the failing insert // did not run. let data_result = rt() - .block_on(async { db.query_data("T".to_string(), None).await }) + .block_on(async { db.query_data("T".to_string(), None, None, None).await }) .expect("query_data"); assert!( data_result.rows.is_empty(), diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap index 5391ec0..8544c85 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap @@ -24,6 +24,26 @@ Assessment { text: "null", kind: Keyword, }, + Candidate { + text: "true", + kind: Keyword, + }, + Candidate { + text: "false", + kind: Keyword, + }, + Candidate { + text: "Email", + kind: Identifier, + }, + Candidate { + text: "Name", + kind: Identifier, + }, + Candidate { + text: "id", + kind: Identifier, + }, ], }, ), diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_offers_active_table_columns_no_leakage@after_where.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_offers_active_table_columns_no_leakage@after_where.snap index 2b3d80f..0393f41 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_offers_active_table_columns_no_leakage@after_where.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__after_where_offers_active_table_columns_no_leakage@after_where.snap @@ -10,6 +10,26 @@ Assessment { hint: Some( Candidates { items: [ + Candidate { + text: "not", + kind: Keyword, + }, + Candidate { + text: "null", + kind: Keyword, + }, + Candidate { + text: "true", + kind: Keyword, + }, + Candidate { + text: "false", + kind: Keyword, + }, + Candidate { + text: "(", + kind: Punct, + }, Candidate { text: "Name", kind: Identifier, @@ -30,6 +50,26 @@ Assessment { ), partial_prefix: "", candidates: [ + Candidate { + text: "not", + kind: Keyword, + }, + Candidate { + text: "null", + kind: Keyword, + }, + Candidate { + text: "true", + kind: Keyword, + }, + Candidate { + text: "false", + kind: Keyword, + }, + Candidate { + text: "(", + kind: Punct, + }, Candidate { text: "Name", kind: Identifier, diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__delete_with_datetime_column_says_yyyy_mm_dd_t@datetime_column.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__delete_with_datetime_column_says_yyyy_mm_dd_t@datetime_column.snap index af8524f..3ec3f98 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__delete_with_datetime_column_says_yyyy_mm_dd_t@datetime_column.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__delete_with_where__delete_with_datetime_column_says_yyyy_mm_dd_t@datetime_column.snap @@ -24,6 +24,54 @@ Assessment { text: "null", kind: Keyword, }, + Candidate { + text: "true", + kind: Keyword, + }, + Candidate { + text: "false", + kind: Keyword, + }, + Candidate { + text: "auto", + kind: Identifier, + }, + Candidate { + text: "b", + kind: Identifier, + }, + Candidate { + text: "d", + kind: Identifier, + }, + Candidate { + text: "data", + kind: Identifier, + }, + Candidate { + text: "dt", + kind: Identifier, + }, + Candidate { + text: "k", + kind: Identifier, + }, + Candidate { + text: "note", + kind: Identifier, + }, + Candidate { + text: "r", + kind: Identifier, + }, + Candidate { + text: "sid", + kind: Identifier, + }, + Candidate { + text: "ts", + kind: Identifier, + }, ], }, ), diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap index c7f500c..7291101 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_column_equals_offers_typed_prose@after_where_equals.snap @@ -24,6 +24,26 @@ Assessment { text: "null", kind: Keyword, }, + Candidate { + text: "true", + kind: Keyword, + }, + Candidate { + text: "false", + kind: Keyword, + }, + Candidate { + text: "Email", + kind: Identifier, + }, + Candidate { + text: "Name", + kind: Identifier, + }, + Candidate { + text: "id", + kind: Identifier, + }, ], }, ), diff --git a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_keyword_offers_active_table_columns@after_where.snap b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_keyword_offers_active_table_columns@after_where.snap index 74220c1..51fdc81 100644 --- a/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_keyword_offers_active_table_columns@after_where.snap +++ b/tests/typing_surface/snapshots/typing_surface_matrix__typing_surface__update_with_where__after_where_keyword_offers_active_table_columns@after_where.snap @@ -10,6 +10,26 @@ Assessment { hint: Some( Candidates { items: [ + Candidate { + text: "not", + kind: Keyword, + }, + Candidate { + text: "null", + kind: Keyword, + }, + Candidate { + text: "true", + kind: Keyword, + }, + Candidate { + text: "false", + kind: Keyword, + }, + Candidate { + text: "(", + kind: Punct, + }, Candidate { text: "Name", kind: Identifier, @@ -30,6 +50,26 @@ Assessment { ), partial_prefix: "", candidates: [ + Candidate { + text: "not", + kind: Keyword, + }, + Candidate { + text: "null", + kind: Keyword, + }, + Candidate { + text: "true", + kind: Keyword, + }, + Candidate { + text: "false", + kind: Keyword, + }, + Candidate { + text: "(", + kind: Punct, + }, Candidate { text: "Name", kind: Identifier, diff --git a/tests/walking_skeleton.rs b/tests/walking_skeleton.rs index 4871904..829bd37 100644 --- a/tests/walking_skeleton.rs +++ b/tests/walking_skeleton.rs @@ -561,6 +561,8 @@ fn show_data_for_empty_table_renders_placeholder() { app.update(AppEvent::DslDataSucceeded { command: Command::ShowData { name: "Customers".to_string(), + filter: None, + limit: None, }, data, });