fix(sqlite): correct parameter binding when mixing sqlc.arg() with bare "?"#4471
Open
abgoyal wants to merge 1 commit into
Open
fix(sqlite): correct parameter binding when mixing sqlc.arg() with bare "?"#4471abgoyal wants to merge 1 commit into
abgoyal wants to merge 1 commit into
Conversation
…re ? When a SQLite query uses both sqlc.arg() named parameters and bare ? placeholders, the generated SQL has numbered ?N for named params but leaves bare ? unnumbered. SQLite's auto-numbering for bare ? then conflicts with the explicit ?N values, silently binding arguments to wrong columns. Fix by numbering all placeholders sequentially in text order when the mixed case is detected, ensuring positional argument passing matches the generated ?N values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(sqlite): correct parameter binding when mixing sqlc.arg() with bare ?
What happened?
When a SQLite query mixes
sqlc.arg()named parameters with bare positional?placeholders, sqlc generates SQL with mismatched parameter numbering that silently binds values to wrong columns.Given this query:
Before (broken): sqlc generates
SET name = ?2 ... WHERE org_id = ? AND name = ?3— the bare?gets auto-assigned index 1 by SQLite, but the Go function passesNewNameas the 1st positional arg. Result:SET name = OrgID,WHERE org_id = NewName.After (fixed): all placeholders are numbered in text order:
SET name = ?1 ... WHERE org_id = ?2 AND name = ?3, matching the positional argument order.Root cause
Commit c2dcd56 ("Allow for mixed parameters types ($1 or ?) and sqlc.arg()") added mixed-param support for PostgreSQL and MySQL but did not cover SQLite. PostgreSQL doesn't have this issue because
$Nis inherently numbered and args are sorted by number. MySQL doesn't have it because all params emit bare?(no numbering, purely positional). SQLite is the only engine that generates numbered?Nfor named params but also accepts unnumbered?— and those two styles have incompatible binding semantics when mixed in the same query.The
NamedParametersrewriter assigns numbered?Ntosqlc.arg()params while skipping positions pre-reserved for bare?— but it never renumbers the bare?itself. Since SQLite's auto-numbering for bare?is independent of explicit?N, the two schemes conflict.Fix
When SQLite has both named params and bare
?in the same query:ParamSetdoesn't skip them?during the Apply walk alongside named params, assigning all numbers in text order?with explicit?NThis ensures the numbered placeholders in the output SQL match the positional argument order in the generated Go function.
Test plan
sqlitevariant to existingmix_param_typesend-to-end test (matching the pg/mysql variants from the original mixed-param commit)sqlc diffshows no changes)go test -run TestReplay/base ./internal/endtoend/passes