Skip to content

Commit 4257811

Browse files
committed
Bind named placeholders from the compiler, scan DuckDB lists and ClickHouse nullables correctly
A parameter whose placeholder names it, SQL Server's and Spanner's @name and ClickHouse's {name:Type}, is now marked by the compiler: the mssql and googlesql converters carry the name on the ParamRef as the ClickHouse one already did, the analyzer keeps it whichever use of the parameter it sees first, and the plugin Parameter gains a named field. The Go codegen reads that instead of searching the query text for "@name", which bound a CAST-wrapped @A positionally, because the analyzer had named it after the column it was compared with, and matched "@id" inside "@identity_no". A row struct is scanned into and a params struct is passed as arguments, and DuckDB wants different types for the two: a list is duckdb.Composite when scanned and a plain slice when passed. Row structs used the params form. The imports for the element of a generic type such as duckdb.Composite[[]time.Time] were also missing, and sql.Named's database/sql import reached the querier interface, which does not use it. clickhouse-go dereferences a Nullable value only when Nullable is the column's outermost type, so LowCardinality(Nullable(T)) and SimpleAggregateFunction(f, Nullable(T)) come through as pointers that a sql.Null wrapper cannot scan; they take the pointer form now. sqlc-test-setup start skips SQL Server wherever install did, rather than failing on an Ubuntu release the packages are not published for. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rfiHyC4iRLyRWc3UmYEtT
1 parent 456a02b commit 4257811

28 files changed

Lines changed: 257 additions & 140 deletions

File tree

‎cmd/sqlc-test-setup/mssql.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,9 @@ func installMSSQL() error {
4545
return nil
4646
}
4747

48-
ubuntu, err := ubuntuRelease()
48+
ubuntu, err := mssqlSupported()
4949
if err != nil {
50-
log.Printf("sql server packages are published for Ubuntu only, skipping: %s", err)
51-
return nil
52-
}
53-
if runtime.GOOS != "linux" || (runtime.GOARCH != "amd64" && runtime.GOARCH != "arm64") || !mssqlUbuntuReleases[ubuntu] {
54-
log.Printf("sql server packages are not published for Ubuntu %s on %s/%s, skipping", ubuntu, runtime.GOOS, runtime.GOARCH)
50+
log.Printf("skipping: %s", err)
5551
return nil
5652
}
5753

@@ -106,6 +102,19 @@ func installMSSQL() error {
106102
return nil
107103
}
108104

105+
// mssqlSupported reports whether Microsoft publishes SQL Server packages
106+
// for this machine, returning its Ubuntu release, and otherwise why not.
107+
func mssqlSupported() (string, error) {
108+
ubuntu, err := ubuntuRelease()
109+
if err != nil {
110+
return "", fmt.Errorf("sql server packages are published for Ubuntu only: %w", err)
111+
}
112+
if runtime.GOOS != "linux" || (runtime.GOARCH != "amd64" && runtime.GOARCH != "arm64") || !mssqlUbuntuReleases[ubuntu] {
113+
return "", fmt.Errorf("sql server packages are not published for Ubuntu %s on %s/%s", ubuntu, runtime.GOOS, runtime.GOARCH)
114+
}
115+
return ubuntu, nil
116+
}
117+
109118
// ubuntuRelease reads the Ubuntu release from /etc/os-release.
110119
func ubuntuRelease() (string, error) {
111120
data, err := os.ReadFile("/etc/os-release")
@@ -144,8 +153,8 @@ func startMSSQL() error {
144153
return nil
145154
}
146155
if _, err := os.Stat(mssqlServer); err != nil {
147-
if _, err := ubuntuRelease(); err != nil || runtime.GOOS != "linux" {
148-
log.Println("sql server is not installed on this platform, skipping")
156+
if _, err := mssqlSupported(); err != nil {
157+
log.Printf("sql server is not installed here, skipping: %s", err)
149158
return nil
150159
}
151160
return fmt.Errorf("sql server is not installed: run `sqlc-test-setup install mssql` first")

‎internal/cmd/shim.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ func pluginQueryParam(p compiler.Parameter) *plugin.Parameter {
258258
return &plugin.Parameter{
259259
Number: int32(p.Number),
260260
Column: pluginQueryColumn(p.Column),
261+
Named: p.Named,
261262
}
262263
}
263264

‎internal/codegen/golang/clickhouse_type.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,22 @@ func clickhouseGoType(options *opts.Options, t *plugin.TypeExpr, nullable, neste
3838
}
3939
return "any"
4040

41+
// The driver hands database/sql a Nullable value as a pointer, and
42+
// dereferences it only when Nullable is the column's outermost type.
43+
// Wrapped in LowCardinality or SimpleAggregateFunction the pointer
44+
// comes through as is, which a sql.Null wrapper cannot scan, so a
45+
// nullable one takes the pointer form the nested types do.
4146
case "lowcardinality":
4247
if inner := typeExprArg(t, 0); inner != nil {
43-
return clickhouseGoType(options, inner, nullable || inner.Nullable, nested)
48+
wrapped := nullable || t.Nullable || inner.Nullable
49+
return clickhouseGoType(options, inner, wrapped, nested || wrapped)
4450
}
4551
return "any"
4652

4753
case "simpleaggregatefunction":
4854
if inner := typeExprLastArg(t); inner != nil {
49-
return clickhouseGoType(options, inner, nullable || inner.Nullable, nested)
55+
wrapped := nullable || t.Nullable || inner.Nullable
56+
return clickhouseGoType(options, inner, wrapped, nested || wrapped)
5057
}
5158
return "any"
5259

‎internal/codegen/golang/imports.go‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,6 @@ func buildImports(options *opts.Options, queries []Query, uses func(string) bool
182182
if uses("sql.Null") {
183183
std["database/sql"] = struct{}{}
184184
}
185-
for _, q := range queries {
186-
if q.Arg.NamedArgs && !q.Arg.isEmpty() {
187-
std["database/sql"] = struct{}{}
188-
}
189-
}
190185

191186
sqlpkg := parseDriver(options.SqlPackage)
192187
for _, q := range queries {
@@ -427,6 +422,13 @@ func (i *importer) queryImports(filename string) fileImports {
427422
return false
428423
}
429424

425+
// A query bound by name passes sql.Named arguments.
426+
for _, q := range gq {
427+
if q.Arg.NamedArgs && !q.Arg.isEmpty() {
428+
std["database/sql"] = struct{}{}
429+
}
430+
}
431+
430432
if anyNonCopyFrom {
431433
std["context"] = struct{}{}
432434
}
@@ -544,17 +546,62 @@ func trimSliceAndPointerPrefix(v string) string {
544546
// hasPrefixIgnoringSliceAndPointerPrefix reports whether the type s names
545547
// prefix once its slice and pointer prefixes are stripped. A map type names
546548
// it when its key or its value does, so map[string]decimal.Decimal uses the
547-
// decimal package.
549+
// decimal package, and an instantiated generic type names it when the type
550+
// or one of its arguments does, so duckdb.Composite[[]time.Time] uses both
551+
// the duckdb and the time packages.
548552
func hasPrefixIgnoringSliceAndPointerPrefix(s, prefix string) bool {
549553
trimmedS := trimSliceAndPointerPrefix(s)
550554
trimmedPrefix := trimSliceAndPointerPrefix(prefix)
551555
if key, value, ok := splitMapType(trimmedS); ok {
552556
return hasPrefixIgnoringSliceAndPointerPrefix(key, trimmedPrefix) ||
553557
hasPrefixIgnoringSliceAndPointerPrefix(value, trimmedPrefix)
554558
}
559+
if typ, args, ok := splitGenericType(trimmedS); ok {
560+
if strings.HasPrefix(typ, trimmedPrefix) {
561+
return true
562+
}
563+
for _, arg := range args {
564+
if hasPrefixIgnoringSliceAndPointerPrefix(arg, trimmedPrefix) {
565+
return true
566+
}
567+
}
568+
return false
569+
}
555570
return strings.HasPrefix(trimmedS, trimmedPrefix)
556571
}
557572

573+
// splitGenericType splits an instantiated generic type pkg.Name[A, B] into
574+
// pkg.Name and its type arguments, matching the brackets so an argument
575+
// that is itself a slice, a map or a generic type is kept whole.
576+
func splitGenericType(s string) (string, []string, bool) {
577+
open := strings.IndexByte(s, '[')
578+
if open <= 0 || !strings.HasSuffix(s, "]") {
579+
return "", nil, false
580+
}
581+
var args []string
582+
depth, start := 0, open+1
583+
for i := open; i < len(s); i++ {
584+
switch s[i] {
585+
case '[':
586+
depth++
587+
case ']':
588+
depth--
589+
if depth == 0 {
590+
if i != len(s)-1 {
591+
return "", nil, false
592+
}
593+
args = append(args, strings.TrimSpace(s[start:i]))
594+
}
595+
case ',':
596+
if depth == 1 {
597+
args = append(args, strings.TrimSpace(s[start:i]))
598+
start = i + 1
599+
}
600+
}
601+
}
602+
return s[:open], args, true
603+
}
604+
558605
// splitMapType splits map[K]V into K and V, matching the brackets so a key
559606
// that is itself a map or an array is kept whole.
560607
func splitMapType(s string) (string, string, bool) {

‎internal/codegen/golang/result.go‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func buildQueries(req *plugin.GenerateRequest, options *opts.Options, enums []En
231231

232232
qpl := int(*options.QueryParameterLimit)
233233

234-
namedArgs := placeholdersAreNamed(req.Settings.Engine, query.Text, query.Params)
234+
namedArgs := placeholdersAreNamed(query.Params)
235235

236236
if len(query.Params) == 1 && qpl != 0 {
237237
p := query.Params[0]
@@ -428,7 +428,13 @@ func columnsToStruct(req *plugin.GenerateRequest, options *opts.Options, name st
428428
Column: c.Column,
429429
}
430430
if c.embed == nil {
431-
f.Type = qualifyType(goParamType(req, options, c.Column), models, qualifier)
431+
// A row struct is scanned into, a params struct is passed as
432+
// arguments, and a driver can want different types for the two.
433+
if useID {
434+
f.Type = qualifyType(goType(req, options, c.Column), models, qualifier)
435+
} else {
436+
f.Type = qualifyType(goParamType(req, options, c.Column), models, qualifier)
437+
}
432438
} else {
433439
f.Type = qualifyType(c.embed.modelType, models, qualifier)
434440
f.EmbedFields = c.embed.fields
@@ -481,27 +487,16 @@ func checkIncompatibleFieldTypes(fields []Field) error {
481487
}
482488

483489
// placeholdersAreNamed reports whether every parameter of a query is bound
484-
// by name: SQL Server and Spanner name a parameter @name in the query
485-
// text, ClickHouse's server-side {name:Type} does the same, and their
486-
// drivers take such an argument as sql.Named. A query written with ? is
487-
// bound by position, so nothing is named unless every parameter is.
488-
func placeholdersAreNamed(engine, text string, params []*plugin.Parameter) bool {
490+
// by name, which the compiler marks on a parameter whose placeholder names
491+
// it: SQL Server's and Spanner's @name and ClickHouse's {name:Type}, which
492+
// their drivers take as sql.Named. A query written with ? is bound by
493+
// position, so nothing is named unless every parameter is.
494+
func placeholdersAreNamed(params []*plugin.Parameter) bool {
489495
if len(params) == 0 {
490496
return false
491497
}
492498
for _, p := range params {
493-
name := p.Column.GetName()
494-
if name == "" {
495-
return false
496-
}
497-
var named bool
498-
switch engine {
499-
case engineMSSQL, engineGoogleSQL:
500-
named = strings.Contains(text, "@"+name)
501-
case engineClickHouse:
502-
named = strings.Contains(text, "{"+name+":")
503-
}
504-
if !named {
499+
if !p.Named || p.Column.GetName() == "" {
505500
return false
506501
}
507502
}

‎internal/compiler/parse_core.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (c *Compiler) parseQueryCore(raw *ast.RawStmt, src string, pre *preprocess.
6363
cols = append(cols, coreColumn(col))
6464
}
6565
for _, p := range res.Parameters {
66-
params = append(params, Parameter{Number: p.Number, Column: coreParamColumn(p, namedParams)})
66+
params = append(params, Parameter{Number: p.Number, Column: coreParamColumn(p, namedParams), Named: p.Name != ""})
6767
}
6868
expanded, err = source.Mutate(rawSQL, c.expandCore(raw, res.Stars))
6969
if err != nil {

‎internal/compiler/query.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,7 @@ type Query struct {
6464
type Parameter struct {
6565
Number int
6666
Column *Column
67+
// Named is set when the query binds the parameter by name rather than
68+
// by position: its placeholder is @name or {name:Type}.
69+
Named bool
6770
}

‎internal/core/analyzer/analyzer.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func (a *analyzer) typeLimit(n ast.Node) error {
254254
if err != nil {
255255
return err
256256
}
257-
a.inferParam(pr.Number, exprType{typeOID: oid})
257+
a.inferParam(pr, exprType{typeOID: oid})
258258
return nil
259259
}
260260
_, err := a.typeExpr(n)

‎internal/core/analyzer/dml.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func (a *analyzer) bindValue(rel scopeRel, target *core.ClassColumn, v ast.Node)
167167
if target != nil {
168168
switch value := v.(type) {
169169
case *ast.ParamRef:
170-
a.inferParam(value.Number, columnType(rel, *target))
170+
a.inferParam(value, columnType(rel, *target))
171171
return nil
172172
case *ast.A_Const:
173173
return nil

‎internal/core/analyzer/expr.go‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -200,20 +200,27 @@ func flattenFields(fields *ast.List) []string {
200200
return out
201201
}
202202

203-
func (a *analyzer) typeParamRef(p *ast.ParamRef) (exprType, error) {
203+
// param is the parameter p refers to, recorded with the name the
204+
// placeholder carries, whichever of its uses is seen first.
205+
func (a *analyzer) param(p *ast.ParamRef) core.Parameter {
204206
cur, ok := a.params[p.Number]
205207
if !ok {
206-
cur = core.Parameter{Number: p.Number, Name: p.Name}
207-
a.params[p.Number] = cur
208+
cur = core.Parameter{Number: p.Number}
209+
}
210+
if cur.Name == "" {
211+
cur.Name = p.Name
208212
}
213+
a.params[p.Number] = cur
214+
return cur
215+
}
216+
217+
func (a *analyzer) typeParamRef(p *ast.ParamRef) (exprType, error) {
218+
cur := a.param(p)
209219
return exprType{typeOID: cur.TypeOID, expr: cur.Type.WithNullable(false), nullable: !cur.NotNull}, nil
210220
}
211221

212-
func (a *analyzer) inferParam(number int, t exprType) {
213-
cur, ok := a.params[number]
214-
if !ok {
215-
cur = core.Parameter{Number: number}
216-
}
222+
func (a *analyzer) inferParam(p *ast.ParamRef, t exprType) {
223+
cur := a.param(p)
217224
typed := cur.TypeOID == 0 && cur.Type == nil && (t.typeOID != 0 || t.expr != nil)
218225
if typed {
219226
cur.TypeOID = t.typeOID
@@ -232,7 +239,7 @@ func (a *analyzer) inferParam(number int, t exprType) {
232239
}
233240
}
234241
}
235-
a.params[number] = cur
242+
a.params[p.Number] = cur
236243
}
237244

238245
// nameParamAfter names a placeholder compared with a function call after
@@ -301,19 +308,19 @@ func (a *analyzer) typeAExpr(e *ast.A_Expr) (exprType, error) {
301308
}
302309

303310
if pr, ok := e.Lexpr.(*ast.ParamRef); ok && rightT.typeOID != 0 {
304-
a.inferParam(pr.Number, rightT)
311+
a.inferParam(pr, rightT)
305312
a.nameParamAfter(pr.Number, e.Rexpr)
306313
leftT = rightT
307314
} else if pr := castParamRef(e.Lexpr); pr != nil {
308-
a.inferParam(pr.Number, rightT)
315+
a.inferParam(pr, rightT)
309316
a.nameParamAfter(pr.Number, e.Rexpr)
310317
}
311318
if pr, ok := e.Rexpr.(*ast.ParamRef); ok && leftT.typeOID != 0 {
312-
a.inferParam(pr.Number, leftT)
319+
a.inferParam(pr, leftT)
313320
a.nameParamAfter(pr.Number, e.Lexpr)
314321
rightT = leftT
315322
} else if pr := castParamRef(e.Rexpr); pr != nil {
316-
a.inferParam(pr.Number, leftT)
323+
a.inferParam(pr, leftT)
317324
a.nameParamAfter(pr.Number, e.Lexpr)
318325
}
319326

@@ -668,7 +675,7 @@ func (a *analyzer) typeOperands(n ast.Node, other exprType) error {
668675
return err
669676
}
670677
if other.typeOID != 0 || other.expr != nil {
671-
a.inferParam(pr.Number, other)
678+
a.inferParam(pr, other)
672679
}
673680
return nil
674681
}

0 commit comments

Comments
 (0)