diff --git a/src/dsl/walker/driver.rs b/src/dsl/walker/driver.rs index c63951a..a8baac5 100644 --- a/src/dsl/walker/driver.rs +++ b/src/dsl/walker/driver.rs @@ -356,6 +356,21 @@ fn walk_punct( ) -> NodeWalkResult { let bytes = source.as_bytes(); if position < bytes.len() && bytes[position] == ch as u8 { + // ADR-0033 Amendment 4: a `-` does not match when it begins an + // adjacent `--`. The playground supports no `--` line comment, and + // `--` is the DSL flag marker (e.g. `--all-rows`); treating `--` + // as two minus operators silently mis-parsed `set x = 42 + // --all-rows` as arithmetic over phantom columns `all`/`rows`. + // Refusing here makes the SQL expression stop, so the SQL shape + // fails and dispatch falls back to the DSL flag. Spaced `- -3` is + // unaffected (the dashes are not adjacent). `Node::Punct('-')` is + // used only by the SQL expression grammar, so this is scoped to it. + if ch == '-' && bytes.get(position + 1) == Some(&b'-') { + return NodeWalkResult::NoMatch { + position, + expected: vec![Expectation::Punct(ch)], + }; + } path.push(MatchedItem { kind: MatchedKind::Punct(ch), text: ch.to_string(), diff --git a/tests/sql_dml_e2e.rs b/tests/sql_dml_e2e.rs index 6c3778d..844a053 100644 --- a/tests/sql_dml_e2e.rs +++ b/tests/sql_dml_e2e.rs @@ -31,7 +31,9 @@ use rdbms_playground::app::App; use rdbms_playground::db::{Database, DbError, DeleteResult, InsertResult, UpdateResult}; use rdbms_playground::dsl::parser::parse_command_in_mode; use rdbms_playground::dsl::walker::Severity; -use rdbms_playground::dsl::{ColumnSpec, Command, ReferentialAction, Type, parse_command}; +use rdbms_playground::dsl::{ + ColumnSpec, Command, ReferentialAction, RowFilter, Type, parse_command, +}; use rdbms_playground::event::AppEvent; use rdbms_playground::mode::Mode; use rdbms_playground::persistence::Persistence; @@ -513,18 +515,36 @@ fn e2e_single_dml_statement_with_trailing_semicolon_parses() { } #[test] -fn e2e_update_all_rows_in_advanced_does_not_fall_back_to_dsl() { - // ADR-0033 Amendment 3 counter-example: the SQL `UPDATE`'s - // `SET ` absorbs `--all-rows` (as `42 - -all - rows`), so - // the SQL shape matches and there is no DSL fall-back. (Harmless - // at execution — the engine treats `--all-rows` as a line - // comment.) +fn e2e_update_all_rows_in_advanced_falls_back_to_dsl() { + // ADR-0033 Amendment 4 reverses Amendment 3's counter-example: the + // SQL `UPDATE`'s `SET` expression must NOT consume the DSL flag + // `--all-rows`. An adjacent `--` is not two minus operators — the + // playground has no `--` line comment — so the SQL shape fails and + // dispatch falls back to the DSL `Update { AllRows }`, mirroring + // `delete … --all-rows`. assert!( matches!( parse_command_in_mode("update Orders set total = 42 --all-rows", Mode::Advanced), + Ok(Command::Update { filter: RowFilter::AllRows, .. }) + ), + "advanced `update … --all-rows` falls back to the DSL Update", + ); + // Legitimate spaced arithmetic is unaffected — the dashes are not + // adjacent, so this stays a SQL UPDATE (total = 42 - (-3) = 45). + assert!( + matches!( + parse_command_in_mode("update Orders set total = 42 - -3", Mode::Advanced), Ok(Command::SqlUpdate { .. }) ), - "advanced `update … --all-rows` stays SQL (no DSL fall-back)", + "spaced `42 - -3` stays a SQL UPDATE", + ); + // An adjacent `--` before a number is no longer silently accepted as + // arithmetic; with no `--all-rows` flag to fall back to, it is a + // parse error (acceptable per Amendment 4 — contrived input, and the + // playground does not support `--` comments). + assert!( + parse_command_in_mode("update Orders set total = 42--3", Mode::Advanced).is_err(), + "adjacent `42--3` is a parse error (no `--` comment support)", ); } diff --git a/tests/sql_update.rs b/tests/sql_update.rs index 586cc61..b82ba3b 100644 --- a/tests/sql_update.rs +++ b/tests/sql_update.rs @@ -488,3 +488,38 @@ fn advanced_update_set_expression_still_parses_via_sql_expr() { ); } } + +// ================================================================= +// ADR-0033 Amendment 4 — `update … --all-rows` falls back to the DSL +// ================================================================= + +#[test] +fn update_all_rows_flag_in_advanced_updates_every_row() { + // `update … --all-rows` falls back to the DSL Update { AllRows } in + // advanced mode (run_replay parses each line in advanced mode) and + // updates every row — the full pipeline end to end, not just the + // parse-level dispatch (covered in tests/sql_dml_e2e.rs). + let (project, db, _dir) = open_project_db(); + let rt = rt(); + create_cols(&db, &rt, "t", &[("id", Type::Int), ("v", Type::Int)], &["id"]); + seed(&db, &rt, "insert into t (id, v) values (1, 1), (2, 2)", "t"); + std::fs::write( + project.path().join("allrows.commands"), + "update t set v = 9 --all-rows\n", + ) + .expect("write script"); + let events = rt.block_on(run_replay(&db, project.path(), "allrows.commands")); + assert!( + matches!(events.last(), Some(AppEvent::ReplayCompleted { count: 1, .. })), + "the --all-rows update replays through the DSL fall-back; events: {events:?}" + ); + let rows = rt + .block_on(db.query_data("t".to_string(), None, None, None)) + .expect("query") + .rows; + assert_eq!(rows.len(), 2, "both rows present"); + assert!( + rows.iter().all(|r| r[1].as_deref() == Some("9")), + "every row's v set to 9 (all-rows update); rows: {rows:?}" + ); +}