diff --git a/docs/adr/0027-input-validity-indicator.md b/docs/adr/0027-input-validity-indicator.md index 9748335..15180ba 100644 --- a/docs/adr/0027-input-validity-indicator.md +++ b/docs/adr/0027-input-validity-indicator.md @@ -39,7 +39,8 @@ runs if submitted). - **WARNING** — the input is valid and *will* run, but is very likely not what a knowledgeable user wants: a type-mismatched comparison, or `= NULL` (both from - ADR-0026 §7). + ADR-0026 §7). Amendment 1 adds a third trigger — `LIKE` + against a numeric column. The split is *certainty of failure* versus *likely misleading*. The indicator shows the highest severity @@ -289,6 +290,44 @@ choices and deviations from the sketch above: `input_verdict` has a parse-outcome / schema-existence / expression-warning / existing-cases-sweep test set. +## Amendment 1 — `LIKE` on a numeric column (2026-05-19) + +§1 defined the WARNING set as exactly a type-mismatched +comparison and `= NULL`. A third trigger is added: + +- **`LIKE` against a numeric column.** `LIKE` is a + text-pattern match — `%` / `_` wildcards over characters. + Applied to a column of a numeric type (`int`, `real`, + `decimal`, `serial`) it still runs — the engine matches the + pattern against the value's text form — but is almost never + what the user wants; they most likely mean a comparison or + `BETWEEN`. It is a WARNING, consistent with the advisory + posture (§5): the command still submits. The negation is + irrelevant — `NOT LIKE` on a numeric column is just as + dubious. + +Scope is deliberately narrow — only *numeric* target columns +are flagged: + +- `bool` is integer-backed (0/1) but excluded; the handoff + item this implements named numeric columns specifically. +- The text-backed types (`text`, `shortid`, `date`, + `datetime`) are not flagged: `LIKE 'A%'` on text is its + intended use, and a prefix match on an ISO `date` / + `datetime` string is genuinely useful. +- `blob` is left unflagged. + +Widening to those types is a future model extension if a need +appears. + +This is exactly the kind of "we can tell, before it runs, +that this is dubious" case §6 anticipated: the trigger plugs +into the existing model rather than being a one-off check. It +lives alongside the others in `walker::predicate_warnings` +(the `Predicate::Like` arm, via `like_numeric_warning`); the +target column operand's span drives the highlight; the +message key is `diagnostic.like_numeric`. + ## See also - ADR-0003 — input modes; the input field and its mode diff --git a/docs/adr/README.md b/docs/adr/README.md index 0b592ed..72d33b8 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -32,5 +32,5 @@ This directory contains the project's ADRs, recorded per - [ADR-0024 — Unified grammar tree: execution plan](0024-unified-grammar-tree-execution-plan.md) — **Accepted**, the executable spec — implemented (Phases A–F; Phase F shipped "minimal", `parser.rs` retained as the router — see the ADR's Phase F implementation note) - [ADR-0025 — Indexes](0025-indexes.md) — **Accepted**, `add index` / `drop index`, persistence, rebuild-table preservation, and items-list display (`C3` index portion + `S2`) - [ADR-0026 — Complex WHERE expressions](0026-complex-where-expressions.md) — **Accepted**, stratified recursive expression grammar (`AND`/`OR`/`NOT`, comparisons, `LIKE`, `IS NULL`, `IN`, `BETWEEN`) for `update` / `delete` / `show data` filters; `show data` gains `where` + `limit`; adds the `Subgrammar` node and a recursive `Expr` AST (`C5a`) -- [ADR-0027 — Input-field validity indicator](0027-input-validity-indicator.md) — **Accepted**, a debounced `[ERR]` / `[WRN]` marker at the input row's right edge, backed by a walker diagnostics-severity model (parse-outcome + schema-existence); advisory, never blocks submission (`S6`) +- [ADR-0027 — Input-field validity indicator](0027-input-validity-indicator.md) — **Accepted**, a debounced `[ERR]` / `[WRN]` marker at the input row's right edge, backed by a walker diagnostics-severity model (parse-outcome + schema-existence); advisory, never blocks submission (`S6`); Amendment 1 adds a `LIKE`-on-numeric-column WARNING - [ADR-0028 — Query plans (`EXPLAIN QUERY PLAN`)](0028-query-plans.md) — **Accepted**, an `explain` prefix command over `show data` / `update` / `delete`; an annotated, span-styled plan tree; introduces the `OutputLine` styled-runs mechanism (ADR-0016's deferred per-span styling) (`QA1` / `QA2`) diff --git a/src/dsl/types.rs b/src/dsl/types.rs index 0967205..ebe3914 100644 --- a/src/dsl/types.rs +++ b/src/dsl/types.rs @@ -101,6 +101,18 @@ impl Type { ] } + /// True for the numeric types — `int`, `real`, `decimal`, + /// `serial`. `bool` (stored 0/1) and the text- / blob-backed + /// types are not numeric. Used to flag a `LIKE` text-pattern + /// match against a numeric column (ADR-0027, Amendment 1). + #[must_use] + pub const fn is_numeric(self) -> bool { + matches!( + self, + Self::Int | Self::Real | Self::Decimal | Self::Serial + ) + } + /// The user-facing type that an FK column should use to /// reference a primary key of *this* type. For most types /// the answer is the same type; for `serial` and `shortid` diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 240009e..db296a9 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -526,14 +526,54 @@ fn predicate_warnings( } } } - // `LIKE` is inherently a text-pattern test; flagging a - // non-text target is a future model extension. + // `LIKE` is a text-pattern test; against a numeric + // column it runs but is almost never intended + // (ADR-0027, Amendment 1). The negation is irrelevant — + // `NOT LIKE` on a numeric column is just as dubious. + Predicate::Like { target, .. } => { + if let Some((message, span)) = + like_numeric_warning(target, columns) + { + out.push(warn(message, span)); + } + } // `IS [NOT] NULL` is the *correct* null test — never // flagged. - Predicate::Like { .. } | Predicate::IsNull { .. } => {} + Predicate::IsNull { .. } => {} } } +/// A `LIKE` whose target is a numeric column: `LIKE` matches +/// text patterns, so a numeric target is almost certainly a +/// mistake (ADR-0027, Amendment 1). The message is paired with +/// the target column operand's span. `None` when the target is +/// a literal, an unknown column, or a non-numeric column. +fn like_numeric_warning( + target: &Operand, + columns: &[crate::completion::TableColumn], +) -> Option<(String, (usize, usize))> { + let Operand::Column { name, span } = target else { + return None; + }; + let ty = columns + .iter() + .find(|tc| tc.name.eq_ignore_ascii_case(name))? + .user_type; + if !ty.is_numeric() { + return None; + } + Some(( + crate::friendly::translate( + "diagnostic.like_numeric", + &[ + ("column", name as &dyn std::fmt::Display), + ("type", &ty.keyword() as &dyn std::fmt::Display), + ], + ), + *span, + )) +} + const fn is_null_literal(operand: &Operand) -> bool { matches!( operand, @@ -1816,6 +1856,46 @@ mod tests { assert_eq!(&input[s..e], "NoSuchCol"); } + // ---- LIKE on a numeric column (ADR-0027, Amendment 1) ----- + + #[test] + fn like_on_a_numeric_column_is_a_warning() { + // `LIKE` is a text-pattern match — against an int + // column it runs but is almost never intended. + let schema = schema_with("Customers", &[("Age", Type::Int)]); + let input = "delete from Customers where Age like '1%'"; + let diags = diagnostics(input, &schema); + assert_eq!(diags.len(), 1); + assert_eq!(diags[0].severity, super::Severity::Warning); + let (s, e) = diags[0].span; + assert_eq!(&input[s..e], "Age", "the span is the numeric column"); + } + + #[test] + fn not_like_on_a_numeric_column_is_also_a_warning() { + let schema = schema_with("Orders", &[("Total", Type::Decimal)]); + assert_eq!( + super::input_verdict( + "delete from Orders where Total not like '9%'", + Some(&schema), + ), + Some(super::Severity::Warning), + ); + } + + #[test] + fn like_on_a_text_column_is_clean() { + // `LIKE 'A%'` on a text column is its intended use. + let schema = schema_with("Customers", &[("Name", Type::Text)]); + assert_eq!( + super::input_verdict( + "delete from Customers where Name like 'A%'", + Some(&schema), + ), + None, + ); + } + #[test] fn walker_parses_insert_with_explicit_column_list() { assert_eq!( diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 61c7e6f..3d13921 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -39,6 +39,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ // ---- Pre-submit diagnostics (ADR-0027) ---- ("diagnostic.eq_null", &[]), + ("diagnostic.like_numeric", &["column", "type"]), ("diagnostic.type_mismatch", &["column", "type"]), ("diagnostic.unknown_column", &["name", "table"]), ("diagnostic.unknown_table", &["name"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index b57c3ce..386da3c 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -469,6 +469,8 @@ diagnostic: # runs, but the comparison is very likely not intended. type_mismatch: "`{column}` is {type} — this comparison uses a value of a different type" eq_null: "`= NULL` is never true — use `IS NULL` or `IS NOT NULL`" + # ADR-0027 Amendment 1: LIKE is a text-pattern match. + like_numeric: "`{column}` is {type} — `LIKE` is a text-pattern match, not a {type} comparison" # ---- Project lifecycle event notes ----------------------------------- project: