Skip to content

Commit 7445b49

Browse files
committed
fix(sqlite): correct parameter binding when mixing sqlc.arg() with bare ?
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.
1 parent a3b0cfd commit 7445b49

8 files changed

Lines changed: 165 additions & 0 deletions

File tree

internal/endtoend/testdata/mix_param_types/sqlite/go/db.go

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/mix_param_types/sqlite/go/models.go

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/mix_param_types/sqlite/go/test.sql.go

Lines changed: 59 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
CREATE TABLE bar (
2+
id integer not null,
3+
name text not null,
4+
phone text not null
5+
);
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"version": "1",
3+
"packages": [
4+
{
5+
"path": "go",
6+
"name": "querytest",
7+
"schema": "schema.sql",
8+
"queries": "test.sql",
9+
"engine": "sqlite"
10+
}
11+
]
12+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-- name: CountOne :one
2+
SELECT count(1) FROM bar WHERE id = sqlc.arg(id) AND name <> ?;
3+
4+
-- name: CountTwo :one
5+
SELECT count(1) FROM bar WHERE id = ? AND name <> sqlc.arg(name);
6+
7+
-- name: CountThree :one
8+
SELECT count(1) FROM bar WHERE id > ? AND phone <> sqlc.arg(phone) AND name <> ?;

internal/sql/named/param_set.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ func (p *ParamSet) Add(param Param) int {
4747
return argn
4848
}
4949

50+
// AddAnonymous allocates the next available parameter position without
51+
// associating a name. Used for bare ? placeholders that need explicit numbering.
52+
func (p *ParamSet) AddAnonymous() int {
53+
argn := p.nextArgNum()
54+
p.positionToName[argn] = ""
55+
return argn
56+
}
57+
5058
// FetchMerge fetches an indexed parameter, and merges `mergeP` into it
5159
// Returns: the merged parameter and whether it was a named parameter
5260
func (p *ParamSet) FetchMerge(idx int, mergeP Param) (param Param, isNamed bool) {

internal/sql/rewrite/parameters.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,18 @@ func NamedParameters(engine config.Engine, raw *ast.RawStmt, numbs map[int]bool,
8282
foundFunc := astutils.Search(raw, named.IsParamFunc)
8383
foundSign := astutils.Search(raw, named.IsParamSign)
8484
hasNamedParameterSupport := engine != config.EngineMySQL
85+
86+
// SQLite uses numbered ?N for sqlc.arg() but also accepts bare ?. When both
87+
// coexist we must number all placeholders in text order so that positional
88+
// argument binding matches the generated ?N values.
89+
foundBare := astutils.Search(raw, isBareParamRef)
90+
hasMixedParams := engine == config.EngineSQLite &&
91+
len(foundBare.Items) > 0 &&
92+
len(foundFunc.Items)+len(foundSign.Items) > 0
93+
if hasMixedParams {
94+
numbs = nil
95+
}
96+
8597
allParams := named.NewParamSet(numbs, hasNamedParameterSupport)
8698

8799
if len(foundFunc.Items)+len(foundSign.Items) == 0 {
@@ -183,10 +195,29 @@ func NamedParameters(engine config.Engine, raw *ast.RawStmt, numbs map[int]bool,
183195
})
184196
return false
185197

198+
case hasMixedParams && isBareParamRef(node):
199+
ref := node.(*ast.ParamRef)
200+
argn := allParams.AddAnonymous()
201+
cr.Replace(&ast.ParamRef{
202+
Number: argn,
203+
Location: ref.Location,
204+
})
205+
edits = append(edits, source.Edit{
206+
Location: ref.Location - raw.StmtLocation,
207+
Old: "?",
208+
New: fmt.Sprintf("?%d", argn),
209+
})
210+
return false
211+
186212
default:
187213
return true
188214
}
189215
}, nil)
190216

191217
return node.(*ast.RawStmt), allParams, edits
192218
}
219+
220+
func isBareParamRef(node ast.Node) bool {
221+
ref, ok := node.(*ast.ParamRef)
222+
return ok && !ref.Dollar
223+
}

0 commit comments

Comments
 (0)