From fd8b74ba5e168a600c83bad2efd8b7f127a7428f Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Fri, 22 May 2026 20:44:55 +0000 Subject: [PATCH] =?UTF-8?q?grammar+db:=203g=20=E2=80=94=20RETURNING=20on?= =?UTF-8?q?=20INSERT/UPDATE/DELETE=20(ADR-0033=20=C2=A75)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shared RETURNING_CLAUSE (reuses Phase-2 PROJECTION_LIST, now pub(crate)) as an optional tail on all three SQL DML shapes. `returning: bool` on the Command variants, set by the ast-builders and threaded to the worker. run_returning collects the returned rows as a DataResult (RETURNING mutates + yields in one pass), reusing resolve_select_column_types for bare-column type recovery; computed projections stay typeless. DeleteResult gains a `data` field rendered alongside the cascade summary. Follow-set fix: `returning` is added to the table-source and projection bare-alias follow-sets so an INSERT … SELECT row source stops before RETURNING instead of reading it as a table alias. Auto-fill × RETURNING: build_sql_insert stops row_source before the RETURNING token (keeping it preparable for shortid materialisation), and plan_shortid_autofill re-appends the RETURNING tail so generated shortids surface in RETURNING *. Tests (+17): grammar accept on all three; INSERT/UPDATE/DELETE RETURNING incl. *, aliases, multi-row, type recovery + computed- typeless; auto-fill × RETURNING (single + multi-row distinct ids); INSERT…SELECT…RETURNING execution; UPDATE…RETURNING zero-match; DELETE…RETURNING cascade+rows; app-level render of both. Dev sql_insert/sql_update/sql_delete entry words still removed in 3j. 1562 pass / 0 fail / 1 ignored. Clippy clean. --- src/app.rs | 60 ++++++++++ src/db.rs | 203 ++++++++++++++++++++++++++++++---- src/dsl/command.rs | 12 ++ src/dsl/grammar/data.rs | 44 ++++++-- src/dsl/grammar/sql_delete.rs | 11 +- src/dsl/grammar/sql_insert.rs | 14 ++- src/dsl/grammar/sql_select.rs | 25 ++++- src/dsl/grammar/sql_update.rs | 11 +- src/runtime.rs | 19 +++- tests/sql_delete.rs | 46 +++++++- tests/sql_insert.rs | 183 ++++++++++++++++++++++++++++++ tests/sql_update.rs | 55 ++++++++- 12 files changed, 637 insertions(+), 46 deletions(-) diff --git a/src/app.rs b/src/app.rs index 4ab0802..4bd6005 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1355,6 +1355,15 @@ impl App { for effect in &result.cascade { self.note_system(render_cascade_effect(effect)); } + // A `RETURNING` clause (ADR-0033 §5, 3g) carries the deleted + // rows; the cascade summary above surfaces alongside them. A + // column-less result (the DSL `delete` and SQL `DELETE` + // without RETURNING) is skipped, exactly as for UPDATE. + if !result.data.columns.is_empty() { + for line in crate::output_render::render_data_table(&result.data) { + self.note_system(line); + } + } } fn handle_dsl_failure( @@ -3365,6 +3374,7 @@ mod tests { command: Command::SqlUpdate { sql: "update t set v = 1".to_string(), target_table: "t".to_string(), + returning: false, }, result: crate::db::UpdateResult { rows_affected: 2, @@ -3396,6 +3406,7 @@ mod tests { command: Command::SqlUpdate { sql: "update t set v = 1".to_string(), target_table: "t".to_string(), + returning: false, }, result: crate::db::UpdateResult { rows_affected: 1, @@ -3429,6 +3440,7 @@ mod tests { command: Command::SqlDelete { sql: "delete from Customers where id = 1".to_string(), target_table: "Customers".to_string(), + returning: false, }, result: crate::db::DeleteResult { rows_affected: 1, @@ -3438,6 +3450,12 @@ mod tests { rows_changed: 2, action: ReferentialAction::Cascade, }], + data: crate::db::DataResult { + table_name: "Customers".to_string(), + columns: Vec::new(), + column_types: Vec::new(), + rows: Vec::new(), + }, }, }); let texts: Vec = app.output.iter().map(|l| l.text.clone()).collect(); @@ -3455,4 +3473,46 @@ mod tests { "per-relationship cascade summary surfaced: {texts:?}", ); } + + #[test] + fn sql_delete_returning_renders_cascade_and_result_table() { + // ADR-0033 3g: a DELETE … RETURNING surfaces BOTH the cascade + // summary AND the returned-rows table. Pins the render branch + // that tabulates `result.data` when RETURNING populated it + // (the column-less non-RETURNING path is skipped — see the + // sibling test above). + use crate::dsl::ReferentialAction; + let mut app = App::new(); + app.update(AppEvent::DslDeleteSucceeded { + command: Command::SqlDelete { + sql: "delete from Customers where id = 1 returning *".to_string(), + target_table: "Customers".to_string(), + returning: true, + }, + result: crate::db::DeleteResult { + rows_affected: 1, + cascade: vec![crate::db::CascadeEffect { + relationship_name: "places".to_string(), + child_table: "Orders".to_string(), + rows_changed: 2, + action: ReferentialAction::Cascade, + }], + data: crate::db::DataResult { + table_name: "Customers".to_string(), + columns: vec!["id".to_string(), "Name".to_string()], + column_types: vec![Some(Type::Int), Some(Type::Text)], + rows: vec![vec![Some("1".to_string()), Some("Alice".to_string())]], + }, + }, + }); + let texts: Vec = app.output.iter().map(|l| l.text.clone()).collect(); + assert!( + texts.iter().any(|t| t.contains("2 row(s) deleted in `Orders`")), + "cascade summary still surfaces alongside RETURNING: {texts:?}", + ); + assert!( + texts.iter().any(|t| t.contains("Name")) && texts.iter().any(|t| t.contains("Alice")), + "the returned (deleted) row is tabulated: {texts:?}", + ); + } } diff --git a/src/db.rs b/src/db.rs index 27b4f6c..a766aac 100644 --- a/src/db.rs +++ b/src/db.rs @@ -351,6 +351,13 @@ pub struct UpdateResult { pub struct DeleteResult { pub rows_affected: usize, pub cascade: Vec, + /// Rows produced by a `RETURNING` clause (ADR-0033 §5, 3g). + /// Empty (no columns, no rows) when the DELETE had no + /// `RETURNING` — the renderer skips a column-less result, so the + /// non-RETURNING path is unaffected. For SQL `DELETE … RETURNING` + /// these are the rows as they were *before* deletion; the + /// cascade summary surfaces alongside. + pub data: DataResult, } /// One observed change in a child table caused by referential @@ -593,6 +600,7 @@ enum Request { target_table: String, listed_columns: Vec, row_source: String, + returning: bool, reply: oneshot::Sender>, }, /// Run a grammar-validated SQL `UPDATE` (ADR-0033 §2). The @@ -603,6 +611,7 @@ enum Request { sql: String, source: Option, target_table: String, + returning: bool, reply: oneshot::Sender>, }, /// Run a grammar-validated SQL `DELETE` (ADR-0033 §1/§7). The @@ -615,6 +624,7 @@ enum Request { sql: String, source: Option, target_table: String, + returning: bool, reply: oneshot::Sender>, }, /// Capture the query plan for an explainable command via @@ -1086,6 +1096,7 @@ impl Database { target_table: String, listed_columns: Vec, row_source: String, + returning: bool, ) -> Result { let (reply, recv) = oneshot::channel(); self.send(Request::RunSqlInsert { @@ -1094,6 +1105,7 @@ impl Database { target_table, listed_columns, row_source, + returning, reply, }) .await?; @@ -1110,12 +1122,14 @@ impl Database { sql: String, source: Option, target_table: String, + returning: bool, ) -> Result { let (reply, recv) = oneshot::channel(); self.send(Request::RunSqlUpdate { sql, source, target_table, + returning, reply, }) .await?; @@ -1133,12 +1147,14 @@ impl Database { sql: String, source: Option, target_table: String, + returning: bool, ) -> Result { let (reply, recv) = oneshot::channel(); self.send(Request::RunSqlDelete { sql, source, target_table, + returning, reply, }) .await?; @@ -1596,6 +1612,7 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req target_table, listed_columns, row_source, + returning, reply, } => { let _ = reply.send(do_sql_insert( @@ -1606,12 +1623,14 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req &target_table, &listed_columns, &row_source, + returning, )); } Request::RunSqlUpdate { sql, source, target_table, + returning, reply, } => { let _ = reply.send(do_sql_update( @@ -1620,12 +1639,14 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req source.as_deref(), &sql, &target_table, + returning, )); } Request::RunSqlDelete { sql, source, target_table, + returning, reply, } => { let _ = reply.send(do_sql_delete( @@ -1634,6 +1655,7 @@ fn handle_request(conn: &Connection, persistence: Option<&Persistence>, req: Req source.as_deref(), &sql, &target_table, + returning, )); } Request::RebuildFromText { @@ -5826,6 +5848,14 @@ fn do_delete( Ok(DeleteResult { rows_affected, cascade, + // The DSL `delete` has no RETURNING (a SQL-only clause); the + // empty result is skipped by the renderer (ADR-0033 §5, 3g). + data: DataResult { + table_name: table.to_string(), + columns: Vec::new(), + column_types: Vec::new(), + rows: Vec::new(), + }, }) } @@ -5915,6 +5945,7 @@ fn plan_shortid_autofill( sql: &str, listed_columns: &[String], row_source: &str, + returning_tail: &str, ) -> Result<(String, Vec), DbError> { if listed_columns.is_empty() { return Ok((sql.to_string(), Vec::new())); @@ -6009,8 +6040,18 @@ fn plan_shortid_autofill( .join(", "); tuples.push(format!("({placeholders})")); } + // Preserve any RETURNING tail (3g) — the reconstruction would + // otherwise drop it, so `INSERT … RETURNING *` on an auto-filled + // shortid table would return no rows (and the worker would read + // a zero affected-row count). `returning_tail` is "" on the + // non-RETURNING path. + let returning_suffix = if returning_tail.is_empty() { + String::new() + } else { + format!(" {returning_tail}") + }; let exec_sql = format!( - "INSERT INTO {tbl} ({cols_csv}) VALUES {vals};", + "INSERT INTO {tbl} ({cols_csv}) VALUES {vals}{returning_suffix};", tbl = quote_ident(target_table), vals = tuples.join(", "), ); @@ -6039,6 +6080,7 @@ fn plan_shortid_autofill( /// exactly the inserted rows; an INSERT that sets explicit /// non-contiguous rowid/INTEGER-PK values may surface a partial /// view. `RETURNING` (sub-phase 3g) is the precise tool. +#[allow(clippy::too_many_arguments)] fn do_sql_insert( conn: &Connection, persistence: Option<&Persistence>, @@ -6047,8 +6089,24 @@ fn do_sql_insert( target_table: &str, listed_columns: &[String], row_source: &str, + returning: bool, ) -> Result { - debug!(sql = %sql, table = %target_table, "sql_insert"); + debug!(sql = %sql, table = %target_table, returning, "sql_insert"); + // RETURNING (3g): the `shortid` auto-fill rewrite reconstructs + // only `INSERT … VALUES …` and would drop the RETURNING tail, so + // extract it here to re-append. `row_source` is the clean + // VALUES/SELECT text (no RETURNING — `build_sql_insert` stops the + // slice at the RETURNING token), so whatever follows it in the + // full `sql` is the RETURNING clause. On the verbatim (no + // auto-fill) path the original `sql` already carries RETURNING, + // so the tail is only consumed by the rewrite. + let returning_tail: String = if returning && !row_source.is_empty() { + sql.find(row_source) + .map(|i| sql[i + row_source.len()..].trim().trim_end_matches(';').trim().to_string()) + .unwrap_or_default() + } else { + String::new() + }; // Sub-phase 3d: when the user's column list omits one or more // `shortid` columns, the worker materialises the row source, // synthesises fresh distinct ids, and reinserts the augmented @@ -6056,20 +6114,29 @@ fn do_sql_insert( // params vec with the original `sql` means "no auto-fill — // execute verbatim" (the 3b path). let (exec_sql, params) = - plan_shortid_autofill(conn, target_table, sql, listed_columns, row_source)?; + plan_shortid_autofill(conn, target_table, sql, listed_columns, row_source, &returning_tail)?; let tx = conn .unchecked_transaction() .map_err(DbError::from_rusqlite)?; - let rows_affected = - execute_with_fk_enrichment(conn, target_table, &exec_sql, ¶ms)?; - let last = conn.last_insert_rowid(); - let rowids: Vec = if rows_affected == 0 { - Vec::new() + // RETURNING (3g): one pass inserts and yields the inserted rows + // (incl. any auto-filled shortid), so the returned set is the + // precise auto-show and rows_affected is its length. Without + // RETURNING, fall back to the best-effort rowid auto-show. + let (rows_affected, data) = if returning { + let data = run_returning(conn, &exec_sql, ¶ms, target_table)?; + (data.rows.len(), data) } else { - let n = rows_affected as i64; - ((last - n + 1)..=last).collect() + let n = execute_with_fk_enrichment(conn, target_table, &exec_sql, ¶ms)?; + let last = conn.last_insert_rowid(); + let rowids: Vec = if n == 0 { + Vec::new() + } else { + let count = n as i64; + ((last - count + 1)..=last).collect() + }; + let data = query_rows_by_rowid(conn, target_table, &rowids)?; + (n, data) }; - let data = query_rows_by_rowid(conn, target_table, &rowids)?; let changes = Changes { schema_dirty: false, rewritten_tables: vec![target_table.to_string()], @@ -6107,12 +6174,31 @@ fn do_sql_update( source: Option<&str>, sql: &str, target_table: &str, + returning: bool, ) -> Result { - debug!(sql = %sql, table = %target_table, "sql_update"); + debug!(sql = %sql, table = %target_table, returning, "sql_update"); let tx = conn .unchecked_transaction() .map_err(DbError::from_rusqlite)?; - let rows_affected = execute_with_fk_enrichment(conn, target_table, sql, &[])?; + // RETURNING (3g): one pass performs the update and yields the + // modified rows; rows_affected is the row count. Without + // RETURNING the affected-row count surfaces and the (column-less) + // DataResult is skipped by the renderer (3e behaviour). + let (rows_affected, data) = if returning { + let data = run_returning(conn, sql, &[], target_table)?; + (data.rows.len(), data) + } else { + let n = execute_with_fk_enrichment(conn, target_table, sql, &[])?; + ( + n, + DataResult { + table_name: target_table.to_string(), + columns: Vec::new(), + column_types: Vec::new(), + rows: Vec::new(), + }, + ) + }; let changes = Changes { schema_dirty: false, rewritten_tables: vec![target_table.to_string()], @@ -6120,15 +6206,7 @@ fn do_sql_update( }; finalize_persistence(conn, persistence, source, &changes)?; tx.commit().map_err(DbError::from_rusqlite)?; - Ok(UpdateResult { - rows_affected, - data: DataResult { - table_name: target_table.to_string(), - columns: Vec::new(), - column_types: Vec::new(), - rows: Vec::new(), - }, - }) + Ok(UpdateResult { rows_affected, data }) } /// Worker handler for `Request::RunSqlDelete` (ADR-0033 §1/§7, @@ -6167,8 +6245,9 @@ fn do_sql_delete( source: Option<&str>, sql: &str, target_table: &str, + returning: bool, ) -> Result { - debug!(sql = %sql, table = %target_table, "sql_delete"); + debug!(sql = %sql, table = %target_table, returning, "sql_delete"); // Snapshot child-table row counts before the delete so cascade // effects can be detected by diffing afterwards (Amendment 2; @@ -6183,7 +6262,27 @@ fn do_sql_delete( let tx = conn .unchecked_transaction() .map_err(DbError::from_rusqlite)?; - let rows_affected = execute_with_fk_enrichment(conn, target_table, sql, &[])?; + // RETURNING (3g): one pass deletes and yields the rows as they + // were *before* deletion. `rows_affected` is the count of + // directly-deleted rows either way (RETURNING does not yield + // cascade-deleted child rows, so data.rows.len() == direct + // deletes), which keeps the self-ref cascade correction below + // valid. The cascade pre-count was already captured above. + let (rows_affected, data) = if returning { + let data = run_returning(conn, sql, &[], target_table)?; + (data.rows.len(), data) + } else { + let n = execute_with_fk_enrichment(conn, target_table, sql, &[])?; + ( + n, + DataResult { + table_name: target_table.to_string(), + columns: Vec::new(), + column_types: Vec::new(), + rows: Vec::new(), + }, + ) + }; // Compare child-table counts after the delete; positive diffs // are cascade effects. Collect the cascaded tables so the @@ -6228,6 +6327,7 @@ fn do_sql_delete( Ok(DeleteResult { rows_affected, cascade, + data, }) } @@ -6245,6 +6345,61 @@ fn do_sql_delete( /// `None`. The renderer (ADR-0016) handles typed columns /// (bool → true/false, etc.) and falls back to neutral /// alignment for `None`. +/// Execute a grammar-validated SQL DML statement carrying a +/// `RETURNING` clause and collect its returned rows into a +/// [`DataResult`] (ADR-0033 §5, sub-phase 3g). +/// +/// A `RETURNING` DML, when stepped, *both* performs the mutation +/// *and* yields one result row per affected row — so `query_map` +/// does the write and the read in one pass. Result-column +/// playground types are recovered via the same column-origin path +/// SELECT uses (`resolve_select_column_types`), so a bare-column +/// `RETURNING` ref renders with its playground type; computed +/// projections stay typeless. `params` carries the bound values for +/// the `shortid` auto-fill rewrite (empty on the verbatim path). +/// +/// `table_name` labels the result for the renderer; the columns are +/// the RETURNING projection, which may not be the table's columns +/// (aliases, expressions), exactly as for a SELECT. +fn run_returning( + conn: &Connection, + sql: &str, + params: &[rusqlite::types::Value], + table_name: &str, +) -> Result { + let mut stmt = conn.prepare(sql).map_err(DbError::from_rusqlite)?; + let column_names: Vec = + stmt.column_names().into_iter().map(String::from).collect(); + let col_count = column_names.len(); + let column_types = resolve_select_column_types(conn, &stmt); + let rows_iter = stmt + .query_map(rusqlite::params_from_iter(params.iter()), |row| { + let mut cells: Vec = Vec::with_capacity(col_count); + for i in 0..col_count { + cells.push(row.get(i)?); + } + Ok(cells) + }) + .map_err(DbError::from_rusqlite)?; + let mut rows: Vec>> = Vec::new(); + for r in rows_iter { + let cells = r.map_err(DbError::from_rusqlite)?; + rows.push( + cells + .into_iter() + .enumerate() + .map(|(i, v)| format_cell(v, column_types.get(i).copied().flatten())) + .collect(), + ); + } + Ok(DataResult { + table_name: table_name.to_string(), + columns: column_names, + column_types, + rows, + }) +} + fn do_run_select(conn: &Connection, sql: &str) -> Result { debug!(sql = %sql, "run_select"); let mut stmt = conn.prepare(sql).map_err(DbError::from_rusqlite)?; diff --git a/src/dsl/command.rs b/src/dsl/command.rs index 9651e08..fbe6e72 100644 --- a/src/dsl/command.rs +++ b/src/dsl/command.rs @@ -311,6 +311,11 @@ pub enum Command { target_table: String, listed_columns: Vec, row_source: String, + /// Whether a `RETURNING` clause matched (ADR-0033 §5, + /// sub-phase 3g). The worker collects the returned rows as a + /// `DataResult` when true; otherwise it surfaces the + /// affected-row count (+ auto-show) as before. + returning: bool, }, /// A SQL `UPDATE` validated by the walker (ADR-0033 §2, /// advanced mode). Grammar-as-text: the worker executes `sql` @@ -319,6 +324,9 @@ pub enum Command { SqlUpdate { sql: String, target_table: String, + /// Whether a `RETURNING` clause matched (ADR-0033 §5, + /// sub-phase 3g). + returning: bool, }, /// A SQL `DELETE` validated by the walker (ADR-0033 §1/§7, /// advanced mode). Grammar-as-text: the worker executes `sql`, @@ -331,6 +339,10 @@ pub enum Command { SqlDelete { sql: String, target_table: String, + /// Whether a `RETURNING` clause matched (ADR-0033 §5, + /// sub-phase 3g). The cascade summary surfaces alongside the + /// returned rows when true. + returning: bool, }, /// App-lifecycle command (per ADR-0003). These work in both /// simple and advanced modes; the dispatcher branches on the diff --git a/src/dsl/grammar/data.rs b/src/dsl/grammar/data.rs index 4430699..2bc7bae 100644 --- a/src/dsl/grammar/data.rs +++ b/src/dsl/grammar/data.rs @@ -890,10 +890,19 @@ fn build_sql_insert(path: &MatchedPath, source: &str) -> Result None, }) .collect(); - // The row source is everything from the `VALUES` / `SELECT` / - // `WITH` keyword onward. Located by the first matching *Word - // token* in the path (not a text scan), so a string literal - // like `values ('select')` can't be mistaken for the keyword. + // The row source is the `VALUES` / `SELECT` / `WITH` clause — + // from that keyword up to (but not including) any `RETURNING` + // tail (3g) or trailing `;`. Both boundaries are located by + // *Word token* in the path (not a text scan), so a string + // literal like `values ('select')` / `values ('returning')` + // can't be mistaken for a keyword. Excluding RETURNING keeps the + // row source independently preparable for `shortid` auto-fill + // (`VALUES … RETURNING …` is not a valid standalone statement). + let returning_start = path + .items + .iter() + .find(|item| matches!(item.kind, MatchedKind::Word("returning"))) + .map(|item| item.span.0); let row_source = path .items .iter() @@ -901,7 +910,8 @@ fn build_sql_insert(path: &MatchedPath, source: &str) -> Result Result bool { + path.items + .iter() + .any(|item| matches!(item.kind, MatchedKind::Word("returning"))) +} + /// Build `Command::SqlUpdate` from a validated SQL `UPDATE` /// (ADR-0033 §2, sub-phase 3e). Extracts the target table from the /// matched path so the worker re-persists the right CSV. @@ -949,7 +971,11 @@ fn build_sql_update(path: &MatchedPath, source: &str) -> Result Result 0 then 'y' else 'n' end)"); } + #[test] + fn returning_tail_admitted() { + // 3g: optional RETURNING projection_list tail, on both row + // sources. + good("into orders values (1, 2.0) returning *"); + good("into orders (id, total) values (1, 2.0) returning id"); + good("into orders values (1, 'a'), (2, 'b') returning id, total"); + good("into archive select * from orders returning *"); + good("into orders values (1) returning id as new_id;"); + } + #[test] fn internal_target_table_rejected() { bad("into __rdbms_playground_columns values (1)"); diff --git a/src/dsl/grammar/sql_select.rs b/src/dsl/grammar/sql_select.rs index 23d1a0f..a8d40b6 100644 --- a/src/dsl/grammar/sql_select.rs +++ b/src/dsl/grammar/sql_select.rs @@ -143,6 +143,11 @@ static EMPTY_NOMATCH: Node = Node::Choice(&[]); const PROJECTION_FOLLOW_SET: &[&str] = &[ "from", "where", "group", "order", "having", "limit", "union", "intersect", "except", + // `returning` belongs to an enclosing DML statement + // (`INSERT … SELECT … RETURNING …`, ADR-0033 §5), never to a + // projection item's bare alias — so a no-FROM SELECT row source + // (`select id returning *`) stops before it. + "returning", ]; /// Continuation keywords that may legitimately follow a table @@ -156,6 +161,10 @@ const TABLE_SOURCE_FOLLOW_SET: &[&str] = &[ "where", "group", "order", "having", "limit", "union", "intersect", "except", "inner", "left", "right", "full", "cross", "join", "on", + // `returning` belongs to an enclosing DML statement + // (`INSERT … SELECT … FROM t RETURNING …`, ADR-0033 §5), so the + // SELECT row source must not read it as table `t`'s bare alias. + "returning", ]; fn peek_next_ident_lower(source: &str, pos: usize) -> Option { @@ -325,12 +334,26 @@ fn projection_item_factory( static PROJECTION_ITEM: Node = Node::Lookahead(projection_item_factory); -static PROJECTION_LIST: Node = Node::Repeated { +pub(crate) static PROJECTION_LIST: Node = Node::Repeated { inner: &PROJECTION_ITEM, separator: Some(&COMMA), min: 1, }; +/// `RETURNING projection_list` — the optional tail shared by the +/// SQL DML statements (ADR-0033 §5, sub-phase 3g). Reuses the +/// Phase-2 projection list unchanged (`*`, bare/qualified column +/// refs, `expr AS alias`, computed expressions), so a RETURNING +/// projection is parsed, completed and highlighted exactly as a +/// SELECT projection. The worker collects the returned rows as a +/// `DataResult`; result-column playground types are recovered via +/// the same column-origin path SELECT uses (ADR-0032 §12). +pub(crate) static RETURNING_CLAUSE: Node = Node::Seq(RETURNING_CLAUSE_NODES); +static RETURNING_CLAUSE_NODES: &[Node] = &[ + Node::Word(Word::keyword("returning")), + Node::Subgrammar(&PROJECTION_LIST), +]; + // ================================================================= // DISTINCT / ALL prefix // ================================================================= diff --git a/src/dsl/grammar/sql_update.rs b/src/dsl/grammar/sql_update.rs index a1730da..53a5f05 100644 --- a/src/dsl/grammar/sql_update.rs +++ b/src/dsl/grammar/sql_update.rs @@ -15,7 +15,7 @@ //! written (ADR-0030 §12). `RETURNING` (3g) lands later. use crate::dsl::grammar::sql_expr; -use crate::dsl::grammar::sql_select::{WHERE_CLAUSE, reject_internal_table}; +use crate::dsl::grammar::sql_select::{RETURNING_CLAUSE, WHERE_CLAUSE, reject_internal_table}; use crate::dsl::grammar::{IdentSource, Node, Word}; static COMMA: Node = Node::Punct(','); @@ -79,6 +79,7 @@ static SQL_UPDATE_TAIL_NODES: &[Node] = &[ Node::Word(Word::keyword("set")), ASSIGNMENT_LIST, Node::Optional(&WHERE_CLAUSE), + Node::Optional(&RETURNING_CLAUSE), Node::Optional(&Node::Punct(';')), ]; @@ -153,6 +154,14 @@ mod tests { good("t set v = (select max(other) from other_table) where id = 1"); } + #[test] + fn returning_tail_admitted() { + // 3g: optional RETURNING projection_list tail. + good("t set v = 1 where id = 1 returning *"); + good("t set v = 1 returning id, v"); + good("t set v = 1 where id = 1 returning v as new_v;"); + } + #[test] fn internal_target_table_rejected() { bad("__rdbms_playground_columns set a = 1"); diff --git a/src/runtime.rs b/src/runtime.rs index 424564c..b2003ba 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1890,16 +1890,21 @@ async fn execute_command_typed( target_table, listed_columns, row_source, + returning, } => database - .run_sql_insert(sql, src, target_table, listed_columns, row_source) + .run_sql_insert(sql, src, target_table, listed_columns, row_source, returning) .await .map(CommandOutcome::Insert), // A SQL `UPDATE` (advanced mode; ADR-0033 §2). Grammar-as- // text: the worker runs the validated `sql` and re-persists // the parsed `target_table`'s CSV. Reuses the DSL update // outcome (affected-row count). - Command::SqlUpdate { sql, target_table } => database - .run_sql_update(sql, src, target_table) + Command::SqlUpdate { + sql, + target_table, + returning, + } => database + .run_sql_update(sql, src, target_table, returning) .await .map(CommandOutcome::Update), // A SQL `DELETE` (advanced mode; ADR-0033 §1/§7). Grammar- @@ -1908,8 +1913,12 @@ async fn execute_command_typed( // plus every cascade-affected child. Reuses the DSL delete // outcome (affected-row count + per-relationship cascade // summary). - Command::SqlDelete { sql, target_table } => database - .run_sql_delete(sql, src, target_table) + Command::SqlDelete { + sql, + target_table, + returning, + } => database + .run_sql_delete(sql, src, target_table, returning) .await .map(CommandOutcome::Delete), // `EXPLAIN QUERY PLAN` never executes the wrapped diff --git a/tests/sql_delete.rs b/tests/sql_delete.rs index db075ae..3bcf173 100644 --- a/tests/sql_delete.rs +++ b/tests/sql_delete.rs @@ -71,6 +71,7 @@ fn seed(db: &Database, rt: &tokio::runtime::Runtime, sql: &str, target: &str) { target.to_string(), Vec::new(), String::new(), + false, )) .unwrap_or_else(|e| panic!("seed {sql:?}: {e:?}")); } @@ -82,8 +83,8 @@ fn run_delete( input: &str, ) -> Result { match parse_command(input).expect("parse sql_delete") { - Command::SqlDelete { sql, target_table } => { - rt.block_on(db.run_sql_delete(sql, Some(input.to_string()), target_table)) + Command::SqlDelete { sql, target_table, returning } => { + rt.block_on(db.run_sql_delete(sql, Some(input.to_string()), target_table, returning)) } other => panic!("expected Command::SqlDelete, got {other:?}"), } @@ -117,7 +118,7 @@ fn parse_path_lowers_sql_delete_to_command() { let command = parse_command("sql_delete from Orders where id = 1") .expect("sql_delete parses in advanced mode"); match command { - Command::SqlDelete { sql, target_table } => { + Command::SqlDelete { sql, target_table, .. } => { assert_eq!(sql, "delete from Orders where id = 1"); assert_eq!(target_table, "Orders"); } @@ -424,3 +425,42 @@ fn internal_target_table_rejected_at_parse() { "internal table must be rejected at the DELETE target slot" ); } + +// ================================================================= +// Sub-phase 3g — RETURNING (ADR-0033 §5) +// ================================================================= + +#[test] +fn delete_returning_yields_predelete_row() { + let (project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("v", Type::Text)], &["id"]); + seed(&db, &rt, "insert into t (id, v) values (1, 'gone'), (2, 'keep')", "t"); + let result = run_delete(&db, &rt, "sql_delete from t where id = 1 returning *") + .expect("DELETE … RETURNING * runs"); + assert_eq!(result.rows_affected, 1, "one row deleted"); + // RETURNING yields the row as it was BEFORE deletion. + assert_eq!(result.data.rows.len(), 1, "the deleted row was returned"); + assert_eq!(result.data.rows[0][1], Some("gone".to_string())); + // And it really is gone from the table. + let csv = read_csv(&project, "t").expect("t.csv"); + assert!(!csv.contains("gone") && csv.contains("keep"), "row actually deleted: {csv:?}"); +} + +#[test] +fn delete_returning_with_cascade_surfaces_both() { + // 3g: a parent DELETE … RETURNING must surface BOTH the returned + // (deleted) parent rows AND the per-relationship cascade summary. + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + cascade_fixture(&db, &rt); + let result = run_delete(&db, &rt, "sql_delete from Customers where id = 1 returning *") + .expect("cascading DELETE … RETURNING runs"); + assert_eq!(result.rows_affected, 1, "one parent row deleted"); + // RETURNING gave the deleted customer row. + assert_eq!(result.data.rows.len(), 1, "deleted parent row returned"); + // Cascade summary still computed alongside the result set. + assert_eq!(result.cascade.len(), 1, "cascade reported"); + assert_eq!(result.cascade[0].child_table, "Orders"); + assert_eq!(result.cascade[0].rows_changed, 2, "both Alice's orders cascaded"); +} diff --git a/tests/sql_insert.rs b/tests/sql_insert.rs index 687696e..9b44e13 100644 --- a/tests/sql_insert.rs +++ b/tests/sql_insert.rs @@ -74,6 +74,7 @@ fn single_row_insert_persists_and_counts() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("insert runs"); assert_eq!(result.rows_affected, 1, "one row inserted"); @@ -93,6 +94,7 @@ fn multi_row_insert_persists_both_rows() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("multi-row insert runs"); assert_eq!(result.rows_affected, 2, "two rows inserted"); @@ -115,6 +117,7 @@ fn no_column_list_full_arity_insert_persists() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("full-arity insert runs"); assert_eq!(result.rows_affected, 1); @@ -135,6 +138,7 @@ fn insert_appends_literal_line_to_history() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("insert runs"); let body = std::fs::read_to_string(project.path().join("history.log")) @@ -157,6 +161,7 @@ fn failed_insert_rolls_back_and_does_not_repersist() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("first insert runs"); // Second insert violates the primary key — it must fail and @@ -168,6 +173,7 @@ fn failed_insert_rolls_back_and_does_not_repersist() { "T".to_string(), Vec::new(), String::new(), + false, )); assert!(outcome.is_err(), "duplicate PK must fail: {outcome:?}"); let csv = read_csv(&project, "T").expect("T.csv still present"); @@ -191,6 +197,7 @@ fn failed_multi_row_insert_is_atomic() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("seed row"); // Row (2,…) is new but (1,…) collides on the PK — the whole @@ -201,6 +208,7 @@ fn failed_multi_row_insert_is_atomic() { "T".to_string(), Vec::new(), String::new(), + false, )); assert!(outcome.is_err(), "multi-row PK conflict must fail: {outcome:?}"); let csv = read_csv(&project, "T").expect("T.csv still present"); @@ -296,6 +304,7 @@ fn insert_select_copies_rows_and_persists() { "source".to_string(), Vec::new(), String::new(), + false, )) .expect("seed source"); let result = rt @@ -305,6 +314,7 @@ fn insert_select_copies_rows_and_persists() { "archive".to_string(), Vec::new(), String::new(), + false, )) .expect("INSERT … SELECT runs"); assert_eq!(result.rows_affected, 2, "both source rows copied"); @@ -327,6 +337,7 @@ fn insert_select_with_column_list_and_projection_persists() { "source".to_string(), Vec::new(), String::new(), + false, )) .expect("seed source"); let result = rt @@ -336,6 +347,7 @@ fn insert_select_with_column_list_and_projection_persists() { "target".to_string(), Vec::new(), String::new(), + false, )) .expect("column-list + projection INSERT … SELECT runs"); assert_eq!(result.rows_affected, 1); @@ -356,6 +368,7 @@ fn with_prefixed_insert_select_runs_and_persists() { "orders".to_string(), Vec::new(), String::new(), + false, )) .expect("seed orders"); let result = rt @@ -365,6 +378,7 @@ fn with_prefixed_insert_select_runs_and_persists() { "archive".to_string(), Vec::new(), String::new(), + false, )) .expect("WITH-prefixed INSERT … SELECT runs"); assert_eq!(result.rows_affected, 2); @@ -389,6 +403,7 @@ fn insert_select_from_self_runs_as_plain_insert() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("seed"); let result = rt @@ -398,6 +413,7 @@ fn insert_select_from_self_runs_as_plain_insert() { "T".to_string(), Vec::new(), String::new(), + false, )) .expect("self-sourced INSERT … SELECT runs"); assert_eq!(result.rows_affected, 2, "two rows copied with shifted PK"); @@ -426,12 +442,14 @@ fn run_sqlinsert( target_table, listed_columns, row_source, + returning, } => rt.block_on(db.run_sql_insert( sql, Some(input.to_string()), target_table, listed_columns, row_source, + returning, )), other => panic!("expected Command::SqlInsert, got {other:?}"), } @@ -714,3 +732,168 @@ fn autofill_insert_select_narrower_projection_is_rejected() { assert!(outcome.is_err(), "narrower projection must be rejected: {outcome:?}"); assert!(csv_rows(&project, "t").is_empty(), "nothing should land"); } + +// ================================================================= +// Sub-phase 3g — RETURNING (ADR-0033 §5) +// ================================================================= + +#[test] +fn insert_returning_star_returns_inserted_row() { + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("b", Type::Text)], &["id"]); + let result = run_sqlinsert(&db, &rt, "sqlinsert into t (id, b) values (1, 'Ada') returning *") + .expect("INSERT … RETURNING * runs"); + assert_eq!(result.rows_affected, 1, "one row inserted"); + assert_eq!(result.data.rows.len(), 1, "RETURNING yielded the inserted row"); + assert_eq!(result.data.columns, vec!["id".to_string(), "b".to_string()]); + assert_eq!(result.data.rows[0][1], Some("Ada".to_string())); +} + +#[test] +fn insert_multirow_returning_id_yields_distinct_rows() { + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("b", Type::Text)], &["id"]); + let result = run_sqlinsert( + &db, + &rt, + "sqlinsert into t (id, b) values (1, 'a'), (2, 'b'), (3, 'c') returning id", + ) + .expect("multi-row INSERT … RETURNING id runs"); + assert_eq!(result.rows_affected, 3, "three rows inserted"); + assert_eq!(result.data.columns, vec!["id".to_string()]); + let ids: std::collections::BTreeSet<_> = + result.data.rows.iter().map(|r| r[0].clone()).collect(); + assert_eq!(ids.len(), 3, "three distinct ids returned: {:?}", result.data.rows); +} + +#[test] +fn insert_returning_autofills_shortid_and_returns_it() { + // The auto-fill × RETURNING interaction (3d × 3g): the worker + // rewrites the INSERT to add the generated shortid, and the + // rewrite must PRESERVE the RETURNING tail so the generated id + // surfaces in the returned row. + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); + let result = run_sqlinsert(&db, &rt, "sqlinsert into t (label) values ('x') returning *") + .expect("auto-fill INSERT … RETURNING * runs"); + assert_eq!(result.rows_affected, 1, "one row inserted (RETURNING-counted)"); + assert_eq!(result.data.rows.len(), 1, "RETURNING yielded the row"); + // `id` is the auto-filled shortid column; it must be non-empty in + // the returned row (proving the rewrite kept RETURNING). + let id_idx = result.data.columns.iter().position(|c| c == "id").expect("id column"); + let id_val = result.data.rows[0][id_idx].clone(); + assert!(id_val.is_some_and(|s| !s.is_empty()), "generated shortid surfaced via RETURNING"); +} + +#[test] +fn insert_returning_recovers_bare_column_type() { + // 3g type recovery: a bare-column RETURNING ref recovers its + // playground type via the column-origin path (a `bool` column + // renders as the word, not 0/1). + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("active", Type::Bool)], &["id"]); + let result = run_sqlinsert(&db, &rt, "sqlinsert into t (id, active) values (1, true) returning active") + .expect("INSERT … RETURNING active runs"); + assert_eq!(result.data.column_types, vec![Some(Type::Bool)], "bool type recovered"); + assert_eq!(result.data.rows[0][0], Some("true".to_string()), "rendered as the bool word"); +} + +#[test] +fn insert_returning_computed_expression_is_typeless() { + // 3g: a computed RETURNING projection has no base-table origin, + // so its recovered type is None (renders with neutral alignment). + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("n", Type::Int)], &["id"]); + let result = run_sqlinsert(&db, &rt, "sqlinsert into t (id, n) values (1, 5) returning n + 1") + .expect("INSERT … RETURNING runs"); + assert_eq!(result.data.column_types, vec![None], "computed projection is typeless"); + assert_eq!(result.data.rows[0][0], Some("6".to_string()), "engine evaluated n + 1"); +} + +#[test] +fn insert_returning_recovers_multiple_bare_column_types() { + // 3g type recovery spans the playground vocabulary. RETURNING + // reuses the SELECT column-origin path (`resolve_select_column_ + // types`), exhaustively type-tested on the SELECT side; this + // pins a representative spread reached via the RETURNING tail. + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols( + &db, + &rt, + "t", + &[ + ("id", Type::Int), + ("txt", Type::Text), + ("amount", Type::Decimal), + ("ratio", Type::Real), + ("flag", Type::Bool), + ], + &["id"], + ); + let result = run_sqlinsert( + &db, + &rt, + "sqlinsert into t (id, txt, amount, ratio, flag) values (1, 'a', 9.50, 1.5, true) returning id, txt, amount, ratio, flag", + ) + .expect("INSERT … RETURNING runs"); + assert_eq!( + result.data.column_types, + vec![ + Some(Type::Int), + Some(Type::Text), + Some(Type::Decimal), + Some(Type::Real), + Some(Type::Bool), + ], + "each bare-column RETURNING ref recovered its playground type", + ); +} + +#[test] +fn multirow_autofill_returning_yields_distinct_generated_ids() { + // DA gate (3d × 3g): multi-row INSERT with an omitted shortid PK + // AND RETURNING — the auto-fill rewrite produces N tuples with N + // distinct generated ids, and RETURNING * must surface all N + // rows each carrying its own generated id. + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::ShortId), ("label", Type::Text)], &["id"]); + let result = run_sqlinsert( + &db, + &rt, + "sqlinsert into t (label) values ('a'), ('b'), ('c') returning *", + ) + .expect("multi-row auto-fill INSERT … RETURNING * runs"); + assert_eq!(result.rows_affected, 3, "three rows inserted"); + assert_eq!(result.data.rows.len(), 3, "three rows returned"); + let id_idx = result.data.columns.iter().position(|c| c == "id").expect("id column"); + let ids: std::collections::BTreeSet<_> = + result.data.rows.iter().map(|r| r[id_idx].clone()).collect(); + assert_eq!(ids.len(), 3, "three DISTINCT generated ids via RETURNING: {:?}", result.data.rows); + assert!(ids.iter().all(|v| v.as_ref().is_some_and(|s| !s.is_empty())), "all ids non-empty"); +} + +#[test] +fn insert_select_returning_executes_and_returns_rows() { + // DA gate: the grammar accepts INSERT … SELECT … RETURNING; this + // pins that it also EXECUTES through run_returning (the SELECT row + // source feeds the insert, and RETURNING yields the inserted rows). + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "src", &[("id", Type::Int), ("b", Type::Text)], &["id"]); + create_cols(&db, &rt, "dst", &[("id", Type::Int), ("b", Type::Text)], &["id"]); + run_sqlinsert(&db, &rt, "sqlinsert into src (id, b) values (1, 'x'), (2, 'y')").expect("seed src"); + let result = run_sqlinsert(&db, &rt, "sqlinsert into dst select * from src returning id, b") + .expect("INSERT … SELECT … RETURNING runs"); + assert_eq!(result.rows_affected, 2, "two rows copied"); + assert_eq!(result.data.rows.len(), 2, "RETURNING yielded both inserted rows"); + let bs: std::collections::BTreeSet<_> = + result.data.rows.iter().map(|r| r[1].clone()).collect(); + assert!(bs.contains(&Some("x".to_string())) && bs.contains(&Some("y".to_string()))); +} diff --git a/tests/sql_update.rs b/tests/sql_update.rs index 04b4db7..75866f7 100644 --- a/tests/sql_update.rs +++ b/tests/sql_update.rs @@ -58,6 +58,7 @@ fn seed(db: &Database, rt: &tokio::runtime::Runtime, sql: &str, target: &str) { target.to_string(), Vec::new(), String::new(), + false, )) .unwrap_or_else(|e| panic!("seed {sql:?}: {e:?}")); } @@ -69,8 +70,8 @@ fn run_update( input: &str, ) -> Result { match parse_command(input).expect("parse sql_update") { - Command::SqlUpdate { sql, target_table } => { - rt.block_on(db.run_sql_update(sql, Some(input.to_string()), target_table)) + Command::SqlUpdate { sql, target_table, returning } => { + rt.block_on(db.run_sql_update(sql, Some(input.to_string()), target_table, returning)) } other => panic!("expected Command::SqlUpdate, got {other:?}"), } @@ -81,7 +82,7 @@ fn parse_path_lowers_sql_update_to_command() { let command = parse_command("sql_update Orders set total = 0 where id = 1") .expect("sql_update parses in advanced mode"); match command { - Command::SqlUpdate { sql, target_table } => { + Command::SqlUpdate { sql, target_table, .. } => { assert_eq!(sql, "update Orders set total = 0 where id = 1"); assert_eq!(target_table, "Orders"); } @@ -203,3 +204,51 @@ fn update_appends_literal_line_to_history() { .expect("history.log present"); assert!(body.contains(input), "history records the literal line: {body:?}"); } + +// ================================================================= +// Sub-phase 3g — RETURNING (ADR-0033 §5) +// ================================================================= + +#[test] +fn update_returning_yields_modified_columns() { + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("v", Type::Text)], &["id"]); + seed(&db, &rt, "insert into t (id, v) values (1, 'old'), (2, 'keep')", "t"); + let result = run_update(&db, &rt, "sql_update t set v = 'new' where id = 1 returning id, v") + .expect("UPDATE … RETURNING runs"); + assert_eq!(result.rows_affected, 1, "one row updated"); + assert_eq!(result.data.columns, vec!["id".to_string(), "v".to_string()]); + assert_eq!(result.data.rows.len(), 1); + // RETURNING reflects the POST-update value. + assert_eq!(result.data.rows[0][1], Some("new".to_string()), "modified value returned"); +} + +#[test] +fn update_returning_recovers_bare_column_type() { + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("active", Type::Bool)], &["id"]); + seed(&db, &rt, "insert into t (id, active) values (1, false)", "t"); + let result = run_update(&db, &rt, "sql_update t set active = true where id = 1 returning active") + .expect("UPDATE … RETURNING active runs"); + assert_eq!(result.data.column_types, vec![Some(Type::Bool)], "bool type recovered"); + assert_eq!(result.data.rows[0][0], Some("true".to_string())); +} + +#[test] +fn update_returning_matching_no_rows_is_ok_and_empty() { + // DA gate: RETURNING makes data.columns non-empty even when no + // rows match (unlike the 3e column-less case). The operation + // succeeds with zero rows and an empty result set — no panic, no + // phantom row. + let (_project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("v", Type::Text)], &["id"]); + seed(&db, &rt, "insert into t (id, v) values (1, 'keep')", "t"); + let result = run_update(&db, &rt, "sql_update t set v = 'x' where id = 999 returning id, v") + .expect("no-match UPDATE … RETURNING is a success"); + assert_eq!(result.rows_affected, 0, "no rows matched"); + assert!(result.data.rows.is_empty(), "no rows returned"); + assert_eq!(result.data.columns, vec!["id".to_string(), "v".to_string()], "columns still present"); +}