From 9a23e28f30160c28035a93ee88bd81e964374eec Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Wed, 27 May 2026 21:25:02 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20update=20=E2=80=A6=20--all-rows=20falls?= =?UTF-8?q?=20back=20to=20the=20DSL=20instead=20of=20misparsing=20(ADR-003?= =?UTF-8?q?3=20Am4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Advanced-mode `update T set x = 42 --all-rows` parsed the `--all-rows` DSL flag as the arithmetic `42 - -all - rows` over phantom columns `all`/`rows` (Amendment 3's counter-example), masked only by the engine's `--` comment leniency. The playground supports no `--` line comment, so this was a misparse (ADR-0027: flag input known to fail at runtime). Fix: walk_punct refuses a `-` that begins an adjacent `--`. Only the SQL expression uses Node::Punct('-'), so this is scoped to it. The SET expression then stops, the SQL UPDATE shape fails, and dispatch falls back to the DSL Update { AllRows } — symmetry with delete … --all-rows. Behaviour: `42 --all-rows` → DSL Update{AllRows}; spaced `42 - -3` stays SqlUpdate (= 45, preserved); adjacent `42--3` → parse error (contrived; no `--` comment support). Tests: inverted parse test (+ arithmetic-preserved + adjacent-error assertions); new full-pipeline update_all_rows_flag_in_advanced_updates_every_row. Suite 1963/0/1; clippy clean. --- src/dsl/walker/driver.rs | 15 +++++++++++++++ tests/sql_dml_e2e.rs | 36 ++++++++++++++++++++++++++++-------- tests/sql_update.rs | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 8 deletions(-) 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:?}" + ); +}