From a184d36f4bfbdf588aa9e33a3f5de3588ca471ee Mon Sep 17 00:00:00 2001 From: Svetlin Ralchev Date: Sat, 5 Sep 2026 15:15:34 +0400 Subject: [PATCH] refactor: ask the dialect whether it is positional sqlx-cel now answers this itself, so the local inference goes. The comparison it made is the trait's default, which is why nothing in the test suite changes -- including the custom `?1` dialect, which does not override the method and so exercises that default and gets the same answer the local copy gave. The reasoning moves upstream with the code: `Dialect::is_positional` documents why the property matters and what the middle case is. --- src/cursor.rs | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/src/cursor.rs b/src/cursor.rs index 5c04ad1..0835cfe 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -32,7 +32,7 @@ use crate::error::{Dimension, Error}; /// in each of them and is bound once — but a positional `?` has no way to point /// backwards, so each one consumes its own bind and the value list repeats. /// Three ordering fields bind three values on Postgres and six on SQLite, for -/// the same predicate. +/// the same predicate. [`Dialect::is_positional`] is which. /// /// Getting this wrong does not raise an error. The binds shift by one and the /// page silently resumes from the wrong row. @@ -67,7 +67,7 @@ pub(crate) fn rewrite( .map(|(field, value)| bind(field, value)) .collect::>()?; - let positional = is_positional(dialect); + let positional = dialect.is_positional(); let mut repeated: Vec = Vec::new(); let mut sql = String::from("("); @@ -115,25 +115,6 @@ pub(crate) fn rewrite( Ok((Some(sql), if positional { repeated } else { keys })) } -/// Whether `dialect` renders every placeholder alike, so that a bind cannot be -/// referenced twice. -/// -/// Decided by asking the dialect rather than by naming the three built in, so a -/// caller's own [`Dialect`] is classified correctly too. It handles the awkward -/// middle case for free: a dialect emitting SQLite's numbered `?1` / `?2` form -/// is *positional in syntax but addressable*, renders the two differently, and -/// is correctly treated as numbered. -/// -/// This infers a behavioural property from rendered text, which is a smell. The -/// honest fix is a `Dialect::is_positional` in sqlx-cel, defaulting to exactly -/// this comparison; until that exists, this is the only signal the trait -/// offers. -fn is_positional(dialect: &impl Dialect) -> bool { - // Two arbitrary adjacent indices. Any dialect that distinguishes parameters - // at all distinguishes these, so rendering them alike means it does not. - dialect.placeholder(1) == dialect.placeholder(2) -} - /// Converts one cursor value into the bind value it compares against. /// /// Mechanical apart from the null: [`CursorValue`] widens sized integers to 64