Skip to content

feat(dialect): report whether placeholders are positional - #4

Merged
iamralch merged 1 commit into
mainfrom
feat/dialect-is-positional
Sep 5, 2026
Merged

feat(dialect): report whether placeholders are positional#4
iamralch merged 1 commit into
mainfrom
feat/dialect-is-positional

Conversation

@iamralch

@iamralch iamralch commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes #3.

Dialect described placeholder syntax but not the consequence that follows
from it: whether a bind can be referenced more than once.

That does not matter to this crate — 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:

-- 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" > ?)

Two ordering fields bind two values on Postgres and three on SQLite for the same
predicate. Getting it wrong raises nothing — the binds shift by one and the page
silently resumes from the wrong row.

The change

fn is_positional(&self) -> bool {
    self.placeholder(1) == self.placeholder(2)
}

Defaulted, so no existing Dialect impl breaks, and the default is exactly the
inference sqlx-aip was making from outside. It handles 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.

The three built-ins answer directly rather than rendering anything, and the
blanket impl Dialect for &D forwards it — a missed forward there would look
like a silent fallback to the default rather than a compile error, so a test
calls through a reference.

One test pins the default against the three explicit answers. Without it nothing
would notice the inference drifting away from what the built-ins say, and the
inference is what every non-overriding custom dialect gets.

Testing

58 lib tests (+3), clippy and fmt clean across sqlite,mysql.

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.
@iamralch
iamralch merged commit 3dde9bd into main Sep 5, 2026
4 checks passed
@iamralch
iamralch deleted the feat/dialect-is-positional branch September 5, 2026 11:12
iamralch added a commit to sqlx-contrib/sqlx-aip that referenced this pull request Sep 5, 2026
Follow-up to sqlx-contrib/sqlx-cel#4, which added Dialect::is_positional.

The local inference in cursor.rs goes away -- the trait answers it now. The
comparison the local helper made is the trait's default, so 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dialect should say whether its placeholders are positional

1 participant