diff --git a/src/app.rs b/src/app.rs index 581fc6d..d231b55 100644 --- a/src/app.rs +++ b/src/app.rs @@ -498,7 +498,14 @@ impl App { self.note_system(crate::t!("ddl.create_index_skipped_exists", name = name)); Vec::new() } - AppEvent::DslDataSucceeded { command, data } => { + AppEvent::DslDataSucceeded { + command, + data, + echo, + } => { + // Stash the teaching echo (ADR-0038) for `note_ok_summary` + // to render beneath `[ok]` — consumed synchronously below. + self.pending_echo = echo; self.handle_dsl_query_success(&command, &data); Vec::new() } @@ -510,23 +517,48 @@ impl App { self.handle_dsl_insert_success(&command, &result); Vec::new() } - AppEvent::DslUpdateSucceeded { command, result } => { + AppEvent::DslUpdateSucceeded { + command, + result, + echo, + } => { + self.pending_echo = echo; self.handle_dsl_update_success(&command, &result); Vec::new() } - AppEvent::DslDeleteSucceeded { command, result } => { + AppEvent::DslDeleteSucceeded { + command, + result, + echo, + } => { + self.pending_echo = echo; self.handle_dsl_delete_success(&command, &result); Vec::new() } - AppEvent::DslChangeColumnSucceeded { command, result } => { + AppEvent::DslChangeColumnSucceeded { + command, + result, + echo, + } => { + self.pending_echo = echo; self.handle_dsl_change_column_success(&command, result); Vec::new() } - AppEvent::DslAddColumnSucceeded { command, result } => { + AppEvent::DslAddColumnSucceeded { + command, + result, + echo, + } => { + self.pending_echo = echo; self.handle_dsl_add_column_success(&command, result); Vec::new() } - AppEvent::DslDropColumnSucceeded { command, result } => { + AppEvent::DslDropColumnSucceeded { + command, + result, + echo, + } => { + self.pending_echo = echo; self.handle_dsl_drop_column_success(&command, result); Vec::new() } @@ -2879,6 +2911,143 @@ mod tests { ); } + #[test] + fn bucket_a_success_events_render_the_teaching_echo_beneath_ok() { + // ADR-0038 Phase 1: every Bucket A success event that gained an + // `echo` field stashes it for `note_ok_summary` to render + // immediately beneath `[ok]`. One case per event guards its + // update() arm's stash + ordering — including the handlers that + // render extra content (structure / notes), where the echo must + // still sit directly beneath `[ok]`, above that content. + use crate::db::{ + AddColumnResult, ChangeColumnTypeResult, DataResult, DeleteResult, DropColumnResult, + UpdateResult, + }; + use crate::dsl::command::{ChangeColumnMode, RowFilter}; + use crate::dsl::value::Value; + + fn assert_echo_beneath_ok(app: &App, expected: &str) { + let texts: Vec<&str> = app.output.iter().map(|l| l.text.as_str()).collect(); + let ok_idx = texts + .iter() + .position(|t| t.starts_with("[ok]")) + .expect("an [ok] line"); + let echo_idx = texts + .iter() + .position(|t| t.contains("Executing SQL:")) + .expect("an echo line"); + assert_eq!(echo_idx, ok_idx + 1, "echo sits immediately beneath [ok]: {texts:?}"); + assert!(texts[echo_idx].contains(expected), "echo carries the SQL: {texts:?}"); + } + + fn empty_data() -> DataResult { + DataResult { + table_name: "T".to_string(), + columns: Vec::new(), + column_types: Vec::new(), + rows: Vec::new(), + } + } + + // show data → DslDataSucceeded (the post-execution query path). + let mut app = App::new(); + app.update(AppEvent::DslDataSucceeded { + command: Command::ShowData { + name: "T".to_string(), + filter: None, + limit: None, + }, + data: empty_data(), + echo: Some("SELECT * FROM T".to_string()), + }); + assert_echo_beneath_ok(&app, "SELECT * FROM T"); + + // update … --all-rows → DslUpdateSucceeded. + let mut app = App::new(); + app.update(AppEvent::DslUpdateSucceeded { + command: Command::Update { + table: "T".to_string(), + assignments: vec![("v".to_string(), Value::Number("1".to_string()))], + filter: RowFilter::AllRows, + }, + result: UpdateResult { + rows_affected: 1, + data: empty_data(), + }, + echo: Some("UPDATE T SET v = 1".to_string()), + }); + assert_echo_beneath_ok(&app, "UPDATE T SET v = 1"); + + // delete … --all-rows → DslDeleteSucceeded. + let mut app = App::new(); + app.update(AppEvent::DslDeleteSucceeded { + command: Command::Delete { + table: "T".to_string(), + filter: RowFilter::AllRows, + }, + result: DeleteResult { + rows_affected: 1, + cascade: Vec::new(), + data: empty_data(), + }, + echo: Some("DELETE FROM T".to_string()), + }); + assert_echo_beneath_ok(&app, "DELETE FROM T"); + + // add column → DslAddColumnSucceeded (handler also renders structure). + let mut app = App::new(); + app.update(AppEvent::DslAddColumnSucceeded { + command: Command::AddColumn { + table: "T".to_string(), + column: "c".to_string(), + ty: Type::Int, + not_null: false, + unique: false, + default: None, + check: None, + }, + result: AddColumnResult { + description: sample_description("T"), + client_side_notes: Vec::new(), + }, + echo: Some("ALTER TABLE T ADD COLUMN c int".to_string()), + }); + assert_echo_beneath_ok(&app, "ALTER TABLE T ADD COLUMN c int"); + + // drop column → DslDropColumnSucceeded. + let mut app = App::new(); + app.update(AppEvent::DslDropColumnSucceeded { + command: Command::DropColumn { + table: "T".to_string(), + column: "c".to_string(), + cascade: false, + }, + result: DropColumnResult { + description: sample_description("T"), + dropped_indexes: Vec::new(), + }, + echo: Some("ALTER TABLE T DROP COLUMN c".to_string()), + }); + assert_echo_beneath_ok(&app, "ALTER TABLE T DROP COLUMN c"); + + // change column → DslChangeColumnSucceeded. + let mut app = App::new(); + app.update(AppEvent::DslChangeColumnSucceeded { + command: Command::ChangeColumnType { + table: "T".to_string(), + column: "c".to_string(), + ty: Type::Text, + mode: ChangeColumnMode::Default, + }, + result: ChangeColumnTypeResult { + description: sample_description("T"), + client_side: None, + }, + echo: Some("ALTER TABLE T ALTER COLUMN c SET DATA TYPE text".to_string()), + }); + assert_echo_beneath_ok(&app, "ALTER TABLE T ALTER COLUMN c SET DATA TYPE text"); + } + #[test] fn mode_command_switches_persistently() { let mut app = App::new(); @@ -3982,6 +4151,7 @@ mod tests { rows: Vec::new(), }, }, + echo: None, }); let texts: Vec = app.output.iter().map(|l| l.text.clone()).collect(); assert!( @@ -4015,6 +4185,7 @@ mod tests { rows: vec![vec![Some("1".to_string()), Some("9".to_string())]], }, }, + echo: None, }); let texts: Vec = app.output.iter().map(|l| l.text.clone()).collect(); assert!( @@ -4055,6 +4226,7 @@ mod tests { rows: Vec::new(), }, }, + echo: None, }); let texts: Vec = app.output.iter().map(|l| l.text.clone()).collect(); assert!( @@ -4102,6 +4274,7 @@ mod tests { rows: vec![vec![Some("1".to_string()), Some("Alice".to_string())]], }, }, + echo: None, }); let texts: Vec = app.output.iter().map(|l| l.text.clone()).collect(); assert!( diff --git a/src/echo.rs b/src/echo.rs index a3592c8..4a4e40c 100644 --- a/src/echo.rs +++ b/src/echo.rs @@ -15,7 +15,10 @@ use crate::app::EffectiveMode; use crate::dsl::Command; -use crate::dsl::command::ColumnSpec; +use crate::dsl::command::{ + ColumnSpec, CompareOp, Constraint, ConstraintKind, Expr, Operand, Predicate, RowFilter, +}; +use crate::dsl::value::Value; /// The teaching echo for a command submitted under `mode`, or `None`. /// @@ -24,6 +27,11 @@ use crate::dsl::command::ColumnSpec; /// only for a DSL-form command that has an echo (`command_to_sql`; a /// `Sql*` / app command returns `None`). This is the runtime's gate; /// replay never reaches it (it bypasses the spawn). (ADR-0037 + ADR-0038) +/// +/// This pre-execution path covers every Bucket A row whose echo is a pure +/// function of the `Command`. The one exception is `show data`, whose +/// limited form orders by the table's primary key (not on the `Command`); +/// that is built post-execution by [`echo_for_query`] (ADR-0038 §4). #[must_use] pub fn echo_for(command: &Command, mode: EffectiveMode) -> Option { if mode.is_advanced() { @@ -33,8 +41,39 @@ pub fn echo_for(command: &Command, mode: EffectiveMode) -> Option { } } +/// The teaching echo for a `Query`-outcome command (ADR-0038). +/// +/// `show data` is the only DSL-form query that echoes — a `Command::Select` +/// is already SQL, so it has none. Fires only in an advanced effective mode. +/// +/// `primary_key` is the table's primary-key columns, sourced from the +/// schema *after* execution: the limited form (`show data T limit n`) +/// orders by the primary key for a stable "first n", and that column list +/// is not carried on the `Command`. It is unused when the query is +/// unlimited. This is the one Bucket A row that needs schema info beyond +/// the `Command` (handoff §5 / ADR-0038 §4). +#[must_use] +pub fn echo_for_query( + command: &Command, + mode: EffectiveMode, + primary_key: &[String], +) -> Option { + if !mode.is_advanced() { + return None; + } + match command { + Command::ShowData { + name, + filter, + limit, + } => Some(render_show_data(name, filter.as_ref(), *limit, primary_key)), + _ => None, + } +} + /// Render the equivalent advanced-mode SQL for a DSL-form command, or -/// `None` when it has no echo. +/// `None` when it has no echo (Bucket C, or a form covered elsewhere — +/// `show data` goes through [`echo_for_query`]). #[must_use] pub fn command_to_sql(command: &Command) -> Option { match command { @@ -43,7 +82,88 @@ pub fn command_to_sql(command: &Command) -> Option { columns, primary_key, } => Some(render_create_table(name, columns, primary_key)), - // Remaining Bucket A/B forms land in follow-up slices (ADR-0038 §8). + Command::AddColumn { + table, + column, + ty, + not_null, + unique, + default, + check, + } => { + let mut s = format!("ALTER TABLE {table} ADD COLUMN {column} {}", ty.keyword()); + append_constraints(&mut s, *not_null, *unique, default.as_ref(), check.as_ref()); + Some(s) + } + // `--cascade` also drops the column's covering indexes — a + // multi-statement echo (Bucket B / category 2, Phase 2). The plain + // drop is a single statement. + Command::DropColumn { + table, + column, + cascade, + } => (!cascade).then(|| format!("ALTER TABLE {table} DROP COLUMN {column}")), + Command::RenameColumn { table, old, new } => { + Some(format!("ALTER TABLE {table} RENAME COLUMN {old} TO {new}")) + } + // The headline form (every conversion mode emits it); the + // `--dont-convert` *caveat* line is category-3 (ADR-0038 §6, Phase 3). + // `SET DATA TYPE` is the canonical ISO spelling (ADR-0035 Amendment 2). + Command::ChangeColumnType { + table, column, ty, .. + } => Some(format!( + "ALTER TABLE {table} ALTER COLUMN {column} SET DATA TYPE {}", + ty.keyword() + )), + Command::AddConstraint { + table, + column, + constraint, + } => Some(match constraint { + Constraint::NotNull => { + format!("ALTER TABLE {table} ALTER COLUMN {column} SET NOT NULL") + } + Constraint::Default(v) => format!( + "ALTER TABLE {table} ALTER COLUMN {column} SET DEFAULT {}", + value_to_sql_literal(v) + ), + Constraint::Unique => format!("ALTER TABLE {table} ADD UNIQUE ({column})"), + Constraint::Check(e) => { + format!("ALTER TABLE {table} ADD CHECK ({})", expr_to_sql(e)) + } + }), + Command::DropConstraint { + table, + column, + kind, + } => match kind { + ConstraintKind::NotNull => { + Some(format!("ALTER TABLE {table} ALTER COLUMN {column} DROP NOT NULL")) + } + ConstraintKind::Default => { + Some(format!("ALTER TABLE {table} ALTER COLUMN {column} DROP DEFAULT")) + } + // A column-level UNIQUE / CHECK is anonymous in our model — + // no portable name to DROP CONSTRAINT by, so no echo (Bucket C, + // ADR-0035 Amendment 2 residual gap / ADR-0038 §7). + ConstraintKind::Unique | ConstraintKind::Check => None, + }, + // Only the `--all-rows` fall-throughs echo: a WHERE-filtered + // update / delete routes SQL-first in advanced mode (`Sql*`), so it + // is already SQL and never reaches here as a DSL-form command + // (ADR-0033 Amendment 3/4, ADR-0038 §7). + Command::Update { + table, + assignments, + filter: RowFilter::AllRows, + } => Some(format!("UPDATE {table} SET {}", render_assignments(assignments))), + Command::Delete { + table, + filter: RowFilter::AllRows, + } => Some(format!("DELETE FROM {table}")), + // Remaining forms: Bucket B (resolved names / multi-line — Phase 2), + // category-3 prose (Phase 3), Bucket C (no echo), `show data` + // (`echo_for_query`), and the `Sql*` / `App` variants. _ => None, } } @@ -63,12 +183,10 @@ fn render_create_table(name: &str, columns: &[ColumnSpec], primary_key: &[String if inline_pk && c.name == primary_key[0] { s.push_str(" PRIMARY KEY"); } - if c.not_null { - s.push_str(" NOT NULL"); - } - if c.unique { - s.push_str(" UNIQUE"); - } + // The same column-constraint suffix `add column` emits (ADR-0029): + // simple-mode `create table` can carry `default` / `check` too, so + // the echo must render them or it is not equivalent (§1 contract). + append_constraints(&mut s, c.not_null, c.unique, c.default.as_ref(), c.check.as_ref()); s }) .collect(); @@ -83,9 +201,166 @@ fn render_create_table(name: &str, columns: &[ColumnSpec], primary_key: &[String } } +/// `SELECT * FROM [WHERE …] [ORDER BY LIMIT n]` — the `show +/// data` echo (ADR-0038 §7). When `limit` is set the worker orders by the +/// primary key for a stable "first n" (`build_query_data_sql`); the echo +/// reproduces that `ORDER BY` so it has the same effect (§1). The +/// `ORDER BY` is dropped when the table has no primary key, matching the +/// worker. +fn render_show_data( + name: &str, + filter: Option<&Expr>, + limit: Option, + primary_key: &[String], +) -> String { + let mut s = format!("SELECT * FROM {name}"); + if let Some(expr) = filter { + s.push_str(&format!(" WHERE {}", expr_to_sql(expr))); + } + if let Some(n) = limit { + if !primary_key.is_empty() { + s.push_str(&format!(" ORDER BY {}", primary_key.join(", "))); + } + s.push_str(&format!(" LIMIT {n}")); + } + s +} + +/// Append the `NOT NULL` / `UNIQUE` / `DEFAULT` / `CHECK` column-constraint +/// suffix (ADR-0029). The advanced-mode column-constraint grammar is +/// order-independent (`Repeated(Choice…)`, ADR-0035 §4a), so this fixed +/// order round-trips. Used by both `create table` and `add column`. +fn append_constraints( + s: &mut String, + not_null: bool, + unique: bool, + default: Option<&Value>, + check: Option<&Expr>, +) { + if not_null { + s.push_str(" NOT NULL"); + } + if unique { + s.push_str(" UNIQUE"); + } + if let Some(v) = default { + s.push_str(&format!(" DEFAULT {}", value_to_sql_literal(v))); + } + if let Some(e) = check { + s.push_str(&format!(" CHECK ({})", expr_to_sql(e))); + } +} + +/// ` = , …` — an `UPDATE … SET` assignment list. +fn render_assignments(assignments: &[(String, Value)]) -> String { + assignments + .iter() + .map(|(col, val)| format!("{col} = {}", value_to_sql_literal(val))) + .collect::>() + .join(", ") +} + +/// A `Value` as a runnable SQL literal (ADR-0038 §5). Most forms reuse +/// `Value`'s own `Display` (`'O''Hara'`, bare numbers, `true`/`false`, +/// quoted ISO dates); only `NULL` differs — `Display` writes lowercase +/// `null`, but §5 (and the advanced grammar's canonical form) want `NULL`. +fn value_to_sql_literal(value: &Value) -> String { + match value { + Value::Null => "NULL".to_string(), + other => other.to_string(), + } +} + +/// A WHERE / CHECK [`Expr`] as advanced-mode SQL. Mirrors the worker's +/// `compile_expr` operator spellings (`<>`, `LIKE`, `BETWEEN`, `IN`, +/// `IS NULL`, parenthesised `AND` / `OR` / `NOT`) but emits **bare column +/// identifiers** (the echo's unquoted style, matching `render_create_table`) +/// and **inline literals** instead of `?` placeholders, so the line is +/// runnable (§1). +fn expr_to_sql(expr: &Expr) -> String { + match expr { + Expr::Or(terms) => join_terms(terms, "OR"), + Expr::And(terms) => join_terms(terms, "AND"), + Expr::Not(inner) => format!("(NOT {})", expr_to_sql(inner)), + Expr::Predicate(p) => predicate_to_sql(p), + } +} + +fn join_terms(terms: &[Expr], op: &str) -> String { + let parts: Vec = terms.iter().map(expr_to_sql).collect(); + format!("({})", parts.join(&format!(" {op} "))) +} + +fn predicate_to_sql(predicate: &Predicate) -> String { + match predicate { + Predicate::Compare { left, op, right } => format!( + "{} {} {}", + operand_to_sql(left), + compare_op_sql(*op), + operand_to_sql(right) + ), + Predicate::Like { + target, + pattern, + negated, + } => { + let not = if *negated { "NOT " } else { "" }; + format!("{} {not}LIKE {}", operand_to_sql(target), operand_to_sql(pattern)) + } + Predicate::Between { + target, + low, + high, + negated, + } => { + let not = if *negated { "NOT " } else { "" }; + format!( + "{} {not}BETWEEN {} AND {}", + operand_to_sql(target), + operand_to_sql(low), + operand_to_sql(high) + ) + } + Predicate::In { + target, + items, + negated, + } => { + let not = if *negated { "NOT " } else { "" }; + let rendered: Vec = items.iter().map(operand_to_sql).collect(); + format!("{} {not}IN ({})", operand_to_sql(target), rendered.join(", ")) + } + Predicate::IsNull { target, negated } => { + let not = if *negated { "NOT " } else { "" }; + format!("{} IS {not}NULL", operand_to_sql(target)) + } + } +} + +fn operand_to_sql(operand: &Operand) -> String { + match operand { + Operand::Column { name, .. } => name.clone(), + Operand::Literal { value, .. } => value_to_sql_literal(value), + } +} + +/// The SQL spelling of a comparison operator — `<>` for inequality, the +/// standard form (ADR-0026 §6), matching the worker's `compare_op_sql`. +const fn compare_op_sql(op: CompareOp) -> &'static str { + match op { + CompareOp::Eq => "=", + CompareOp::NotEq => "<>", + CompareOp::Lt => "<", + CompareOp::LtEq => "<=", + CompareOp::Gt => ">", + CompareOp::GtEq => ">=", + } +} + #[cfg(test)] mod tests { use super::*; + use crate::dsl::command::{ChangeColumnMode, ConstraintKind}; use crate::dsl::types::Type; fn create_table(name: &str, cols: Vec, pk: &[&str]) -> Command { @@ -96,6 +371,28 @@ mod tests { } } + /// A `column = value` equality predicate, the most common WHERE leaf. + fn eq(column: &str, value: Value) -> Expr { + Expr::Predicate(Predicate::Compare { + left: Operand::Column { + name: column.to_string(), + span: Operand::NO_SPAN, + }, + op: CompareOp::Eq, + right: Operand::Literal { + value, + span: Operand::NO_SPAN, + }, + }) + } + + /// Parse `sql` through the advanced-mode walker (the round-trip target). + fn reparse(sql: &str) -> Result { + crate::dsl::parser::parse_command_in_mode(sql, crate::mode::Mode::Advanced) + } + + // --- create table (ADR-0038 §7 Bucket A) ------------------------- + #[test] fn create_table_single_serial_pk_renders_inline() { let cmd = create_table("Other", vec![ColumnSpec::new("id", Type::Serial)], &["id"]); @@ -118,18 +415,431 @@ mod tests { ); } + #[test] + fn create_table_renders_default_and_check_constraints() { + // §1 copy-paste contract: simple-mode `create table` can carry + // per-column `default` / `check` (ADR-0029), so the echo must too, + // or it is not equivalent. + let age = ColumnSpec { + check: Some(Expr::Predicate(Predicate::Compare { + left: Operand::Column { + name: "age".to_string(), + span: Operand::NO_SPAN, + }, + op: CompareOp::GtEq, + right: Operand::Literal { + value: Value::Number("0".to_string()), + span: Operand::NO_SPAN, + }, + })), + ..ColumnSpec::new("age", Type::Int) + }; + let grade = ColumnSpec { + default: Some(Value::Text("A".to_string())), + ..ColumnSpec::new("grade", Type::Text) + }; + let cmd = create_table("T", vec![ColumnSpec::new("id", Type::Serial), age, grade], &["id"]); + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!( + sql, + "CREATE TABLE T (id serial PRIMARY KEY, age int CHECK (age >= 0), grade text DEFAULT 'A')" + ); + assert!(matches!(reparse(&sql), Ok(Command::SqlCreateTable { .. }))); + } + #[test] fn create_table_echo_round_trips_in_advanced_mode() { // ADR-0038 §1 copy-paste contract: the echo is runnable advanced SQL. let cmd = create_table("Other", vec![ColumnSpec::new("id", Type::Serial)], &["id"]); let sql = command_to_sql(&cmd).expect("echo"); - let reparsed = crate::dsl::parser::parse_command_in_mode(&sql, crate::mode::Mode::Advanced); - assert!( - matches!(reparsed, Ok(Command::SqlCreateTable { .. })), - "echo must round-trip as runnable advanced SQL; got {reparsed:?}" + assert!(matches!(reparse(&sql), Ok(Command::SqlCreateTable { .. }))); + } + + // --- add column -------------------------------------------------- + + #[test] + fn add_column_renders_type_and_constraints_and_round_trips() { + let cmd = Command::AddColumn { + table: "T".to_string(), + column: "note".to_string(), + ty: Type::Text, + not_null: true, + unique: false, + default: Some(Value::Text("n/a".to_string())), + check: None, + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ADD COLUMN note text NOT NULL DEFAULT 'n/a'"); + assert!(matches!( + reparse(&sql), + Ok(Command::SqlAlterTable { .. }) + )); + } + + #[test] + fn add_column_with_unique_and_check_round_trips() { + // ADD COLUMN's constraint grammar is its own production — confirm + // the full UNIQUE / CHECK suffix round-trips there too, not just on + // CREATE TABLE. + let cmd = Command::AddColumn { + table: "T".to_string(), + column: "score".to_string(), + ty: Type::Int, + not_null: false, + unique: true, + default: None, + check: Some(Expr::Predicate(Predicate::Compare { + left: Operand::Column { + name: "score".to_string(), + span: Operand::NO_SPAN, + }, + op: CompareOp::GtEq, + right: Operand::Literal { + value: Value::Number("0".to_string()), + span: Operand::NO_SPAN, + }, + })), + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ADD COLUMN score int UNIQUE CHECK (score >= 0)"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + // --- drop column ------------------------------------------------- + + #[test] + fn drop_column_plain_round_trips() { + let cmd = Command::DropColumn { + table: "T".to_string(), + column: "c".to_string(), + cascade: false, + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "ALTER TABLE T DROP COLUMN c"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + #[test] + fn drop_column_cascade_has_no_echo_yet() { + // Multi-statement (drops covering indexes too) — Bucket B, Phase 2. + let cmd = Command::DropColumn { + table: "T".to_string(), + column: "c".to_string(), + cascade: true, + }; + assert!(command_to_sql(&cmd).is_none()); + } + + // --- rename column ----------------------------------------------- + + #[test] + fn rename_column_round_trips() { + let cmd = Command::RenameColumn { + table: "T".to_string(), + old: "a".to_string(), + new: "b".to_string(), + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "ALTER TABLE T RENAME COLUMN a TO b"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + // --- change column type ------------------------------------------ + + #[test] + fn change_column_renders_set_data_type_for_every_mode() { + for mode in [ + ChangeColumnMode::Default, + ChangeColumnMode::ForceConversion, + ChangeColumnMode::DontConvert, + ] { + let cmd = Command::ChangeColumnType { + table: "T".to_string(), + column: "c".to_string(), + ty: Type::Text, + mode, + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ALTER COLUMN c SET DATA TYPE text"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + } + + #[test] + fn change_column_to_playground_type_keyword_round_trips() { + let cmd = Command::ChangeColumnType { + table: "T".to_string(), + column: "code".to_string(), + ty: Type::ShortId, + mode: ChangeColumnMode::Default, + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ALTER COLUMN code SET DATA TYPE shortid"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + // --- add constraint ---------------------------------------------- + + fn add_constraint(constraint: Constraint) -> Command { + Command::AddConstraint { + table: "T".to_string(), + column: "c".to_string(), + constraint, + } + } + + #[test] + fn add_constraint_not_null_round_trips() { + let sql = command_to_sql(&add_constraint(Constraint::NotNull)).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ALTER COLUMN c SET NOT NULL"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + #[test] + fn add_constraint_default_round_trips() { + let sql = command_to_sql(&add_constraint(Constraint::Default(Value::Number( + "0".to_string(), + )))) + .expect("echo"); + assert_eq!(sql, "ALTER TABLE T ALTER COLUMN c SET DEFAULT 0"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + #[test] + fn add_constraint_unique_round_trips() { + let sql = command_to_sql(&add_constraint(Constraint::Unique)).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ADD UNIQUE (c)"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + #[test] + fn add_constraint_check_round_trips() { + let expr = Expr::Predicate(Predicate::Compare { + left: Operand::Column { + name: "c".to_string(), + span: Operand::NO_SPAN, + }, + op: CompareOp::Gt, + right: Operand::Literal { + value: Value::Number("0".to_string()), + span: Operand::NO_SPAN, + }, + }); + let sql = command_to_sql(&add_constraint(Constraint::Check(expr))).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ADD CHECK (c > 0)"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + // --- drop constraint --------------------------------------------- + + fn drop_constraint(kind: ConstraintKind) -> Command { + Command::DropConstraint { + table: "T".to_string(), + column: "c".to_string(), + kind, + } + } + + #[test] + fn drop_constraint_not_null_round_trips() { + let sql = command_to_sql(&drop_constraint(ConstraintKind::NotNull)).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ALTER COLUMN c DROP NOT NULL"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + #[test] + fn drop_constraint_default_round_trips() { + let sql = command_to_sql(&drop_constraint(ConstraintKind::Default)).expect("echo"); + assert_eq!(sql, "ALTER TABLE T ALTER COLUMN c DROP DEFAULT"); + assert!(matches!(reparse(&sql), Ok(Command::SqlAlterTable { .. }))); + } + + #[test] + fn drop_constraint_unique_and_check_have_no_echo() { + // Column-level UNIQUE / CHECK is anonymous — Bucket C (ADR-0038 §7). + assert!(command_to_sql(&drop_constraint(ConstraintKind::Unique)).is_none()); + assert!(command_to_sql(&drop_constraint(ConstraintKind::Check)).is_none()); + } + + // --- update / delete --all-rows ---------------------------------- + + #[test] + fn update_all_rows_round_trips() { + let cmd = Command::Update { + table: "T".to_string(), + assignments: vec![ + ("status".to_string(), Value::Text("done".to_string())), + ("score".to_string(), Value::Number("10".to_string())), + ], + filter: RowFilter::AllRows, + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "UPDATE T SET status = 'done', score = 10"); + assert!(matches!(reparse(&sql), Ok(Command::SqlUpdate { .. }))); + } + + #[test] + fn update_with_where_has_no_echo() { + // A WHERE-filtered update is SQL-first in advanced mode (SqlUpdate). + let cmd = Command::Update { + table: "T".to_string(), + assignments: vec![("a".to_string(), Value::Number("1".to_string()))], + filter: RowFilter::Where(eq("id", Value::Number("1".to_string()))), + }; + assert!(command_to_sql(&cmd).is_none()); + } + + #[test] + fn delete_all_rows_round_trips() { + let cmd = Command::Delete { + table: "T".to_string(), + filter: RowFilter::AllRows, + }; + let sql = command_to_sql(&cmd).expect("echo"); + assert_eq!(sql, "DELETE FROM T"); + assert!(matches!(reparse(&sql), Ok(Command::SqlDelete { .. }))); + } + + #[test] + fn delete_with_where_has_no_echo() { + let cmd = Command::Delete { + table: "T".to_string(), + filter: RowFilter::Where(eq("id", Value::Number("1".to_string()))), + }; + assert!(command_to_sql(&cmd).is_none()); + } + + // --- show data (echo_for_query) ---------------------------------- + + #[test] + fn show_data_plain_round_trips() { + let cmd = Command::ShowData { + name: "T".to_string(), + filter: None, + limit: None, + }; + let sql = echo_for_query(&cmd, EffectiveMode::AdvancedPersistent, &[]).expect("echo"); + assert_eq!(sql, "SELECT * FROM T"); + assert!(matches!(reparse(&sql), Ok(Command::Select { .. }))); + } + + #[test] + fn show_data_with_where_round_trips() { + let cmd = Command::ShowData { + name: "T".to_string(), + filter: Some(eq("name", Value::Text("Bob".to_string()))), + limit: None, + }; + let sql = echo_for_query(&cmd, EffectiveMode::AdvancedPersistent, &[]).expect("echo"); + assert_eq!(sql, "SELECT * FROM T WHERE name = 'Bob'"); + assert!(matches!(reparse(&sql), Ok(Command::Select { .. }))); + } + + #[test] + fn show_data_with_limit_orders_by_primary_key() { + let cmd = Command::ShowData { + name: "T".to_string(), + filter: None, + limit: Some(5), + }; + let pk = vec!["id".to_string()]; + let sql = echo_for_query(&cmd, EffectiveMode::AdvancedPersistent, &pk).expect("echo"); + assert_eq!(sql, "SELECT * FROM T ORDER BY id LIMIT 5"); + assert!(matches!(reparse(&sql), Ok(Command::Select { .. }))); + } + + #[test] + fn show_data_with_limit_and_compound_pk_orders_by_all_pk_columns() { + let cmd = Command::ShowData { + name: "T".to_string(), + filter: Some(eq("active", Value::Bool(true))), + limit: Some(3), + }; + let pk = vec!["a".to_string(), "b".to_string()]; + let sql = echo_for_query(&cmd, EffectiveMode::AdvancedPersistent, &pk).expect("echo"); + assert_eq!(sql, "SELECT * FROM T WHERE active = true ORDER BY a, b LIMIT 3"); + assert!(matches!(reparse(&sql), Ok(Command::Select { .. }))); + } + + #[test] + fn show_data_with_limit_but_no_primary_key_omits_order_by() { + // Matches the worker: no PK → no ORDER BY (build_query_data_sql). + let cmd = Command::ShowData { + name: "T".to_string(), + filter: None, + limit: Some(2), + }; + let sql = echo_for_query(&cmd, EffectiveMode::AdvancedPersistent, &[]).expect("echo"); + assert_eq!(sql, "SELECT * FROM T LIMIT 2"); + } + + #[test] + fn show_data_is_silent_in_simple_mode() { + let cmd = Command::ShowData { + name: "T".to_string(), + filter: None, + limit: None, + }; + assert!(echo_for_query(&cmd, EffectiveMode::Simple, &[]).is_none()); + } + + #[test] + fn select_is_not_echoed_as_a_query() { + // A `Command::Select` is already SQL — no echo (ADR-0038 §7). + let cmd = Command::Select { + sql: "select * from T".to_string(), + }; + assert!(echo_for_query(&cmd, EffectiveMode::AdvancedPersistent, &[]).is_none()); + } + + // --- expr / literal rendering ------------------------------------ + + #[test] + fn expr_renders_boolean_combinators_and_operators() { + // age >= 18 AND (name <> 'x' OR active = true) + let expr = Expr::And(vec![ + Expr::Predicate(Predicate::Compare { + left: Operand::Column { + name: "age".to_string(), + span: Operand::NO_SPAN, + }, + op: CompareOp::GtEq, + right: Operand::Literal { + value: Value::Number("18".to_string()), + span: Operand::NO_SPAN, + }, + }), + Expr::Or(vec![ + Expr::Predicate(Predicate::Compare { + left: Operand::Column { + name: "name".to_string(), + span: Operand::NO_SPAN, + }, + op: CompareOp::NotEq, + right: Operand::Literal { + value: Value::Text("x".to_string()), + span: Operand::NO_SPAN, + }, + }), + eq("active", Value::Bool(true)), + ]), + ]); + assert_eq!( + expr_to_sql(&expr), + "(age >= 18 AND (name <> 'x' OR active = true))" ); } + #[test] + fn value_literal_renders_null_uppercase_and_quotes_text() { + assert_eq!(value_to_sql_literal(&Value::Null), "NULL"); + assert_eq!(value_to_sql_literal(&Value::Text("O'Hara".to_string())), "'O''Hara'"); + assert_eq!(value_to_sql_literal(&Value::Number("3.14".to_string())), "3.14"); + assert_eq!(value_to_sql_literal(&Value::Bool(false)), "false"); + } + + // --- gating ------------------------------------------------------ + #[test] fn echo_for_gates_on_advanced_mode() { let cmd = create_table("Other", vec![ColumnSpec::new("id", Type::Serial)], &["id"]); diff --git a/src/event.rs b/src/event.rs index 8b12ccf..963c8de 100644 --- a/src/event.rs +++ b/src/event.rs @@ -61,8 +61,15 @@ pub enum AppEvent { command: Command, name: String, }, - /// A `show data` query succeeded. - DslDataSucceeded { command: Command, data: DataResult }, + /// A `show data` query succeeded. `echo` is the DSL → SQL teaching + /// echo (ADR-0038) — built post-execution because the limited form + /// orders by the table's primary key (handoff §5). `None` for a + /// SQL-entered `SELECT` or any simple-mode submission. + DslDataSucceeded { + command: Command, + data: DataResult, + echo: Option, + }, /// An `explain …` command succeeded (ADR-0028). `plan` /// carries the captured query plan; nothing was executed. DslExplainSucceeded { command: Command, plan: QueryPlan }, @@ -73,10 +80,18 @@ pub enum AppEvent { DslUpdateSucceeded { command: Command, result: UpdateResult, + /// The DSL → SQL teaching echo (ADR-0038): `UPDATE T SET …` for an + /// `update … --all-rows` fall-through. `None` for a SQL-entered + /// `UPDATE` or any simple-mode submission. + echo: Option, }, DslDeleteSucceeded { command: Command, result: DeleteResult, + /// The DSL → SQL teaching echo (ADR-0038): `DELETE FROM T` for a + /// `delete … --all-rows` fall-through. `None` for a SQL-entered + /// `DELETE` or any simple-mode submission. + echo: Option, }, /// A `change column …` succeeded. `result` carries both the /// post-rebuild description (for the auto-show) and the @@ -84,6 +99,10 @@ pub enum AppEvent { DslChangeColumnSucceeded { command: Command, result: ChangeColumnTypeResult, + /// The DSL → SQL teaching echo (ADR-0038): `ALTER TABLE T ALTER + /// COLUMN c SET DATA TYPE …`. `None` in simple mode. (The + /// `--dont-convert` caveat line is category-3, a later slice.) + echo: Option, }, /// An `add column …` succeeded. `result` carries the /// post-add description plus any `[client-side]` notes @@ -91,6 +110,9 @@ pub enum AppEvent { DslAddColumnSucceeded { command: Command, result: AddColumnResult, + /// The DSL → SQL teaching echo (ADR-0038): `ALTER TABLE T ADD + /// COLUMN c …`. `None` in simple mode. + echo: Option, }, /// A `drop column …` succeeded. `result` carries the /// post-drop description plus the names of any indexes @@ -98,6 +120,10 @@ pub enum AppEvent { DslDropColumnSucceeded { command: Command, result: DropColumnResult, + /// The DSL → SQL teaching echo (ADR-0038): `ALTER TABLE T DROP + /// COLUMN c` for a plain (non-`--cascade`) drop. `None` in simple + /// mode, and for `--cascade` (a multi-statement echo, Phase 2). + echo: Option, }, /// A DSL command failed. `error` is the structured /// payload, `facts` is the runtime-built schema-resolved diff --git a/src/runtime.rs b/src/runtime.rs index f2a33d9..15e5065 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1292,10 +1292,19 @@ fn spawn_dsl_dispatch( name, } } - Ok(CommandOutcome::Query(data)) => AppEvent::DslDataSucceeded { - command: command.clone(), - data, - }, + Ok(CommandOutcome::Query(data)) => { + // ADR-0038: `show data` is the only DSL-form query that + // echoes; its limited form orders by the table's primary + // key, which is not on the Command — so the echo is built + // post-execution from the schema (handoff §5). A + // SQL-entered `SELECT` (also a Query outcome) has no echo. + let echo = build_show_data_echo(&database, &command, submission_mode).await; + AppEvent::DslDataSucceeded { + command: command.clone(), + data, + echo, + } + } Ok(CommandOutcome::QueryPlan(plan)) => AppEvent::DslExplainSucceeded { command: command.clone(), plan, @@ -1307,22 +1316,27 @@ fn spawn_dsl_dispatch( Ok(CommandOutcome::Update(result)) => AppEvent::DslUpdateSucceeded { command: command.clone(), result, + echo, }, Ok(CommandOutcome::Delete(result)) => AppEvent::DslDeleteSucceeded { command: command.clone(), result, + echo, }, Ok(CommandOutcome::ChangeColumn(result)) => AppEvent::DslChangeColumnSucceeded { command: command.clone(), result, + echo, }, Ok(CommandOutcome::AddColumn(result)) => AppEvent::DslAddColumnSucceeded { command: command.clone(), result, + echo, }, Ok(CommandOutcome::DropColumn(result)) => AppEvent::DslDropColumnSucceeded { command: command.clone(), result, + echo, }, Err(DbError::PersistenceFatal { operation, @@ -1365,6 +1379,50 @@ fn spawn_dsl_dispatch( }); } +/// Build the `show data` DSL → SQL teaching echo (ADR-0038). +/// +/// `show data` is the one Bucket A row whose echo needs schema info beyond +/// the `Command`: the limited form (`show data T limit n`) orders by the +/// table's primary key for a stable "first n" (the worker's +/// `build_query_data_sql`), and that column list is not on the `Command`. +/// So when — and only when — the query is limited, this resolves the +/// primary key via `describe_table` (the same best-effort schema lookup +/// `enrich_dsl_failure` uses) and feeds it to [`crate::echo::echo_for_query`]. +/// +/// Silent in simple mode (gated before any lookup) and for a SQL-entered +/// `SELECT` (not a `ShowData`). A describe failure or a primary-key-less +/// table simply drops the `ORDER BY`, exactly as the worker does. +async fn build_show_data_echo( + database: &Database, + command: &Command, + submission_mode: crate::app::EffectiveMode, +) -> Option { + if !submission_mode.is_advanced() { + return None; + } + // The primary key is needed only for the `ORDER BY` of a limited query; + // skip the lookup otherwise so the common case stays round-trip-free. + let primary_key = match command { + Command::ShowData { + name, + limit: Some(_), + .. + } => database + .describe_table(name.clone(), None) + .await + .map(|desc| { + desc.columns + .iter() + .filter(|c| c.primary_key) + .map(|c| c.name.clone()) + .collect::>() + }) + .unwrap_or_default(), + _ => Vec::new(), + }; + crate::echo::echo_for_query(command, submission_mode, &primary_key) +} + /// Build schema-resolved enrichment for a DSL failure (ADR-0019 §6). /// /// Best-effort: every lookup is independently fallible and a @@ -2516,4 +2574,60 @@ mod tests { assert_eq!(d.visible(), None); assert!(!d.is_armed()); } + + // --- ADR-0038: the `show data` teaching echo's primary-key sourcing --- + + /// End-to-end cover for `build_show_data_echo` against a real worker: + /// the limited `show data` echo orders by the table's primary key, + /// resolved from the schema post-execution (handoff §5 / ADR-0038 §4). + /// The pure renderer is unit-tested in `echo`; this pins the describe → + /// PK → `ORDER BY` glue, plus the simple-mode gate and the + /// unlimited-no-lookup path. + #[tokio::test] + async fn show_data_echo_orders_by_resolved_primary_key_when_limited() { + use crate::app::EffectiveMode; + use crate::db::Database; + use crate::dsl::Command; + use crate::dsl::command::ColumnSpec; + use crate::dsl::types::Type; + + let db = Database::open(":memory:").expect("open in-memory"); + db.create_table( + "Customers".to_string(), + vec![ + ColumnSpec::new("id", Type::Serial), + ColumnSpec::new("name", Type::Text), + ], + vec!["id".to_string()], + None, + ) + .await + .expect("create table"); + + let limited = Command::ShowData { + name: "Customers".to_string(), + filter: None, + limit: Some(5), + }; + // Limited → ORDER BY the resolved primary key. + assert_eq!( + super::build_show_data_echo(&db, &limited, EffectiveMode::AdvancedPersistent).await, + Some("SELECT * FROM Customers ORDER BY id LIMIT 5".to_string()), + ); + // Simple mode → silent, gated before any lookup. + assert_eq!( + super::build_show_data_echo(&db, &limited, EffectiveMode::Simple).await, + None, + ); + // Unlimited → no describe, no ORDER BY. + let unlimited = Command::ShowData { + name: "Customers".to_string(), + filter: None, + limit: None, + }; + assert_eq!( + super::build_show_data_echo(&db, &unlimited, EffectiveMode::AdvancedPersistent).await, + Some("SELECT * FROM Customers".to_string()), + ); + } } diff --git a/tests/walking_skeleton.rs b/tests/walking_skeleton.rs index f9b4241..d093585 100644 --- a/tests/walking_skeleton.rs +++ b/tests/walking_skeleton.rs @@ -602,6 +602,7 @@ fn show_data_for_empty_table_renders_placeholder() { limit: None, }, data, + echo: None, }); let rendered = rendered_text(&mut app, &Theme::dark(), 80, 24); assert!(rendered.contains("(no rows)"), "{rendered}");