From 7d0b7189509cd223ec6b975ecd2aca2b2a4fbc7b Mon Sep 17 00:00:00 2001 From: Svetlin Ralchev Date: Sat, 5 Sep 2026 15:09:26 +0400 Subject: [PATCH] feat(dialect): report whether placeholders are positional Closes #3. `Dialect` described placeholder syntax but not the consequence: whether a bind can be referenced more than once. That does not matter here -- `transpile` binds each literal once and never points at an earlier one -- but it decides correctness for a caller splicing a fragment that names one bind from two places. sqlx-aip's key-set cursor predicate pins each more-significant column in every clause after the first, so two ordering fields bind two values on Postgres and three on SQLite for the same predicate, and getting it wrong shifts the page rather than raising anything. Defaulted, so no existing impl breaks, and the default is the inference sqlx-aip was making from outside: render two adjacent indices and compare them. That handles the awkward middle case -- SQLite's numbered `?1` form is positional in syntax but addressable, renders the two differently, and is correctly reported as not positional. The three built-ins answer directly instead, and a test pins the default against them: without it nothing would notice the inference drifting away from the explicit answers, which is what a custom dialect gets. --- README.md | 7 ++++ src/dialect.rs | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/README.md b/README.md index c77ec2c..f7b1140 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,13 @@ of them with an identical value list: † SQLite parses `REGEXP` but resolves it to a function the application must register; without one it fails at execution with `no such function: REGEXP`. +`Dialect::is_positional` reports whether the placeholders render alike, so a +bind cannot be referenced twice. Nothing here needs it — `transpile` binds each +literal once — but a caller splicing a fragment that names one bind from two +places does, and getting it wrong shifts every subsequent bind. It defaults to +comparing two rendered placeholders, which correctly reports SQLite's numbered +`?1` form as *not* positional. + Dialects are pure text and always available. *Binding* the values needs the matching Cargo feature — `postgres` (default), `sqlite`, `mysql` — which supplies `Encode`/`Type` for `Value` and enables `bind_all`. With no driver diff --git a/src/dialect.rs b/src/dialect.rs index bc96ff3..4725d7b 100644 --- a/src/dialect.rs +++ b/src/dialect.rs @@ -24,6 +24,34 @@ pub trait Dialect { /// still emitted in the order they must be bound. fn placeholder(&self, index: usize) -> String; + /// Whether every placeholder renders alike, so that a bind cannot be + /// referenced twice. + /// + /// This crate never needs it: [`transpile`](crate::transpile) binds each + /// literal once and never points at an earlier one. It matters to callers + /// that splice a fragment referencing a bind more than once — a key-set + /// cursor predicate pins each more-significant column in every clause after + /// the first: + /// + /// ```sql + /// -- numbered: $1 is bound once, referenced twice + /// ("title" > $1) OR ("title" = $1 AND "id" > $2) + /// -- positional: each ? consumes its own bind, so the value list repeats + /// ("title" > ?) OR ("title" = ? AND "id" > ?) + /// ``` + /// + /// The default asks [`placeholder`](Dialect::placeholder) to render two + /// adjacent indices and compares them, which is right for any dialect that + /// distinguishes parameters at all. That includes the awkward middle case: + /// a dialect emitting SQLite's numbered `?1` / `?2` form is positional in + /// syntax but still addressable, renders the two differently, and is + /// correctly reported as *not* positional. + /// + /// Override it to answer without rendering anything. + fn is_positional(&self) -> bool { + self.placeholder(1) == self.placeholder(2) + } + /// Quotes a possibly-dotted column path, one segment at a time. /// /// Defaults to ANSI double quotes with embedded `"` doubled, which is what @@ -72,6 +100,9 @@ impl Dialect for &D { fn placeholder(&self, index: usize) -> String { (**self).placeholder(index) } + fn is_positional(&self) -> bool { + (**self).is_positional() + } fn quote_ident(&self, column: &str) -> String { (**self).quote_ident(column) } @@ -96,6 +127,12 @@ impl Dialect for Postgres { format!("${index}") } + /// `$1` names a parameter, so one bind can be referenced from several + /// places in the same statement. + fn is_positional(&self) -> bool { + false + } + /// POSIX regex. CEL's `matches` is RE2 and Postgres's `~` is POSIX ERE; /// they agree on common syntax and diverge at the edges, so a pattern that /// leans on RE2 specifics may behave differently or raise a Postgres error. @@ -128,6 +165,12 @@ impl Dialect for Sqlite { "?".to_string() } + /// SQLite also accepts a numbered `?NNN`, which *is* addressable, but this + /// dialect emits the anonymous form. + fn is_positional(&self) -> bool { + true + } + fn regex(&self, lhs: &str, rhs: &str) -> Option { Some(format!("{lhs} REGEXP {rhs}")) } @@ -150,6 +193,10 @@ impl Dialect for MySql { "?".to_string() } + fn is_positional(&self) -> bool { + true + } + /// Backticks, because `"…"` is a string literal in MySQL's default /// `ANSI_QUOTES`-off mode. An embedded backtick is doubled. fn quote_ident(&self, column: &str) -> String { @@ -225,6 +272,69 @@ mod tests { assert_eq!(MySql.placeholder(7), "?"); } + /// The property a caller splicing a bind into two places depends on. + #[test] + fn positional_dialects_are_the_ones_that_render_every_placeholder_alike() { + assert!(!Postgres.is_positional()); + assert!(Sqlite.is_positional()); + assert!(MySql.is_positional()); + } + + /// The three override the default, so nothing checks the default against + /// them unless something does it here. A divergence would mean a custom + /// dialect that does not override gets a different answer to a built-in + /// with the same placeholder syntax. + #[test] + fn the_default_inference_agrees_with_every_explicit_answer() { + fn inferred(dialect: &impl Dialect) -> bool { + dialect.placeholder(1) == dialect.placeholder(2) + } + assert_eq!(inferred(&Postgres), Postgres.is_positional()); + assert_eq!(inferred(&Sqlite), Sqlite.is_positional()); + assert_eq!(inferred(&MySql), MySql.is_positional()); + } + + /// A dialect that does not override gets the inference -- including the + /// case the inference exists for: `?1` looks positional and is not. + #[test] + fn a_custom_dialect_falls_back_to_the_inference() { + // Calls through the blanket impl for a reference, which has to forward + // the new method or it silently falls back to the default. + fn by_reference(dialect: impl Dialect) -> bool { + dialect.is_positional() + } + + struct Anonymous; + impl Dialect for Anonymous { + fn name(&self) -> &'static str { + "anonymous" + } + fn placeholder(&self, _: usize) -> String { + "?".to_string() + } + fn regex(&self, _: &str, _: &str) -> Option { + None + } + } + + struct Numbered; + impl Dialect for Numbered { + fn name(&self) -> &'static str { + "numbered" + } + fn placeholder(&self, index: usize) -> String { + format!("?{index}") + } + fn regex(&self, _: &str, _: &str) -> Option { + None + } + } + + assert!(Anonymous.is_positional()); + assert!(!Numbered.is_positional(), "?1 is addressable"); + assert!(!by_reference(&Numbered)); + } + #[test] fn mysql_concatenates_with_concat_not_pipes() { assert_eq!(