walker: flag LIKE on a numeric column (ADR-0027 Amendment 1)
LIKE is a text-pattern match; against a numeric column (int, real, decimal, serial) it runs but is almost never intended. predicate_warnings now emits a WARNING for it, spanned at the target column. New Type::is_numeric; catalog key diagnostic.like_numeric; ADR-0027 gains "Amendment 1" and the adr/README index line is updated per the index-upkeep rule. bool and the text-/blob-backed types are deliberately not flagged — see the amendment for the rationale. 3 walker tests (int, decimal NOT LIKE, text-column clean). 1108 passing, clippy clean.
This commit is contained in:
@@ -39,7 +39,8 @@ runs if submitted).
|
|||||||
- **WARNING** — the input is valid and *will* run, but is
|
- **WARNING** — the input is valid and *will* run, but is
|
||||||
very likely not what a knowledgeable user wants: a
|
very likely not what a knowledgeable user wants: a
|
||||||
type-mismatched comparison, or `= NULL` (both from
|
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
|
The split is *certainty of failure* versus *likely
|
||||||
misleading*. The indicator shows the highest severity
|
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 /
|
`input_verdict` has a parse-outcome / schema-existence /
|
||||||
expression-warning / existing-cases-sweep test set.
|
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
|
## See also
|
||||||
|
|
||||||
- ADR-0003 — input modes; the input field and its mode
|
- ADR-0003 — input modes; the input field and its mode
|
||||||
|
|||||||
+1
-1
@@ -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-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-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-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`)
|
- [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`)
|
||||||
|
|||||||
@@ -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
|
/// The user-facing type that an FK column should use to
|
||||||
/// reference a primary key of *this* type. For most types
|
/// reference a primary key of *this* type. For most types
|
||||||
/// the answer is the same type; for `serial` and `shortid`
|
/// the answer is the same type; for `serial` and `shortid`
|
||||||
|
|||||||
+83
-3
@@ -526,14 +526,54 @@ fn predicate_warnings(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// `LIKE` is inherently a text-pattern test; flagging a
|
// `LIKE` is a text-pattern test; against a numeric
|
||||||
// non-text target is a future model extension.
|
// 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
|
// `IS [NOT] NULL` is the *correct* null test — never
|
||||||
// flagged.
|
// 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 {
|
const fn is_null_literal(operand: &Operand) -> bool {
|
||||||
matches!(
|
matches!(
|
||||||
operand,
|
operand,
|
||||||
@@ -1816,6 +1856,46 @@ mod tests {
|
|||||||
assert_eq!(&input[s..e], "NoSuchCol");
|
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]
|
#[test]
|
||||||
fn walker_parses_insert_with_explicit_column_list() {
|
fn walker_parses_insert_with_explicit_column_list() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@@ -39,6 +39,7 @@
|
|||||||
pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
||||||
// ---- Pre-submit diagnostics (ADR-0027) ----
|
// ---- Pre-submit diagnostics (ADR-0027) ----
|
||||||
("diagnostic.eq_null", &[]),
|
("diagnostic.eq_null", &[]),
|
||||||
|
("diagnostic.like_numeric", &["column", "type"]),
|
||||||
("diagnostic.type_mismatch", &["column", "type"]),
|
("diagnostic.type_mismatch", &["column", "type"]),
|
||||||
("diagnostic.unknown_column", &["name", "table"]),
|
("diagnostic.unknown_column", &["name", "table"]),
|
||||||
("diagnostic.unknown_table", &["name"]),
|
("diagnostic.unknown_table", &["name"]),
|
||||||
|
|||||||
@@ -469,6 +469,8 @@ diagnostic:
|
|||||||
# runs, but the comparison is very likely not intended.
|
# runs, but the comparison is very likely not intended.
|
||||||
type_mismatch: "`{column}` is {type} — this comparison uses a value of a different type"
|
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`"
|
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 lifecycle event notes -----------------------------------
|
||||||
project:
|
project:
|
||||||
|
|||||||
Reference in New Issue
Block a user