grammar+walker: 3j — shared insert/update/delete entry words (ADR-0033 §2 / Amendments 1 & 3)

Wire `insert`/`update`/`delete` as shared DSL/SQL entry words through the
category-grouped dispatcher (ADR-0033 Amendment 1): the Advanced SQL nodes
move off the dev words (`sqlinsert`/`sql_update`/`sql_delete`) to the real
keywords, registered alongside the Simple DSL nodes. Remove the dev-word
scaffold; collapse build_sql_{insert,update,delete} to source.trim();
de-duplicate the two REGISTRY entry-word listing sites.

Dispatch model (ADR-0033 Amendment 3, written this round):
- A command is the mode-rooted grammar-path outcome; identity is intrinsic.
  Advanced mode tries SQL first, falling back to the Simple DSL command when
  no SQL branch matches a token (`delete … --all-rows` falls back;
  `update … --all-rows` does not — the SET expression absorbs it, harmless
  since the engine treats `--all-rows` as a comment).
- Simple mode commits the DSL candidate for a shared word, surfacing the real
  DSL error; bare "this is SQL" is reserved for SQL-only entry words
  (`select`/`with`). A content rejection on the SQL candidate (internal
  table) is committed, never masked by the DSL fallback.

Combined DSL-error + advanced-SQL pointer (ADR-0033 Amendment 3): a Simple-mode
definite DSL error that would run as SQL in advanced mode gains the
`advanced_mode.also_valid_sql` suffix — in the live hint (ambient_hint_in_mode)
and on submit (dispatch_dsl), via the shared advanced_alternative_note — so the
actionable DSL fix and the mode pointer coexist (submit covers constructs that
surface only on submit, e.g. `delete … returning`).

Internal-table rejection symmetrised (/runda finding B, ADR-0030 §6): the DSL
data-command target slots (insert/update/delete/show data/show table) gained
reject_internal_table, so `__rdbms_*` tables are refused in Simple mode too —
previously only the advanced SQL grammar rejected them.

Mode-awareness: classify_input_with_schema_in_mode and
invalid_ident_at_cursor_in_mode stop leaking the advanced SQL view into
simple-mode hints for shared words.

Tests: dev-word inputs migrated to the real words (advanced); DSL grammar /
completion / phase-D / db tests parse in Simple mode (the DSL surface); replay
keeps its advanced-mode model (one stale assertion fixed); dispatcher routing,
combined-pointer, and internal-table tests added. Suite 1626 pass / 0 fail /
1 ignored; clippy --all-targets -D warnings clean.

Defer M4 (execution-time mode side-channel; tracked in requirements.md) to its
own ADR.
This commit is contained in:
claude@clouddev1
2026-05-23 21:13:39 +00:00
parent c16196fc7f
commit d5c7f63513
22 changed files with 956 additions and 314 deletions
+52
View File
@@ -1187,6 +1187,20 @@ impl App {
"parse.error",
detail = parse_error_message(&err)
));
// ADR-0033 Amendment 3: combine the DSL error with a
// pointer to advanced mode when the same line would
// run as SQL there. Only in simple mode (a one-shot
// `:` or persistent advanced submission uses the SQL
// surface already). This mirrors the live hint and
// covers SQL constructs that surface only on submit
// (e.g. `delete … returning`, where the live hint
// shows WHERE-completion rather than an error).
if submission_mode == Mode::Simple
&& let Some(note) =
crate::input_render::advanced_alternative_note(input, &self.schema_cache)
{
self.note_error(note);
}
// ADR-0021 §2: append the usage block (if a
// known command-entry keyword was consumed) or
// the available-commands fallback (§5).
@@ -2326,6 +2340,44 @@ mod tests {
);
}
#[test]
fn simple_mode_submit_of_sql_construct_appends_advanced_pointer() {
// ADR-0033 Amendment 3: submitting a line in simple mode that
// fails as DSL but would run as SQL in advanced mode appends
// the `advanced_mode.also_valid_sql` pointer to the parse
// error — keeping the DSL detail and pointing at advanced
// mode. Multi-row VALUES is a definite DSL error and valid SQL
// (no schema needed).
let mut app = App::new();
type_str(&mut app, "insert into T values (1, 2), (3, 4)");
let actions = submit(&mut app);
assert!(actions.is_empty(), "the bad line must not dispatch");
let has_pointer = app
.output
.iter()
.any(|l| l.kind == OutputKind::Error && l.text.contains("advanced mode"));
assert!(
has_pointer,
"expected the advanced-mode pointer on submit; output:\n{}",
error_lines(&app),
);
}
#[test]
fn simple_mode_submit_of_pure_dsl_error_has_no_advanced_pointer() {
// A DSL error that is *not* valid SQL either (unknown command)
// must not carry the advanced-mode pointer — there is nothing
// to switch modes for.
let mut app = App::new();
type_str(&mut app, "frobulate widgets");
let _ = submit(&mut app);
let has_pointer = app
.output
.iter()
.any(|l| l.text.contains("valid as SQL in advanced mode"));
assert!(!has_pointer, "unknown command must not point at advanced mode");
}
#[test]
fn enter_in_advanced_mode_dispatches_select_with_advanced_tag() {
// The pre-ADR-0030 placeholder echoed any advanced-mode
+58 -18
View File
@@ -973,6 +973,24 @@ pub fn invalid_ident_at_cursor(
input: &str,
cursor: usize,
cache: &SchemaCache,
) -> Option<InvalidIdent> {
invalid_ident_at_cursor_in_mode(input, cursor, cache, Mode::Advanced)
}
/// Mode-aware [`invalid_ident_at_cursor`].
///
/// The slot's expected set is computed in `mode`, so a simple-mode
/// caller doesn't get the advanced (SQL) grammar's view of a shared
/// `insert`/`update`/`delete` entry word — e.g. it won't flag `rows`
/// as an "unknown column" in `update T … --all-rows`, which is a DSL
/// flag in simple mode rather than the SQL expression `- -all - rows`
/// (ADR-0033 Amendment 3).
#[must_use]
pub fn invalid_ident_at_cursor_in_mode(
input: &str,
cursor: usize,
cache: &SchemaCache,
mode: Mode,
) -> Option<InvalidIdent> {
let cursor = cursor.min(input.len());
let bytes = input.as_bytes();
@@ -1000,7 +1018,7 @@ pub fn invalid_ident_at_cursor(
return None;
}
let leading = &input[..start];
let expected = expected_at(leading, Mode::Advanced);
let expected = expected_at(leading, mode);
if expected.is_empty() {
return None;
}
@@ -1094,6 +1112,17 @@ mod tests {
.map_or_else(Vec::new, |c| c.candidates.into_iter().map(|c| c.text).collect())
}
/// Simple-mode completion candidates — the DSL surface
/// (ADR-0003). Used by tests of DSL-only completion (the
/// `--all-rows` flag, the DSL value-literal slots), which since
/// sub-phase 3j must run in Simple mode: `insert`/`update`/
/// `delete` are shared entry words (ADR-0033 Amendment 3) and
/// Advanced mode surfaces the SQL grammar's completions instead.
fn cands_simple(input: &str, cursor: usize) -> Vec<String> {
candidates_at_cursor_in_mode(input, cursor, &SchemaCache::default(), Mode::Simple)
.map_or_else(Vec::new, |c| c.candidates.into_iter().map(|c| c.text).collect())
}
fn cand_kinds_with(
input: &str,
cursor: usize,
@@ -1178,14 +1207,17 @@ mod tests {
// a `,` (more assignments), `where` (where clause),
// or `--all-rows` (flag). Punctuation isn't surfaced;
// `where` and `--all-rows` should appear.
let cs = cands("update T set Name='hi' ", 23);
// `--all-rows` is a DSL-only rail (Simple mode); in Advanced
// mode `update`/`delete` route to the SQL grammar, which has
// no such flag (ADR-0033 Amendment 3).
let cs = cands_simple("update T set Name='hi' ", 23);
assert!(cs.contains(&"where".to_string()), "got {cs:?}");
assert!(cs.contains(&"--all-rows".to_string()), "got {cs:?}");
}
#[test]
fn delete_filter_position_offers_where_and_all_rows() {
let cs = cands("delete from T ", 14);
let cs = cands_simple("delete from T ", 14);
assert!(cs.contains(&"where".to_string()), "got {cs:?}");
assert!(cs.contains(&"--all-rows".to_string()), "got {cs:?}");
}
@@ -1195,12 +1227,18 @@ mod tests {
// Hint-panel colouring distinguishes flags from
// keywords (amber vs purple) — flags get their own
// CandidateKind so the renderer can apply tok_flag.
let kinds = candidates_at_cursor("delete from T ", 14, &SchemaCache::default())
.expect("some completion")
.candidates
.into_iter()
.map(|c| (c.text, c.kind))
.collect::<Vec<_>>();
// Simple mode: `--all-rows` is the DSL rail.
let kinds = candidates_at_cursor_in_mode(
"delete from T ",
14,
&SchemaCache::default(),
Mode::Simple,
)
.expect("some completion")
.candidates
.into_iter()
.map(|c| (c.text, c.kind))
.collect::<Vec<_>>();
let flag = kinds
.iter()
.find(|(t, _)| t == "--all-rows")
@@ -1210,7 +1248,7 @@ mod tests {
#[test]
fn flag_candidates_filter_by_partial_prefix() {
let cs = cands("delete from T --", 16);
let cs = cands_simple("delete from T --", 16);
assert!(cs.contains(&"--all-rows".to_string()), "got {cs:?}");
}
@@ -1327,7 +1365,9 @@ mod tests {
// `true`, `false` as Tab candidates — actively
// misleading at a slot where the user is more likely
// entering a number / text / date. Suppress.
let cs = cands("insert into T values (", 22);
// DSL value-literal slot (Simple mode); in Advanced mode
// `insert` routes to the SQL grammar (ADR-0033 Amendment 3).
let cs = cands_simple("insert into T values (", 22);
assert!(cs.is_empty(), "got misleading candidates {cs:?}");
}
@@ -1337,15 +1377,15 @@ mod tests {
// completion applies — `n` → `null`, `tr` → `true`,
// `fa` → `false`.
assert_eq!(
cands("insert into T values (n", 23),
cands_simple("insert into T values (n", 23),
vec!["null".to_string()],
);
assert_eq!(
cands("insert into T values (tr", 24),
cands_simple("insert into T values (tr", 24),
vec!["true".to_string()],
);
assert_eq!(
cands("insert into T values (fa", 24),
cands_simple("insert into T values (fa", 24),
vec!["false".to_string()],
);
}
@@ -1355,21 +1395,21 @@ mod tests {
// Comma-separated value positions all hit the same slot
// signature. `insert into T values (1, ` → expected:
// null/true/false/number/string. Suppress.
let cs = cands("insert into T values (1, ", 25);
let cs = cands_simple("insert into T values (1, ", 25);
assert!(cs.is_empty(), "got {cs:?}");
}
#[test]
fn update_set_value_slot_suppresses() {
// `update T set col=` is also a value-literal slot.
let cs = cands("update T set col=", 17);
let cs = cands_simple("update T set col=", 17);
assert!(cs.is_empty(), "got {cs:?}");
}
#[test]
fn where_value_slot_suppresses() {
// `where col=` is also a value-literal slot.
let cs = cands("delete from T where col=", 24);
let cs = cands_simple("delete from T where col=", 24);
assert!(cs.is_empty(), "got {cs:?}");
}
@@ -2166,7 +2206,7 @@ mod tests {
// ADR-0033 §9: `excluded.|` inside a DO UPDATE action
// completes to the INSERT target table's columns.
let cache = two_table_schema();
let input = "sqlinsert into a (id, name) values (1, 'x') \
let input = "insert into a (id, name) values (1, 'x') \
on conflict (id) do update set name = excluded.";
let cs = cands_with(input, input.len(), &cache);
assert!(
+13 -3
View File
@@ -9573,8 +9573,13 @@ mod tests {
/// Pull the `RowFilter` out of an `update` / `delete` parsed
/// from DSL — the readable way to build a complex `Expr`.
/// Parses in Simple mode: `update`/`delete` are shared entry
/// words since sub-phase 3j (ADR-0033 Amendment 3), so the DSL
/// `Command::Update`/`Delete` is only produced in Simple mode.
fn parse_filter(dsl: &str) -> RowFilter {
match crate::dsl::parser::parse_command(dsl).expect("filter parse") {
match crate::dsl::parser::parse_command_in_mode(dsl, crate::mode::Mode::Simple)
.expect("filter parse")
{
crate::dsl::command::Command::Update { filter, .. }
| crate::dsl::command::Command::Delete { filter, .. } => filter,
other => panic!("expected update/delete, got {other:?}"),
@@ -9775,9 +9780,14 @@ mod tests {
// --- explain / query plans (ADR-0028) -------------------
/// Parse a non-`explain` query command for use as the inner
/// command of `explain_query_plan`.
/// command of `explain_query_plan`. Simple mode: `explain`
/// wraps the DSL `show data` / `update` / `delete` commands
/// (ADR-0028; SQL DML is not explainable, ADR-0030 §13 OOS-2),
/// and `update`/`delete` only yield the DSL variant in Simple
/// mode (shared entry words since sub-phase 3j).
fn parse_inner(dsl: &str) -> Command {
crate::dsl::parser::parse_command(dsl).expect("inner command parse")
crate::dsl::parser::parse_command_in_mode(dsl, crate::mode::Mode::Simple)
.expect("inner command parse")
}
#[tokio::test]
+58 -73
View File
@@ -32,8 +32,14 @@ use crate::dsl::walker::outcome::{MatchedItem, MatchedKind, MatchedPath};
const TABLE_NAME_EXISTING: Node = Node::Ident {
source: IdentSource::Tables,
// Reject `__rdbms_*` internal tables at the table-source slot
// (ADR-0030 §6 — "every table-source slot"), matching the SQL
// grammar's `reject_internal_table`. Without this, simple-mode DSL
// data commands could read/write the internal metadata tables
// even though advanced-mode SQL rejects them (ADR-0033
// Amendment 3 / `/runda` finding B).
role: "table_name",
validator: None,
validator: Some(sql_select::reject_internal_table),
highlight_override: None,
writes_table: false,
writes_column: false,
@@ -49,8 +55,10 @@ writes_projection_alias: false,
/// dispatch typed slots per column.
const TABLE_NAME_INSERT: Node = Node::Ident {
source: IdentSource::Tables,
// Reject `__rdbms_*` internal tables (ADR-0030 §6; `/runda`
// finding B) — see `TABLE_NAME_EXISTING`.
role: "table_name",
validator: None,
validator: Some(sql_select::reject_internal_table),
highlight_override: None,
writes_table: true,
writes_column: false,
@@ -224,8 +232,12 @@ const INSERT_SHAPE: Node = Node::Seq(INSERT_NODES);
/// can resolve column types (Phase D).
const TABLE_NAME_WRITES: Node = Node::Ident {
source: IdentSource::Tables,
// Reject `__rdbms_*` internal tables (ADR-0030 §6; `/runda`
// finding B) — see `TABLE_NAME_EXISTING`. Shared by `update`,
// `delete`, and `show data`, so all three reject the internal
// metadata tables, matching the SQL grammar.
role: "table_name",
validator: None,
validator: Some(sql_select::reject_internal_table),
highlight_override: None,
writes_table: true,
writes_column: false,
@@ -856,14 +868,10 @@ fn build_select(_path: &MatchedPath, source: &str) -> Result<Command, Validation
}
/// Build `Command::SqlInsert` from a validated SQL `INSERT`
/// (ADR-0033 §1, sub-phase 3b). Extracts the target table from
/// the matched path so the worker re-persists the right CSV.
///
/// Dev-scaffold detail: the entry word is `sqlinsert` (not valid
/// SQL), so the statement is reconstructed as `insert` + the
/// matched tail. Sub-phase 3j wires the real `insert` entry word,
/// at which point this collapses to `source.trim()` like
/// `build_select`.
/// (ADR-0033 §1). Extracts the target table from the matched path
/// so the worker re-persists the right CSV. `insert` is now the
/// real (shared) entry word, so the validated `source` runs
/// verbatim — like `build_select` (sub-phase 3j).
fn build_sql_insert(path: &MatchedPath, source: &str) -> Result<Command, ValidationError> {
let target_table = path
.items
@@ -938,13 +946,10 @@ fn build_sql_insert(path: &MatchedPath, source: &str) -> Result<Command, Validat
.to_string()
})
.unwrap_or_default();
// Everything after the entry word is the `INTO …` tail; prefix
// the real `insert` keyword for the engine.
let tail = path
.items
.first()
.map_or(source, |entry| &source[entry.span.1..]);
let sql = format!("insert {}", tail.trim());
// The entry word is the real `insert` keyword (sub-phase 3j),
// so the validated line runs verbatim (grammar-as-text,
// ADR-0030 §4) — no keyword reconstruction.
let sql = source.trim().to_string();
Ok(Command::SqlInsert {
sql,
target_table,
@@ -966,13 +971,10 @@ fn path_has_returning(path: &MatchedPath) -> bool {
}
/// 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.
///
/// Dev-scaffold detail: the entry word is `sql_update` (not valid
/// SQL), so the statement is reconstructed as `update` + the
/// matched tail. Sub-phase 3j wires the real `update` entry word,
/// at which point this collapses to `source.trim()`.
/// (ADR-0033 §2). Extracts the target table from the matched path
/// so the worker re-persists the right CSV. `update` is now the
/// real (shared) entry word, so the validated `source` runs
/// verbatim (sub-phase 3j).
fn build_sql_update(path: &MatchedPath, source: &str) -> Result<Command, ValidationError> {
// The UPDATE target is the first `table_name` ident (it
// precedes any table referenced inside a SET / WHERE subquery).
@@ -986,11 +988,7 @@ fn build_sql_update(path: &MatchedPath, source: &str) -> Result<Command, Validat
_ => None,
})
.unwrap_or_default();
let tail = path
.items
.first()
.map_or(source, |entry| &source[entry.span.1..]);
let sql = format!("update {}", tail.trim());
let sql = source.trim().to_string();
Ok(Command::SqlUpdate {
sql,
target_table,
@@ -999,17 +997,13 @@ fn build_sql_update(path: &MatchedPath, source: &str) -> Result<Command, Validat
}
/// Build `Command::SqlDelete` from a validated SQL `DELETE`
/// (ADR-0033 §1/§7, sub-phase 3f). Extracts the target table from
/// the matched path so the worker re-persists the right CSV and
/// snapshots the right inbound children for cascade diffing. No
/// WHERE clause is captured — the worker executes the verbatim SQL
/// and never inspects the predicate (Amendment 2).
///
/// Dev-scaffold detail: the entry word is `sql_delete` (not valid
/// SQL), so the statement is reconstructed as `delete` + the matched
/// tail (which opens at `from`). Sub-phase 3j wires the real
/// `delete` entry word, at which point this collapses to
/// `source.trim()`.
/// (ADR-0033 §1/§7). Extracts the target table from the matched
/// path so the worker re-persists the right CSV and snapshots the
/// right inbound children for cascade diffing. No WHERE clause is
/// captured — the worker executes the verbatim SQL and never
/// inspects the predicate (Amendment 2). `delete` is now the real
/// (shared) entry word, so the validated `source` runs verbatim
/// (sub-phase 3j).
fn build_sql_delete(path: &MatchedPath, source: &str) -> Result<Command, ValidationError> {
// The DELETE target is the first `table_name` ident (it precedes
// any table referenced inside a WHERE subquery).
@@ -1023,11 +1017,7 @@ fn build_sql_delete(path: &MatchedPath, source: &str) -> Result<Command, Validat
_ => None,
})
.unwrap_or_default();
let tail = path
.items
.first()
.map_or(source, |entry| &source[entry.span.1..]);
let sql = format!("delete {}", tail.trim());
let sql = source.trim().to_string();
Ok(Command::SqlDelete {
sql,
target_table,
@@ -1110,51 +1100,46 @@ pub static WITH: CommandNode = CommandNode {
help_id: None,
usage_ids: &["parse.usage.select"],};
/// SQL `INSERT` development scaffold (ADR-0033 sub-phase 3b3i).
/// SQL `INSERT` — the `Advanced`-category node of the shared
/// `insert` entry word (ADR-0033 §2, Amendment 1, sub-phase 3j).
///
/// Registered under the temporary entry word `sqlinsert` so the
/// SQL INSERT grammar and execution path can be exercised in
/// isolation, WITHOUT yet making `insert` a shared DSL/SQL entry
/// word. Sharing `insert` is sub-phase 3j, which depends on
/// `shortid` auto-fill (3d) so advanced-mode DSL inserts keep
/// parity rather than regressing through an incomplete SQL path.
/// This scaffold (entry word + reconstruction in `build_sql_insert`)
/// is removed when 3j wires the real `insert` entry word.
/// `insert` is a shared entry word: this `Advanced` SQL node and
/// the `Simple` DSL [`INSERT`] node both register under `insert`.
/// In Advanced mode the dispatcher (`walker::walk` / `decide`)
/// tries this SQL node first and falls back to the DSL node when
/// the SQL shape does not match; in Simple mode only the DSL node
/// is reachable (Amendment 3 — command identity is the mode-rooted
/// grammar-path outcome).
pub static SQL_INSERT: CommandNode = CommandNode {
entry: Word::keyword("sqlinsert"),
entry: Word::keyword("insert"),
shape: Node::Subgrammar(&sql_insert::SQL_INSERT_SHAPE),
ast_builder: build_sql_insert,
help_id: None,
usage_ids: &[],
};
/// SQL `UPDATE` development scaffold (ADR-0033 sub-phase 3e).
/// SQL `UPDATE` — the `Advanced` node of the shared `update` word.
///
/// Registered under the temporary entry word `sql_update` so the
/// SQL UPDATE grammar and execution path can be exercised in
/// isolation, WITHOUT yet making `update` a shared DSL/SQL entry
/// word. Sharing `update` is sub-phase 3j. This scaffold (entry
/// word + reconstruction in `build_sql_update`) is removed when 3j
/// wires the real `update` entry word.
/// ADR-0033 §2 / Amendment 1, sub-phase 3j. Pairs with the `Simple`
/// DSL [`UPDATE`] node; dispatch is SQL-first / DSL-fallback in
/// Advanced mode, DSL-only in Simple.
pub static SQL_UPDATE: CommandNode = CommandNode {
entry: Word::keyword("sql_update"),
entry: Word::keyword("update"),
shape: Node::Subgrammar(&sql_update::SQL_UPDATE_SHAPE),
ast_builder: build_sql_update,
help_id: None,
usage_ids: &[],
};
/// SQL `DELETE` development scaffold (ADR-0033 sub-phase 3f).
/// SQL `DELETE` — the `Advanced` node of the shared `delete` word.
///
/// Registered under the temporary entry word `sql_delete` so the
/// SQL DELETE grammar and execution path (including cascade-summary
/// parity) can be exercised in isolation, WITHOUT yet making
/// `delete` a shared DSL/SQL entry word. Sharing `delete` is
/// sub-phase 3j. This scaffold (entry word + reconstruction in
/// `build_sql_delete`) is removed when 3j wires the real `delete`
/// entry word.
/// ADR-0033 §2 / Amendment 1, sub-phase 3j. Pairs with the `Simple`
/// DSL [`DELETE`] node; dispatch is SQL-first / DSL-fallback in
/// Advanced mode, DSL-only in Simple. In Advanced mode `delete from t
/// --all-rows` falls back to the DSL node (the SQL shape has no
/// `--all-rows`).
pub static SQL_DELETE: CommandNode = CommandNode {
entry: Word::keyword("sql_delete"),
entry: Word::keyword("delete"),
shape: Node::Subgrammar(&sql_delete::SQL_DELETE_SHAPE),
ast_builder: build_sql_delete,
help_id: None,
+5 -9
View File
@@ -573,17 +573,13 @@ pub static REGISTRY: &[(&CommandNode, CommandCategory)] = &[
(&data::EXPLAIN, CommandCategory::Simple),
(&data::SELECT, CommandCategory::Advanced),
(&data::WITH, CommandCategory::Advanced),
// SQL INSERT development scaffold (sub-phase 3b3i); the
// temporary `sqlinsert` entry word keeps it isolated from the
// DSL `insert` word until 3j wires the shared entry.
// Shared entry words (sub-phase 3j, ADR-0033 §2 / Amendment 1):
// `insert` / `update` / `delete` each appear twice — the
// `Simple` DSL node above and this `Advanced` SQL node. The
// dispatcher tries the SQL node first in Advanced mode and falls
// back to the DSL node when the SQL shape does not match.
(&data::SQL_INSERT, CommandCategory::Advanced),
// SQL UPDATE development scaffold (sub-phase 3e); the temporary
// `sql_update` entry word keeps it isolated from the DSL
// `update` word until 3j wires the shared entry.
(&data::SQL_UPDATE, CommandCategory::Advanced),
// SQL DELETE development scaffold (sub-phase 3f); the temporary
// `sql_delete` entry word keeps it isolated from the DSL
// `delete` word until 3j wires the shared entry.
(&data::SQL_DELETE, CommandCategory::Advanced),
];
+3 -2
View File
@@ -7,8 +7,9 @@
//! plus every cascade-affected child (ADR-0030 §11). The shape here
//! is the post-`DELETE` portion — the entry-word dispatch consumes
//! the leading `DELETE` keyword before this shape walks, so the
//! shape opens at `FROM` (mirroring `sql_update::SQL_UPDATE_SHAPE`,
//! where the dev `sql_update` word stands in for `UPDATE`).
//! shape opens at `FROM`. `delete` is a shared entry word
//! (sub-phase 3j): this `Advanced` SQL shape and the `Simple` DSL
//! delete node both register under `delete`.
//!
//! Scope (3f): `FROM <table> [ WHERE … ] [ ';' ]`, the `__rdbms_*`
//! target rejection, and the shared `sql_expr` on the WHERE
+3 -2
View File
@@ -5,8 +5,9 @@
//! validated SQL text and re-persists the target table's CSV
//! (ADR-0030 §11). The shape here is the post-`UPDATE` portion —
//! the entry-word dispatch consumes the leading `UPDATE` keyword
//! before this shape walks (mirroring `sql_insert::SQL_INSERT_SHAPE`,
//! where the dev `sqlinsert` word stands in for `INSERT`).
//! before this shape walks. `update` is a shared entry word
//! (sub-phase 3j): this `Advanced` SQL shape and the `Simple` DSL
//! update node both register under `update`.
//!
//! Scope (3e): `<table> SET assignment_list [ WHERE … ]`, the
//! `__rdbms_*` target rejection, and the shared `sql_expr` on both
+154 -2
View File
@@ -360,12 +360,23 @@ mod tests {
use crate::dsl::value::Value;
use pretty_assertions::assert_eq;
// These helpers parse in **Simple mode** — the DSL surface
// (ADR-0003). The tests in this module exercise the DSL
// grammar (`insert`/`update`/`delete` Forms A/B/C, the
// `--all-rows` rail, DDL, app commands), all of which are
// canonical in Simple mode. Since sub-phase 3j made
// `insert`/`update`/`delete` shared entry words (ADR-0033 §2,
// Amendment 3), parsing these in Advanced mode would route the
// overlap to the SQL command variants; the SQL surface is
// covered by `tests/sql_*.rs` instead. No SQL-only command
// (`select`/`with`) is tested through these helpers.
fn ok(input: &str) -> Command {
parse_command(input).unwrap_or_else(|e| panic!("expected ok for {input:?}, got {e:?}"))
parse_command_in_mode(input, Mode::Simple)
.unwrap_or_else(|e| panic!("expected ok for {input:?}, got {e:?}"))
}
fn err(input: &str) -> ParseError {
parse_command(input).expect_err("expected parse error")
parse_command_in_mode(input, Mode::Simple).expect_err("expected parse error")
}
fn err_message(input: &str) -> String {
@@ -1163,6 +1174,147 @@ mod tests {
assert!(matches!(e, ParseError::Invalid { .. }), "got {e:?}");
}
// =====================================================
// Sub-phase 3j — shared-entry-word dispatch (ADR-0033 §2,
// Amendment 1 / Amendment 3).
//
// `insert` / `update` / `delete` are *shared* entry words: a
// `Simple` DSL node and an `Advanced` SQL node both register
// under each. A command's identity is the outcome of the
// mode-rooted grammar path:
// - Advanced mode tries the SQL shape first and falls back to
// the DSL shape only when the SQL shape *structurally* can't
// match (e.g. the DSL-only `--all-rows` flag). A content
// rejection (a `__rdbms_*` target) on the SQL shape is
// surfaced, never masked by the DSL fallback.
// - Simple mode commits the DSL shape; it points the user at
// advanced mode ("this is SQL") only when the input is
// SQL-only (the DSL shape structurally mismatches and the SQL
// shape matches — e.g. a `returning` tail). A DSL command
// that is merely incomplete or has a bad value still commits
// the DSL node so the user sees DSL completion / DSL errors.
// The §6/§7 parity guarantees mean the two variants execute to
// identical effects for an overlapping input.
// =====================================================
#[test]
fn advanced_ambiguous_insert_routes_to_sql() {
assert!(matches!(
parse_command_in_mode("insert into Orders values (1, 2)", Mode::Advanced),
Ok(Command::SqlInsert { .. })
));
}
#[test]
fn advanced_ambiguous_update_routes_to_sql() {
assert!(matches!(
parse_command_in_mode(
"update Orders set total = 0 where id = 1",
Mode::Advanced,
),
Ok(Command::SqlUpdate { .. })
));
}
#[test]
fn advanced_ambiguous_delete_routes_to_sql() {
assert!(matches!(
parse_command_in_mode("delete from Orders where id = 1", Mode::Advanced),
Ok(Command::SqlDelete { .. })
));
}
#[test]
fn advanced_dsl_only_delete_falls_back_to_dsl() {
// `--all-rows` is DSL-only; the SQL DELETE shape can't consume
// the trailing flag, so dispatch falls back to the DSL node.
assert_eq!(
parse_command_in_mode("delete from Orders --all-rows", Mode::Advanced).unwrap(),
Command::Delete {
table: "Orders".to_string(),
filter: RowFilter::AllRows,
},
);
}
#[test]
fn simple_mode_data_commands_reject_internal_tables() {
// ADR-0030 §6 ("every table-source slot") / `/runda` finding
// B: the DSL data-command target slots reject `__rdbms_*`
// internal tables in simple mode too — matching the SQL
// grammar. Without this, simple-mode DML could read/write the
// internal metadata tables while advanced-mode SQL rejected
// them.
for input in [
"insert into __rdbms_playground_columns values (1)",
"update __rdbms_playground_columns set x = 1 where id = 1",
"delete from __rdbms_playground_columns where id = 1",
"show data __rdbms_playground_columns",
"show table __rdbms_playground_relationships",
] {
assert!(
parse_command_in_mode(input, Mode::Simple).is_err(),
"internal table must be rejected in simple mode: {input:?}",
);
}
}
#[test]
fn advanced_internal_table_insert_is_rejected_not_fallen_back() {
// The SQL insert's `reject_internal_table` rail must surface
// even though the DSL insert node lacks it: a content
// rejection commits the SQL candidate rather than falling
// through to the DSL node that would accept it.
assert!(
parse_command_in_mode(
"insert into __rdbms_playground_columns values (1)",
Mode::Advanced,
)
.is_err(),
);
}
#[test]
fn simple_dsl_delete_stays_dsl() {
assert_eq!(
parse_command_in_mode("delete from Orders where id = 1", Mode::Simple).unwrap(),
Command::Delete {
table: "Orders".to_string(),
filter: RowFilter::eq("id", Value::Number("1".to_string())),
},
);
}
#[test]
fn simple_sql_only_entry_word_points_at_advanced_mode() {
// A SQL-only *entry word* (`select`) has no DSL form, so
// simple mode emits the "this is SQL" hint at the parse level
// (ADR-0030 §2).
match parse_command_in_mode("select Name from Orders", Mode::Simple) {
Err(ParseError::Invalid { message, .. }) => assert!(
message.contains("advanced"),
"expected the this-is-SQL hint, got: {message}",
),
other => panic!("expected the this-is-SQL hint, got {other:?}"),
}
}
#[test]
fn simple_shared_word_with_sql_construct_is_a_dsl_parse_error() {
// `returning` is SQL-only, but `delete` is a *shared* entry
// word, so simple mode commits the DSL shape and surfaces a
// DSL parse error (ADR-0033 Amendment 3). The "(valid as SQL
// in advanced mode)" pointer is added at the hint layer
// (input_render), not in the parsed command/error here.
assert!(matches!(
parse_command_in_mode(
"delete from Orders where id = 1 returning *",
Mode::Simple,
),
Err(ParseError::Invalid { .. })
));
}
#[test]
fn show_data_command() {
assert_eq!(
+146 -106
View File
@@ -293,12 +293,18 @@ pub fn completion_probe_in_mode(
use crate::dsl::grammar::{REGISTRY, is_advanced_only};
let mode_filtered_entries = || -> Vec<outcome::Expectation> {
// De-duplicate shared entry words (sub-phase 3j): `insert` /
// `update` / `delete` each appear twice in `REGISTRY` (a
// `Simple` DSL node and an `Advanced` SQL node), but the
// entry word should be offered to the user only once.
let mut seen = std::collections::HashSet::new();
REGISTRY
.iter()
.filter(|(c, _)| {
mode == crate::mode::Mode::Advanced
|| !is_advanced_only(c.entry.primary)
})
.filter(|(c, _)| seen.insert(c.entry.primary))
.map(|(c, _)| outcome::Expectation::Word(c.entry.primary))
.collect()
};
@@ -2096,12 +2102,17 @@ pub fn expected_at_input_in_mode(
use crate::dsl::grammar::{REGISTRY, is_advanced_only};
let mode_filtered = || -> Vec<outcome::Expectation> {
// De-duplicate shared entry words (sub-phase 3j): `insert` /
// `update` / `delete` each appear twice in `REGISTRY` (Simple
// DSL + Advanced SQL nodes); offer the word once.
let mut seen = std::collections::HashSet::new();
REGISTRY
.iter()
.filter(|(c, _)| {
mode == crate::mode::Mode::Advanced
|| !is_advanced_only(c.entry.primary)
})
.filter(|(c, _)| seen.insert(c.entry.primary))
.map(|(c, _)| outcome::Expectation::Word(c.entry.primary))
.collect()
};
@@ -2353,27 +2364,25 @@ fn decide(
match mode {
crate::mode::Mode::Simple => {
let Some(&(sidx, snode)) = simple.first() else {
// No DSL candidate — the entry word is SQL-only.
let primary = candidates.first().map_or("", |(_, n, _)| n.entry.primary);
return Decision::ThisIsSql { primary };
};
if advanced.is_empty() {
return Decision::Commit { idx: sidx, node: snode };
// Simple mode is the DSL surface (ADR-0003). A shared
// entry word (`insert`/`update`/`delete`) commits its DSL
// candidate so the user sees DSL completion and the *real*
// DSL error — with its position — rather than a bare
// "this is SQL" that discards the actionable detail
// (ADR-0033 Amendment 3). "This is SQL" is reserved for
// entry words with no DSL form (`select` / `with`): there
// the DSL surface has nothing to offer. For a DSL line
// that fails here but *would* run in advanced mode, a
// "(valid as SQL in advanced mode)" pointer is appended at
// the rendering layer (see `advanced_alternative_note`),
// combining the DSL fix with the mode hint.
match simple.first() {
Some(&(sidx, snode)) => Decision::Commit { idx: sidx, node: snode },
None => {
let primary = candidates.first().map_or("", |(_, n, _)| n.entry.primary);
Decision::ThisIsSql { primary }
}
}
// Shared entry word: prefer the DSL node; only point
// at advanced mode when the DSL shape does not match
// but the SQL shape does.
if scratch_full_match(effective_source, kw_start, kw_end, snode, mode, schema) {
return Decision::Commit { idx: sidx, node: snode };
}
let (_, anode) = advanced[0];
if scratch_full_match(effective_source, kw_start, kw_end, anode, mode, schema) {
return Decision::ThisIsSql {
primary: anode.entry.primary,
};
}
Decision::Commit { idx: sidx, node: snode }
}
crate::mode::Mode::Advanced => {
// Advanced candidates first, DSL as the fallback.
@@ -2385,13 +2394,28 @@ fn decide(
let (idx, node) = ordered[0];
return Decision::Commit { idx, node };
}
// Commit the first candidate that fully matches OR is
// rejected by a content validator. A `ValidationFailed`
// means the shape *fits* but the content is invalid (e.g.
// an internal `__rdbms_*` target rejected by
// `reject_internal_table`); that error must surface rather
// than being masked by falling back to a candidate that
// lacks the validator (sub-phase 3j — the DSL `insert`
// node has no internal-table rail). A structural
// `Mismatch` (e.g. the DSL-only `--all-rows` the SQL shape
// can't consume) is *not* committed here, so the fallback
// to the DSL node still works.
for &(idx, node) in &ordered {
if scratch_full_match(effective_source, kw_start, kw_end, node, mode, schema) {
if matches!(
scratch_outcome(effective_source, kw_start, kw_end, node, mode, schema),
WalkOutcome::Match { .. } | WalkOutcome::ValidationFailed { .. }
) {
return Decision::Commit { idx, node };
}
}
// None fully matched — commit the furthest-progress
// candidate, keeping the first (advanced) on ties.
// None matched or content-rejected — commit the
// furthest-progress candidate, keeping the first
// (advanced) on ties.
let mut best = ordered[0];
let mut best_progress =
scratch_progress(effective_source, kw_start, kw_end, best.1, mode, schema);
@@ -2466,22 +2490,6 @@ fn scratch_outcome(
result.outcome
}
/// Whether a candidate fully matches the input (a clean
/// `WalkOutcome::Match`), tested on a scratch context.
fn scratch_full_match(
effective_source: &str,
kw_start: usize,
kw_end: usize,
node: &'static crate::dsl::grammar::CommandNode,
mode: crate::mode::Mode,
schema: Option<&crate::completion::SchemaCache>,
) -> bool {
matches!(
scratch_outcome(effective_source, kw_start, kw_end, node, mode, schema),
WalkOutcome::Match { .. }
)
}
/// How far (byte position) a candidate's walk progressed. A full
/// match scores the whole input; a failure scores its failure
/// position. Used only to tie-break when no candidate fully
@@ -2709,10 +2717,20 @@ mod tests {
//! the walker's contract for the migrated commands so
//! Phase B-F migrations can refactor without regression.
use crate::dsl::command::{AppCommand, Command, MessagesValue, ModeValue};
use crate::dsl::parser::parse_command;
use crate::dsl::parser::parse_command_in_mode;
use crate::mode::Mode;
/// Parse in **Simple mode** — the DSL surface (ADR-0003). The
/// tests in this module exercise the DSL grammar (app commands,
/// DDL, and the `insert`/`update`/`delete` DSL forms), all
/// canonical in Simple mode. Sub-phase 3j made
/// `insert`/`update`/`delete` shared entry words (ADR-0033 §2,
/// Amendment 3), so parsing them in Advanced mode would route the
/// overlap to the SQL command variants; the SQL surface is
/// validated through the advanced-mode diagnostic helpers
/// (`diag_keys` / `diagnostics_advanced`) and `tests/sql_*.rs`.
fn parse(input: &str) -> Result<Command, crate::dsl::ParseError> {
parse_command(input)
parse_command_in_mode(input, Mode::Simple)
}
// ---- Bare no-arg commands ---------------------------------
@@ -3637,11 +3655,18 @@ mod tests {
#[test]
fn not_like_on_a_numeric_column_is_also_a_warning() {
// The `LIKE`-on-numeric predicate warning is a DSL diagnostic
// (its sibling `like_on_a_numeric_column_is_a_warning` uses
// the Simple-mode `diagnostics` helper). Since sub-phase 3j
// made `delete` a shared entry word (ADR-0033 Amendment 3),
// this verdict is taken in Simple mode so the input routes to
// the DSL `delete` and its predicate diagnostics.
let schema = schema_with("Orders", &[("Total", Type::Decimal)]);
assert_eq!(
super::input_verdict(
super::input_verdict_in_mode(
"delete from Orders where Total not like '9%'",
Some(&schema),
crate::mode::Mode::Simple,
),
Some(super::Severity::Warning),
);
@@ -3939,7 +3964,19 @@ mod tests {
// =========================================================
use crate::completion::{SchemaCache, TableColumn};
use crate::dsl::parser::parse_command_with_schema;
use crate::dsl::parser::parse_command_with_schema_in_mode;
/// Phase-D typed value slots are the **DSL** surface (Simple
/// mode). Sub-phase 3j made `insert`/`update`/`delete` shared
/// entry words (ADR-0033 Amendment 3), so these typed-slot tests
/// parse in Simple mode to reach the DSL grammar rather than the
/// SQL one. Thin wrapper keeps the existing call sites unchanged.
fn parse_command_with_schema(
input: &str,
schema: &SchemaCache,
) -> Result<crate::dsl::command::Command, crate::dsl::ParseError> {
parse_command_with_schema_in_mode(input, schema, crate::mode::Mode::Simple)
}
fn schema_with(table: &str, columns: &[(&str, Type)]) -> SchemaCache {
let cols: Vec<TableColumn> = columns
@@ -4522,7 +4559,7 @@ mod tests {
// list (the target isn't a binding, so a targeted pass covers
// it).
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]);
let diags = diag_keys("sqlinsert into t (nonexistent) values (1)", &schema);
let diags = diag_keys("insert into t (nonexistent) values (1)", &schema);
assert!(
diags.iter().any(|d| d.contains("no such column")),
"unknown INSERT column should be flagged; got {diags:?}",
@@ -4537,7 +4574,7 @@ mod tests {
// SELECT's source tables — otherwise the flat-scope
// bare-column branch falsely flags it against `b`.
let schema = two_table_schema(); // a(id,name), b(id,total)
let diags = diag_keys("sqlinsert into a (name) select total from b", &schema);
let diags = diag_keys("insert into a (name) select total from b", &schema);
assert!(
!diags.iter().any(|d| d.contains("no such column")),
"target column `name` must not be flagged against the SELECT source; got {diags:?}",
@@ -4552,7 +4589,7 @@ mod tests {
// against `b`.
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id) select id from b on conflict (name) do nothing",
"insert into a (id) select id from b on conflict (name) do nothing",
&schema,
);
assert!(
@@ -4568,7 +4605,7 @@ mod tests {
// resolves to the target `a`, not the SELECT source `b`.
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id) select id from b on conflict (id) do update set name = name",
"insert into a (id) select id from b on conflict (id) do update set name = name",
&schema,
);
assert!(
@@ -4584,7 +4621,7 @@ mod tests {
// against the target). Covers the SET RHS and the WHERE.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]);
let set_rhs = diag_keys(
"sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = nope",
"insert into t (a, b) values (1, 'x') on conflict (a) do update set b = nope",
&schema,
);
assert!(
@@ -4592,7 +4629,7 @@ mod tests {
"unknown DO UPDATE SET RHS ref should be flagged; got {set_rhs:?}",
);
let where_ref = diag_keys(
"sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where nope = 1",
"insert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where nope = 1",
&schema,
);
assert!(
@@ -4608,7 +4645,7 @@ mod tests {
// target-only column must resolve to the target `a`, not be
// flagged against the SELECT source `b`.
let schema = two_table_schema(); // a(id,name), b(id,total)
let diags = diag_keys("sqlinsert into a (id) select id from b returning name", &schema);
let diags = diag_keys("insert into a (id) select id from b returning name", &schema);
assert!(
!diags.iter().any(|d| d.contains("no such column")),
"RETURNING `name` (a's column) must not flag against `b`; got {diags:?}",
@@ -4621,7 +4658,7 @@ mod tests {
// qualified star) in an INSERT … SELECT resolves to the
// target `a`, not flagged against the SELECT source `b`.
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (id) select id from b returning a.*", &schema);
let diags = diag_keys("insert into a (id) select id from b returning a.*", &schema);
assert!(
diags.is_empty(),
"target-qualified `a.*` in RETURNING must resolve cleanly; got {diags:?}",
@@ -4631,7 +4668,7 @@ mod tests {
#[test]
fn unrelated_qualified_star_in_returning_still_flagged() {
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (id) select id from b returning zzz.*", &schema);
let diags = diag_keys("insert into a (id) select id from b returning zzz.*", &schema);
assert!(
diags.iter().any(|d| d.contains("no such table or alias")),
"unrelated `zzz.*` qualifier should still flag; got {diags:?}",
@@ -4645,7 +4682,7 @@ mod tests {
// target `a`, not flagged as an unknown qualifier (a is the
// target, b is the SELECT source).
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (id) select id from b returning a.name", &schema);
let diags = diag_keys("insert into a (id) select id from b returning a.name", &schema);
assert!(
diags.is_empty(),
"target-qualified RETURNING ref must resolve cleanly; got {diags:?}",
@@ -4656,7 +4693,7 @@ mod tests {
fn target_qualified_ref_unknown_column_still_flagged() {
// `a.nope` — a is the target but nope isn't its column.
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (id) select id from b returning a.nope", &schema);
let diags = diag_keys("insert into a (id) select id from b returning a.nope", &schema);
assert!(
diags.iter().any(|d| d.contains("no such column")),
"unknown column under the target qualifier should flag; got {diags:?}",
@@ -4668,7 +4705,7 @@ mod tests {
// A qualifier that's neither `excluded` nor the target is
// still an unknown qualifier (the leak guard holds).
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (id) select id from b returning zzz.name", &schema);
let diags = diag_keys("insert into a (id) select id from b returning zzz.name", &schema);
assert!(
diags.iter().any(|d| d.contains("no such table or alias")),
"unrelated qualifier should still flag; got {diags:?}",
@@ -4680,7 +4717,7 @@ mod tests {
// Flip side: a RETURNING ref to a column on neither table is
// flagged (against the INSERT target).
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (id) select id from b returning nope", &schema);
let diags = diag_keys("insert into a (id) select id from b returning nope", &schema);
assert!(
diags.iter().any(|d| d.contains("no such column")),
"unknown RETURNING column should be flagged; got {diags:?}",
@@ -4692,9 +4729,9 @@ mod tests {
// The VALUES INSERT … RETURNING case (no bindings at all):
// a valid target column is silent, an unknown one flags.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]);
let ok = diag_keys("sqlinsert into t (a) values (1) returning b", &schema);
let ok = diag_keys("insert into t (a) values (1) returning b", &schema);
assert!(!ok.iter().any(|d| d.contains("no such column")), "got {ok:?}");
let bad = diag_keys("sqlinsert into t (a) values (1) returning nope", &schema);
let bad = diag_keys("insert into t (a) values (1) returning nope", &schema);
assert!(bad.iter().any(|d| d.contains("no such column")), "got {bad:?}");
}
@@ -4703,7 +4740,7 @@ mod tests {
// And a valid bare ref to a target column is silent.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]);
let diags = diag_keys(
"sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where a > 0",
"insert into t (a, b) values (1, 'x') on conflict (a) do update set b = 'y' where a > 0",
&schema,
);
assert!(
@@ -4718,7 +4755,7 @@ mod tests {
// neither table is flagged (against the INSERT target).
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id) values (1) on conflict (nope) do nothing",
"insert into a (id) values (1) on conflict (nope) do nothing",
&schema,
);
assert!(
@@ -4732,7 +4769,7 @@ mod tests {
// The flip side: a column in neither the target nor the source
// is still flagged (against the target, by the dedicated pass).
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (nope) select total from b", &schema);
let diags = diag_keys("insert into a (nope) select total from b", &schema);
assert!(
diags.iter().any(|d| d.contains("no such column")),
"a genuinely unknown INSERT column should still be flagged; got {diags:?}",
@@ -4742,7 +4779,7 @@ mod tests {
#[test]
fn insert_column_list_known_columns_silent() {
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]);
let diags = diag_keys("sqlinsert into t (a, b) values (1, 'x')", &schema);
let diags = diag_keys("insert into t (a, b) values (1, 'x')", &schema);
assert!(
!diags.iter().any(|d| d.contains("no such column")),
"known columns must not flag; got {diags:?}",
@@ -4755,7 +4792,7 @@ mod tests {
// SET column — an unknown DO UPDATE SET column is flagged.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]);
let diags = diag_keys(
"sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set nosuch = 1",
"insert into t (a, b) values (1, 'x') on conflict (a) do update set nosuch = 1",
&schema,
);
assert!(
@@ -4768,7 +4805,7 @@ mod tests {
fn upsert_do_update_known_set_column_silent() {
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Text)]);
let diags = diag_keys(
"sqlinsert into t (a, b) values (1, 'x') on conflict (a) do update set b = excluded.b",
"insert into t (a, b) values (1, 'x') on conflict (a) do update set b = excluded.b",
&schema,
);
assert!(
@@ -4782,7 +4819,7 @@ mod tests {
// 3i cross-cut: the Phase-2 predicate-warning pass fires on a
// DML SET/WHERE sql_expr slot (here `= NULL` in an UPDATE).
let schema = schema_with("t", &[("id", Type::Int), ("v", Type::Int)]);
let diags = diag_keys("sql_update t set v = 1 where v = NULL", &schema);
let diags = diag_keys("update t set v = 1 where v = NULL", &schema);
assert!(
diags.iter().any(|d| d.contains("IS NULL")),
"eq_null should fire on the UPDATE WHERE; got {diags:?}",
@@ -4796,7 +4833,7 @@ mod tests {
"t",
&[("a", Type::Int, false, false), ("b", Type::Text, true, false)],
);
let diags = diag_keys("sqlinsert into t (a) values (1)", &schema);
let diags = diag_keys("insert into t (a) values (1)", &schema);
assert!(
diags.iter().any(|d| d.contains("is required")),
"omitting NOT NULL `b` should warn; got {diags:?}",
@@ -4809,7 +4846,7 @@ mod tests {
"t",
&[("a", Type::Int, false, false), ("b", Type::Text, true, false)],
);
let diags = diag_keys("sqlinsert into t (a, b) values (1, 'x')", &schema);
let diags = diag_keys("insert into t (a, b) values (1, 'x')", &schema);
assert!(
!diags.iter().any(|d| d.contains("is required")),
"including `b` must not warn; got {diags:?}",
@@ -4823,7 +4860,7 @@ mod tests {
"t",
&[("a", Type::Int, false, false), ("b", Type::Text, true, true)],
);
let diags = diag_keys("sqlinsert into t (a) values (1)", &schema);
let diags = diag_keys("insert into t (a) values (1)", &schema);
assert!(
!diags.iter().any(|d| d.contains("is required")),
"a defaulted column must not warn; got {diags:?}",
@@ -4838,7 +4875,7 @@ mod tests {
"t",
&[("id", Type::Serial, true, false), ("b", Type::Text, false, false)],
);
let diags = diag_keys("sqlinsert into t (b) values ('x')", &schema);
let diags = diag_keys("insert into t (b) values ('x')", &schema);
assert!(
!diags.iter().any(|d| d.contains("is required")),
"auto-gen serial must not warn; got {diags:?}",
@@ -4848,7 +4885,7 @@ mod tests {
#[test]
fn insert_arity_mismatch_single_row_fires() {
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys("sqlinsert into t (a, b) values (1, 2, 3)", &schema);
let diags = diag_keys("insert into t (a, b) values (1, 2, 3)", &schema);
assert!(
diags.iter().any(|d| d.contains("value(s) are given")),
"3 values for 2 columns should fire; got {diags:?}",
@@ -4858,7 +4895,7 @@ mod tests {
#[test]
fn insert_arity_match_is_silent() {
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys("sqlinsert into t (a, b) values (1, 2)", &schema);
let diags = diag_keys("insert into t (a, b) values (1, 2)", &schema);
assert!(
!diags.iter().any(|d| d.contains("value(s) are given")),
"matched arity must not fire; got {diags:?}",
@@ -4871,7 +4908,7 @@ mod tests {
// diagnostic; the matched rows stay silent.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys(
"sqlinsert into t (a, b) values (1, 2), (3, 4, 5), (6), (7, 8)",
"insert into t (a, b) values (1, 2), (3, 4, 5), (6), (7, 8)",
&schema,
);
let n = diags.iter().filter(|d| d.contains("value(s) are given")).count();
@@ -4885,7 +4922,7 @@ mod tests {
// tuple mismatch still fires; the conflict target never does.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys(
"sqlinsert into t (a, b) values (1, 2, 3) on conflict (a) do nothing",
"insert into t (a, b) values (1, 2, 3) on conflict (a) do nothing",
&schema,
);
let n = diags.iter().filter(|d| d.contains("value(s) are given")).count();
@@ -4898,7 +4935,7 @@ mod tests {
// silent (the conflict target isn't counted as a tuple).
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys(
"sqlinsert into t (a, b) values (1, 2) on conflict (a) do update set b = excluded.b",
"insert into t (a, b) values (1, 2) on conflict (a) do update set b = excluded.b",
&schema,
);
assert!(
@@ -4913,7 +4950,7 @@ mod tests {
// the walk must stop there (same guard as ON CONFLICT) so the
// RETURNING projection isn't mis-counted as a value tuple.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys("sqlinsert into t (a, b) values (1, 2, 3) returning *", &schema);
let diags = diag_keys("insert into t (a, b) values (1, 2, 3) returning *", &schema);
let n = diags.iter().filter(|d| d.contains("value(s) are given")).count();
assert_eq!(n, 1, "only the 3-value tuple is flagged; got {diags:?}");
}
@@ -4923,7 +4960,7 @@ mod tests {
// A comma inside a function call (depth ≥ 2) is not a tuple
// separator and must not inflate the value count.
let schema = schema_with("t", &[("a", Type::Int), ("b", Type::Int)]);
let diags = diag_keys("sqlinsert into t (a, b) values (1, coalesce(1, 2))", &schema);
let diags = diag_keys("insert into t (a, b) values (1, coalesce(1, 2))", &schema);
assert!(
!diags.iter().any(|d| d.contains("value(s) are given")),
"two values (one a 2-arg call) match the 2-col list; got {diags:?}",
@@ -4934,7 +4971,7 @@ mod tests {
fn insert_select_arity_mismatch_fires() {
// INSERT … SELECT: 1 listed column vs a 2-item projection.
let schema = two_table_schema(); // a(id,name), b(id,total)
let diags = diag_keys("sqlinsert into a (id) select id, total from b", &schema);
let diags = diag_keys("insert into a (id) select id, total from b", &schema);
assert!(
diags.iter().any(|d| d.contains("value(s) are given")),
"2-col projection into 1-col list should fire; got {diags:?}",
@@ -4944,7 +4981,7 @@ mod tests {
#[test]
fn insert_select_arity_match_is_silent() {
let schema = two_table_schema();
let diags = diag_keys("sqlinsert into a (id, name) select id, total from b", &schema);
let diags = diag_keys("insert into a (id, name) select id, total from b", &schema);
assert!(
!diags.iter().any(|d| d.contains("value(s) are given")),
"matched projection arity must not fire; got {diags:?}",
@@ -4955,7 +4992,7 @@ mod tests {
fn auto_column_overridden_fires_on_explicit_serial() {
// ADR-0033 §8.2: listing a serial column explicitly warns.
let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]);
let diags = diag_keys("sqlinsert into t (id, name) values (5, 'x')", &schema);
let diags = diag_keys("insert into t (id, name) values (5, 'x')", &schema);
assert!(
diags.iter().any(|d| d.contains("auto-generated")),
"explicit serial value should warn; got {diags:?}",
@@ -4965,7 +5002,7 @@ mod tests {
#[test]
fn auto_column_overridden_fires_on_explicit_shortid() {
let schema = schema_with("t", &[("id", Type::ShortId), ("name", Type::Text)]);
let diags = diag_keys("sqlinsert into t (id, name) values ('abc', 'x')", &schema);
let diags = diag_keys("insert into t (id, name) values ('abc', 'x')", &schema);
assert!(
diags.iter().any(|d| d.contains("auto-generated")),
"explicit shortid value should warn; got {diags:?}",
@@ -4976,7 +5013,7 @@ mod tests {
fn auto_column_overridden_silent_when_omitted() {
// Negative: omitting the auto-gen column does not warn.
let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]);
let diags = diag_keys("sqlinsert into t (name) values ('x')", &schema);
let diags = diag_keys("insert into t (name) values ('x')", &schema);
assert!(
!diags.iter().any(|d| d.contains("auto-generated")),
"omitting the serial column must not warn; got {diags:?}",
@@ -4990,7 +5027,7 @@ mod tests {
// only the inserted `id` in the column list would.
let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]);
let diags = diag_keys(
"sqlinsert into t (name) values ('x') on conflict (id) do nothing",
"insert into t (name) values ('x') on conflict (id) do nothing",
&schema,
);
assert!(
@@ -5005,7 +5042,7 @@ mod tests {
// resolves to the target table's columns — no diagnostic.
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.name",
"insert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.name",
&schema,
);
assert!(diags.is_empty(), "excluded.name should resolve in DO UPDATE; got {diags:?}");
@@ -5017,7 +5054,7 @@ mod tests {
// `excluded` resolves there too (not just in the SET RHS).
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id, name) values (1, 'x') on conflict (id) do update set name = 'y' where id = excluded.id",
"insert into a (id, name) values (1, 'x') on conflict (id) do update set name = 'y' where id = excluded.id",
&schema,
);
assert!(diags.is_empty(), "excluded in DO UPDATE WHERE should resolve; got {diags:?}");
@@ -5029,7 +5066,7 @@ mod tests {
// under it is still an unknown_column error.
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.nosuch",
"insert into a (id, name) values (1, 'x') on conflict (id) do update set name = excluded.nosuch",
&schema,
);
assert!(
@@ -5044,7 +5081,7 @@ mod tests {
// in scope) has no meaning and must be flagged.
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id, name) values (excluded.name, 'x')",
"insert into a (id, name) values (excluded.name, 'x')",
&schema,
);
assert!(
@@ -5060,7 +5097,7 @@ mod tests {
// resolves). The byte-range scoping must distinguish them.
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into a (id, name) values (excluded.id, 'x') on conflict (id) do update set name = excluded.name",
"insert into a (id, name) values (excluded.id, 'x') on conflict (id) do update set name = excluded.name",
&schema,
);
// Exactly the VALUES-side `excluded.id` is flagged; the
@@ -5083,7 +5120,7 @@ mod tests {
// `nonexistent_col` is not a column of `a`.
let schema = two_table_schema();
let diags = diag_keys(
"sqlinsert into b select nonexistent_col from a",
"insert into b select nonexistent_col from a",
&schema,
);
assert!(
@@ -5098,7 +5135,7 @@ mod tests {
// ADR-0033 sub-phase 3e cross-cut: the schema-existence
// pass fires on the SET assignment column.
let schema = schema_with("t", &[("id", Type::Int), ("v", Type::Text)]);
let diags = diag_keys("sql_update t set nonexistent = 1 where id = 1", &schema);
let diags = diag_keys("update t set nonexistent = 1 where id = 1", &schema);
assert!(
diags.iter().any(|d| d.contains("no such column")),
"expected unknown_column on the SET column; got {diags:?}",
@@ -5110,7 +5147,7 @@ mod tests {
// ADR-0033 sub-phase 3e cross-cut: the predicate-warning
// pass fires on `= NULL` in an UPDATE's WHERE.
let schema = schema_with("t", &[("id", Type::Int), ("v", Type::Int)]);
let diags = diag_keys("sql_update t set v = 1 where v = NULL", &schema);
let diags = diag_keys("update t set v = 1 where v = NULL", &schema);
assert!(
diags.iter().any(|d| d.contains("IS NULL")),
"expected eq_null warning on the WHERE; got {diags:?}",
@@ -5937,26 +5974,29 @@ mod dispatch_3a_tests {
assert_eq!(cmd, Some(Command::App(AppCommand::Quit)));
}
// ---- Exit-gate case 3: Simple + SQL-only input
// ValidationFailed advanced_mode.sql_in_simple ----------
// ---- Exit-gate case 3: Simple + SQL-shaped input on a SHARED
// word commits the DSL node (ADR-0033 Amendment 3) --------
#[test]
fn simple_mode_sql_only_input_is_this_is_sql() {
// Shared word, but the input matches only the SQL tail.
fn simple_mode_shared_word_commits_dsl_even_for_sql_input() {
// A shared word (DSL + SQL) in simple mode always commits its
// DSL candidate, even when the input matches only the SQL
// tail. The DSL grammar then rejects the SQL-only tail with a
// normal parse error (a `Mismatch`), surfacing the actionable
// DSL detail; the "(valid as SQL in advanced mode)" pointer is
// added at the rendering layer, not here. "This is SQL" as a
// dispatch outcome is reserved for entry words with no DSL
// form (see `simple_mode_sql_only_entry_word_is_this_is_sql`).
let cands = shared();
match run_decide("smk sqltail", Mode::Simple, &cands) {
Decision::ThisIsSql { primary } => assert_eq!(primary, "smk"),
Decision::Commit { idx, .. } => {
panic!("expected ThisIsSql, got Commit {{ idx: {idx} }}")
}
}
assert!(
std::ptr::eq(committed_node("smk sqltail", Mode::Simple, &cands), &SMOKE_DSL),
"simple mode must commit the DSL node for a shared word",
);
let (outcome, cmd) = dispatch("smk sqltail", Mode::Simple, &cands);
match outcome {
WalkOutcome::ValidationFailed { error, .. } => {
assert_eq!(error.message_key, "advanced_mode.sql_in_simple");
}
other => panic!("expected ValidationFailed, got {other:?}"),
}
assert!(
matches!(outcome, WalkOutcome::Mismatch { .. }),
"the DSL grammar rejects the SQL-only tail; got {outcome:?}",
);
assert_eq!(cmd, None);
}
+1
View File
@@ -315,6 +315,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
("advanced_mode.not_implemented", &["input"]),
// ---- Advanced-mode SQL surface (ADR-0030) ----
("advanced_mode.sql_in_simple", &["command"]),
("advanced_mode.also_valid_sql", &[]),
("select.internal_table", &["table"]),
(
"cli.invalid_value",
+4
View File
@@ -634,6 +634,10 @@ persistence:
advanced_mode:
not_implemented: "advanced mode SQL not implemented yet — echo: {input}"
sql_in_simple: "`{command}` is SQL — available in advanced mode. Switch with `mode advanced`, or prefix the line with `:` to run it once."
# Appended to a simple-mode DSL error when the same line would run
# in advanced mode — keeps the actionable DSL fix and adds the mode
# pointer (ADR-0033 Amendment 3).
also_valid_sql: "(valid as SQL in advanced mode — `mode advanced` or prefix `:`)"
# ---- SQL SELECT (advanced mode; ADR-0030 / ADR-0031) ----------------
select:
+132 -2
View File
@@ -90,7 +90,9 @@ pub fn render_input_runs_in_mode(
{
overlay_error(&mut runs, pos, theme);
}
if let Some(inv) = crate::completion::invalid_ident_at_cursor(input, cursor_byte, cache) {
if let Some(inv) =
crate::completion::invalid_ident_at_cursor_in_mode(input, cursor_byte, cache, mode)
{
overlay_error(&mut runs, inv.range.0, theme);
}
// Schema-aware diagnostics (ADR-0027 §2): unknown table /
@@ -167,6 +169,27 @@ pub fn classify_input_with_schema(
classify_parse_result(parse_command_with_schema(input, cache))
}
/// Mode-aware [`classify_input_with_schema`].
///
/// Walks the input in `mode` so the simple-mode DSL surface is
/// classified against the DSL grammar rather than the advanced SQL
/// grammar — relevant for the shared `insert`/`update`/`delete` entry
/// words (ADR-0033 Amendment 3). The mode-less entry point keeps its
/// advanced-mode behaviour.
#[must_use]
pub fn classify_input_with_schema_in_mode(
input: &str,
cache: &crate::completion::SchemaCache,
mode: Mode,
) -> InputState {
if input.trim().is_empty() {
return InputState::Empty;
}
classify_parse_result(crate::dsl::parser::parse_command_with_schema_in_mode(
input, cache, mode,
))
}
fn classify_parse_result(
result: Result<crate::dsl::Command, ParseError>,
) -> InputState {
@@ -239,6 +262,13 @@ pub fn ambient_hint(
/// "this is SQL" gate. The simple-mode entry point [`ambient_hint`]
/// forwards here with `Mode::Simple`.
///
/// In simple mode, when the line is a *definite* DSL error but the
/// same line would parse in advanced mode, the DSL error prose is
/// suffixed with the `advanced_mode.also_valid_sql` pointer — so the
/// user keeps the actionable DSL fix *and* learns it would run as SQL
/// in advanced mode (ADR-0033 Amendment 3). Mid-typing (incomplete)
/// input is not suffixed, to avoid noise during normal DSL entry.
///
/// Returns `None` for empty input — caller falls back to
/// `panel.hint_empty`.
#[must_use]
@@ -248,6 +278,58 @@ pub fn ambient_hint_in_mode(
memo: Option<&crate::completion::LastCompletion>,
cache: &crate::completion::SchemaCache,
mode: Mode,
) -> Option<AmbientHint> {
let core = ambient_hint_core_in_mode(input, cursor, memo, cache, mode);
// Combine: a simple-mode *definite* DSL error that would run in
// advanced mode keeps its DSL prose and gains the mode pointer.
// Skip the "command complete" prose — appending the pointer to a
// "submit this" hint would be contradictory (and that prose can
// come from the hint's schemaless fallback even when the
// schema-aware classify is a definite error — a pre-existing
// quirk this combine step deliberately does not amplify).
if mode == Mode::Simple
&& let Some(AmbientHint::Prose(message)) = &core
&& *message != crate::t!("hint.ambient_complete")
&& let Some(suffix) = advanced_alternative_note(input, cache)
{
return Some(AmbientHint::Prose(format!("{message} {suffix}")));
}
core
}
/// The `advanced_mode.also_valid_sql` pointer string, or `None`.
///
/// Returns the pointer when a simple-mode line is a *definite* DSL
/// error (not merely incomplete) yet parses in advanced mode. Used to
/// combine the DSL fix with the mode hint — both while typing
/// (`ambient_hint_in_mode`) and on submit (`App::dispatch_dsl`) — so
/// the pointer reaches SQL constructs that surface only on submit,
/// e.g. `delete … returning` (ADR-0033 Amendment 3).
#[must_use]
pub fn advanced_alternative_note(
input: &str,
cache: &crate::completion::SchemaCache,
) -> Option<String> {
let definite_dsl_error = matches!(
classify_input_with_schema_in_mode(input, cache, Mode::Simple),
InputState::DefiniteErrorAt(_)
);
if !definite_dsl_error {
return None;
}
if parse_command_with_schema_in_mode(input, cache, Mode::Advanced).is_ok() {
Some(crate::t!("advanced_mode.also_valid_sql"))
} else {
None
}
}
fn ambient_hint_core_in_mode(
input: &str,
cursor: usize,
memo: Option<&crate::completion::LastCompletion>,
cache: &crate::completion::SchemaCache,
mode: Mode,
) -> Option<AmbientHint> {
if input.trim().is_empty() {
return None;
@@ -401,7 +483,8 @@ pub fn ambient_hint_in_mode(
// Invalid identifier: cursor sits in a known-set slot but
// the typed prefix matches nothing in the schema. (Stage
// 8e / the user's #5.)
if let Some(inv) = crate::completion::invalid_ident_at_cursor(input, cursor, cache) {
if let Some(inv) = crate::completion::invalid_ident_at_cursor_in_mode(input, cursor, cache, mode)
{
let kind = match inv.source {
crate::dsl::grammar::IdentSource::Tables => "table",
crate::dsl::grammar::IdentSource::Columns => "column",
@@ -989,6 +1072,53 @@ mod tests {
}
}
#[test]
fn ambient_hint_combines_dsl_error_with_advanced_sql_pointer() {
// ADR-0033 Amendment 3: in simple mode, a *definite* DSL
// error whose line would run as SQL in advanced mode keeps
// its actionable DSL prose AND gains the
// `advanced_mode.also_valid_sql` pointer. `Customers(id
// serial, Name, Email)`: DSL Form B auto-skips the serial
// `id`, so three values is a definite DSL error — but the same
// line is a valid SQL insert (all three columns).
use crate::dsl::types::Type;
let cache = schema_with_columns(
"Customers",
&[("id", Type::Serial), ("Name", Type::Text), ("Email", Type::Text)],
);
let input = "insert into Customers values (1, 'Alice', 'a@b.c')";
match ambient_hint(input, input.len(), None, &cache) {
Some(AmbientHint::Prose(p)) => {
// The DSL detail survives …
assert!(p.contains("Name"), "expected DSL slot detail, got: {p:?}");
// … and the advanced-mode pointer is appended.
assert!(
p.contains("advanced mode"),
"expected the advanced-mode pointer, got: {p:?}",
);
}
other => panic!("expected Prose, got {other:?}"),
}
}
#[test]
fn ambient_hint_does_not_add_pointer_for_a_valid_dsl_command() {
// A valid simple-mode DSL command gets no advanced pointer —
// it isn't an error, and there is nothing to switch modes for.
use crate::dsl::types::Type;
let cache = schema_with_columns(
"Customers",
&[("id", Type::Serial), ("Name", Type::Text)],
);
let input = "insert into Customers values ('Alice')";
if let Some(AmbientHint::Prose(p)) = ambient_hint(input, input.len(), None, &cache) {
assert!(
!p.contains("advanced mode"),
"a valid DSL command must not carry the advanced pointer, got: {p:?}",
);
}
}
#[test]
fn ambient_hint_at_insert_second_value_shows_text_prose() {
use crate::dsl::types::Type;
+7
View File
@@ -1644,6 +1644,13 @@ pub async fn run_replay(
// (bad syntax / typed-slot reject) — report and stop
// without dispatching.
let schema = build_schema_cache(database).await;
// Replay parses each line like an interactive submission and
// executes the resulting command — in advanced mode (the full
// surface). A bad value in a shared-entry-word DML line is
// caught either at parse time (a DSL form's typed slot) or at
// execute time (the engine's column-type enforcement); either
// way the offending line fails and replay stops without
// applying it.
let command = match crate::dsl::parser::parse_command_with_schema(
trimmed, &schema,
) {