From 479e19ee9aac81c78493df83abc48d16a8512da2 Mon Sep 17 00:00:00 2001 From: Jille Timmermans Date: Wed, 28 Jun 2023 12:18:59 +0200 Subject: [PATCH 1/4] Understand sqlc.slice() in more expression types This was needed to understand PostgreSQL IN() which doens't surface as an *ast.In --- internal/compiler/resolve.go | 6 ++++ .../postgresql/stdlib/go/query.sql.go | 31 ++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/internal/compiler/resolve.go b/internal/compiler/resolve.go index 02e3c4c7a5..c71d896126 100644 --- a/internal/compiler/resolve.go +++ b/internal/compiler/resolve.go @@ -153,6 +153,7 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar, DataType: dataType, IsNamedParam: isNamed, NotNull: p.NotNull(), + IsSqlcSlice: p.IsSqlcSlice(), }, }) continue @@ -220,6 +221,7 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar, Length: c.Length, Table: table, IsNamedParam: isNamed, + IsSqlcSlice: p.IsSqlcSlice(), }, }) } @@ -284,6 +286,7 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar, IsArray: c.IsArray, Table: table, IsNamedParam: isNamed, + IsSqlcSlice: p.IsSqlcSlice(), }, }) } @@ -352,6 +355,7 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar, DataType: "any", IsNamedParam: isNamed, NotNull: p.NotNull(), + IsSqlcSlice: p.IsSqlcSlice(), }, }) continue @@ -392,6 +396,7 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar, DataType: dataType(paramType), NotNull: p.NotNull(), IsNamedParam: isNamed, + IsSqlcSlice: p.IsSqlcSlice(), }, }) } @@ -457,6 +462,7 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar, Table: &ast.TableName{Schema: schema, Name: rel}, Length: c.Length, IsNamedParam: isNamed, + IsSqlcSlice: p.IsSqlcSlice(), }, }) } else { diff --git a/internal/endtoend/testdata/sqlc_slice/postgresql/stdlib/go/query.sql.go b/internal/endtoend/testdata/sqlc_slice/postgresql/stdlib/go/query.sql.go index 220f024aed..215467ef1d 100644 --- a/internal/endtoend/testdata/sqlc_slice/postgresql/stdlib/go/query.sql.go +++ b/internal/endtoend/testdata/sqlc_slice/postgresql/stdlib/go/query.sql.go @@ -7,6 +7,7 @@ package querytest import ( "context" + "strings" ) const funcParamIdent = `-- name: FuncParamIdent :many @@ -17,11 +18,22 @@ WHERE name = $1 type FuncParamIdentParams struct { Slug string - Favourites int32 + Favourites []int32 } func (q *Queries) FuncParamIdent(ctx context.Context, arg FuncParamIdentParams) ([]string, error) { - rows, err := q.db.QueryContext(ctx, funcParamIdent, arg.Slug, arg.Favourites) + query := funcParamIdent + var queryParams []interface{} + queryParams = append(queryParams, arg.Slug) + if len(arg.Favourites) > 0 { + for _, v := range arg.Favourites { + queryParams = append(queryParams, v) + } + query = strings.Replace(query, "/*SLICE:favourites*/?", strings.Repeat(",?", len(arg.Favourites))[1:], 1) + } else { + query = strings.Replace(query, "/*SLICE:favourites*/?", "NULL", 1) + } + rows, err := q.db.QueryContext(ctx, query, queryParams...) if err != nil { return nil, err } @@ -51,11 +63,22 @@ WHERE name = $1 type FuncParamStringParams struct { Slug string - Favourites int32 + Favourites []int32 } func (q *Queries) FuncParamString(ctx context.Context, arg FuncParamStringParams) ([]string, error) { - rows, err := q.db.QueryContext(ctx, funcParamString, arg.Slug, arg.Favourites) + query := funcParamString + var queryParams []interface{} + queryParams = append(queryParams, arg.Slug) + if len(arg.Favourites) > 0 { + for _, v := range arg.Favourites { + queryParams = append(queryParams, v) + } + query = strings.Replace(query, "/*SLICE:favourites*/?", strings.Repeat(",?", len(arg.Favourites))[1:], 1) + } else { + query = strings.Replace(query, "/*SLICE:favourites*/?", "NULL", 1) + } + rows, err := q.db.QueryContext(ctx, query, queryParams...) if err != nil { return nil, err } From da816bc6ffab9160ee82cd59271dc596484200d2 Mon Sep 17 00:00:00 2001 From: Jille Timmermans Date: Wed, 28 Jun 2023 12:44:03 +0200 Subject: [PATCH 2/4] Fix interaction between query_parameter_limit and sqlc.slice() The sqlc.slice() templating code needed to understand how to refer to a variable when query_parameter_limit prevented struct emission. issue #2268 --- internal/codegen/golang/query.go | 14 +++++++++--- .../golang/templates/stdlib/queryCode.tmpl | 8 +++---- .../postgresql/go/query.sql.go | 22 +++++++++++++++++++ .../postgresql/query.sql | 4 ++++ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/internal/codegen/golang/query.go b/internal/codegen/golang/query.go index 0e553b64ff..9aee4430ed 100644 --- a/internal/codegen/golang/query.go +++ b/internal/codegen/golang/query.go @@ -117,10 +117,8 @@ func (v QueryValue) Params() string { for _, f := range v.Struct.Fields { if !f.HasSqlcSlice() && strings.HasPrefix(f.Type, "[]") && f.Type != "[]byte" && !v.SQLDriver.IsPGX() { out = append(out, "pq.Array("+v.Name+"."+f.Name+")") - } else if !v.EmitStruct() && v.IsStruct() { - out = append(out, toLowerCase(f.Name)) } else { - out = append(out, v.Name+"."+f.Name) + out = append(out, v.VariableForField(f)) } } } @@ -189,6 +187,16 @@ func (v QueryValue) Scan() string { return "\n" + strings.Join(out, ",\n") } +func (v QueryValue) VariableForField(f Field) string { + if !v.IsStruct() { + return v.Name + } + if !v.EmitStruct() { + return toLowerCase(f.Name) + } + return v.Name + "." + f.Name +} + // A struct used to generate methods and fields on the Queries struct type Query struct { Cmd string diff --git a/internal/codegen/golang/templates/stdlib/queryCode.tmpl b/internal/codegen/golang/templates/stdlib/queryCode.tmpl index cde37d81ed..34bc4e0ece 100644 --- a/internal/codegen/golang/templates/stdlib/queryCode.tmpl +++ b/internal/codegen/golang/templates/stdlib/queryCode.tmpl @@ -126,16 +126,16 @@ func (q *Queries) {{.MethodName}}(ctx context.Context, {{ dbarg }} {{.Arg.Pair}} {{- $arg := .Arg }} {{- range .Arg.Struct.Fields }} {{- if .HasSqlcSlice }} - if len({{$arg.Name}}.{{.Name}}) > 0 { - for _, v := range {{$arg.Name}}.{{.Name}} { + if len({{$arg.VariableForField .}}) > 0 { + for _, v := range {{$arg.VariableForField .}} { queryParams = append(queryParams, v) } - query = strings.Replace(query, "/*SLICE:{{.Column.Name}}*/?", strings.Repeat(",?", len({{$arg.Name}}.{{.Name}}))[1:], 1) + query = strings.Replace(query, "/*SLICE:{{.Column.Name}}*/?", strings.Repeat(",?", len({{$arg.VariableForField .}}))[1:], 1) } else { query = strings.Replace(query, "/*SLICE:{{.Column.Name}}*/?", "NULL", 1) } {{- else }} - queryParams = append(queryParams, {{$arg.Name}}.{{.Name}}) + queryParams = append(queryParams, {{$arg.VariableForField .}}) {{- end }} {{- end }} {{- else }} diff --git a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go index b631aad9c5..de6d828bcf 100644 --- a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go +++ b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go @@ -8,6 +8,7 @@ package querytest import ( "context" "database/sql" + "strings" ) const createAuthor = `-- name: CreateAuthor :one @@ -47,6 +48,27 @@ func (q *Queries) DeleteAuthor(ctx context.Context, id int64) error { return err } +const deleteAuthors = `-- name: DeleteAuthors :exec +DELETE FROM authors +WHERE id IN ($2) AND name = $1 +` + +func (q *Queries) DeleteAuthors(ctx context.Context, name string, ids []int64) error { + query := deleteAuthors + var queryParams []interface{} + queryParams = append(queryParams, name) + if len(ids) > 0 { + for _, v := range ids { + queryParams = append(queryParams, v) + } + query = strings.Replace(query, "/*SLICE:ids*/?", strings.Repeat(",?", len(ids))[1:], 1) + } else { + query = strings.Replace(query, "/*SLICE:ids*/?", "NULL", 1) + } + _, err := q.db.ExecContext(ctx, query, queryParams...) + return err +} + const getAuthor = `-- name: GetAuthor :one SELECT id, name, bio, country_code FROM authors WHERE name = $1 AND country_code = $2 LIMIT 1 diff --git a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql index eb10e5cb4c..206ba51003 100644 --- a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql +++ b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql @@ -25,3 +25,7 @@ RETURNING *; -- name: DeleteAuthor :exec DELETE FROM authors WHERE id = $1; + +-- name: DeleteAuthors :exec +DELETE FROM authors +WHERE id IN (sqlc.slice(ids)) AND name = $1; From 4c42ff8bba2844b54d849b4b4c2dc9568380fbd6 Mon Sep 17 00:00:00 2001 From: Jille Timmermans Date: Wed, 28 Jun 2023 12:52:02 +0200 Subject: [PATCH 3/4] Fix imports if a file has both sqlc.slice() and regular arrays The pq import was missing in those cases. I ran into this while writing a test case for #2268 comment 2. --- internal/codegen/golang/imports.go | 7 +++--- .../postgresql/go/models.go | 1 + .../postgresql/go/query.sql.go | 23 ++++++++++++++----- .../postgresql/query.sql | 7 +++--- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/internal/codegen/golang/imports.go b/internal/codegen/golang/imports.go index 083762b1d9..bd6fafd393 100644 --- a/internal/codegen/golang/imports.go +++ b/internal/codegen/golang/imports.go @@ -344,12 +344,12 @@ func (i *importer) queryImports(filename string) fileImports { if !q.Arg.isEmpty() { if q.Arg.IsStruct() { for _, f := range q.Arg.Struct.Fields { - if strings.HasPrefix(f.Type, "[]") && f.Type != "[]byte" { + if strings.HasPrefix(f.Type, "[]") && f.Type != "[]byte" && !f.HasSqlcSlice() { return true } } } else { - if strings.HasPrefix(q.Arg.Type(), "[]") && q.Arg.Type() != "[]byte" { + if strings.HasPrefix(q.Arg.Type(), "[]") && q.Arg.Type() != "[]byte" && !q.Arg.HasSqlcSlices() { return true } } @@ -375,7 +375,8 @@ func (i *importer) queryImports(filename string) fileImports { sqlpkg := parseDriver(i.Settings.Go.SqlPackage) if sqlcSliceScan() { std["strings"] = struct{}{} - } else if sliceScan() && !sqlpkg.IsPGX() { + } + if sliceScan() && !sqlpkg.IsPGX() { pkg[ImportSpec{Path: "github.com/lib/pq"}] = struct{}{} } diff --git a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/models.go b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/models.go index 1413a261d4..a2f05ad411 100644 --- a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/models.go +++ b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/models.go @@ -13,4 +13,5 @@ type Author struct { Name string Bio sql.NullString CountryCode string + Titles []string } diff --git a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go index de6d828bcf..44150161d3 100644 --- a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go +++ b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go @@ -9,31 +9,40 @@ import ( "context" "database/sql" "strings" + + "github.com/lib/pq" ) const createAuthor = `-- name: CreateAuthor :one INSERT INTO authors ( - name, bio, country_code + name, bio, country_code, titles ) VALUES ( - $1, $2, $3 + $1, $2, $3, $4 ) -RETURNING id, name, bio, country_code +RETURNING id, name, bio, country_code, titles ` type CreateAuthorParams struct { Name string Bio sql.NullString CountryCode string + Titles []string } func (q *Queries) CreateAuthor(ctx context.Context, arg CreateAuthorParams) (Author, error) { - row := q.db.QueryRowContext(ctx, createAuthor, arg.Name, arg.Bio, arg.CountryCode) + row := q.db.QueryRowContext(ctx, createAuthor, + arg.Name, + arg.Bio, + arg.CountryCode, + pq.Array(arg.Titles), + ) var i Author err := row.Scan( &i.ID, &i.Name, &i.Bio, &i.CountryCode, + pq.Array(&i.Titles), ) return i, err } @@ -70,7 +79,7 @@ func (q *Queries) DeleteAuthors(ctx context.Context, name string, ids []int64) e } const getAuthor = `-- name: GetAuthor :one -SELECT id, name, bio, country_code FROM authors +SELECT id, name, bio, country_code, titles FROM authors WHERE name = $1 AND country_code = $2 LIMIT 1 ` @@ -82,12 +91,13 @@ func (q *Queries) GetAuthor(ctx context.Context, name string, countryCode string &i.Name, &i.Bio, &i.CountryCode, + pq.Array(&i.Titles), ) return i, err } const listAuthors = `-- name: ListAuthors :many -SELECT id, name, bio, country_code FROM authors +SELECT id, name, bio, country_code, titles FROM authors ORDER BY name ` @@ -105,6 +115,7 @@ func (q *Queries) ListAuthors(ctx context.Context) ([]Author, error) { &i.Name, &i.Bio, &i.CountryCode, + pq.Array(&i.Titles), ); err != nil { return nil, err } diff --git a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql index 206ba51003..f5ab43058f 100644 --- a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql +++ b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql @@ -3,7 +3,8 @@ CREATE TABLE authors ( id BIGSERIAL PRIMARY KEY, name text NOT NULL, bio text, - country_code CHAR(2) NOT NULL + country_code CHAR(2) NOT NULL, + titles TEXT[] ); -- name: GetAuthor :one @@ -16,9 +17,9 @@ ORDER BY name; -- name: CreateAuthor :one INSERT INTO authors ( - name, bio, country_code + name, bio, country_code, titles ) VALUES ( - $1, $2, $3 + $1, $2, $3, $4 ) RETURNING *; From 2d982313a21a535a3f90671e7e582e0eab99e2da Mon Sep 17 00:00:00 2001 From: Jille Timmermans Date: Wed, 28 Jun 2023 12:57:36 +0200 Subject: [PATCH 4/4] Fix broken interaction between query_parameter_limit and pq.Array() issue #2268 --- internal/codegen/golang/query.go | 2 +- .../postgresql/go/query.sql.go | 17 +++++++++++++++++ .../postgresql/query.sql | 3 +++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/codegen/golang/query.go b/internal/codegen/golang/query.go index 9aee4430ed..aeb1c106a2 100644 --- a/internal/codegen/golang/query.go +++ b/internal/codegen/golang/query.go @@ -116,7 +116,7 @@ func (v QueryValue) Params() string { } else { for _, f := range v.Struct.Fields { if !f.HasSqlcSlice() && strings.HasPrefix(f.Type, "[]") && f.Type != "[]byte" && !v.SQLDriver.IsPGX() { - out = append(out, "pq.Array("+v.Name+"."+f.Name+")") + out = append(out, "pq.Array("+v.VariableForField(f)+")") } else { out = append(out, v.VariableForField(f)) } diff --git a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go index 44150161d3..7cc29f9dee 100644 --- a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go +++ b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/go/query.sql.go @@ -47,6 +47,23 @@ func (q *Queries) CreateAuthor(ctx context.Context, arg CreateAuthorParams) (Aut return i, err } +const createAuthorOnlyTitles = `-- name: CreateAuthorOnlyTitles :one +INSERT INTO authors (name, titles) VALUES ($1, $2) RETURNING id, name, bio, country_code, titles +` + +func (q *Queries) CreateAuthorOnlyTitles(ctx context.Context, name string, titles []string) (Author, error) { + row := q.db.QueryRowContext(ctx, createAuthorOnlyTitles, name, pq.Array(titles)) + var i Author + err := row.Scan( + &i.ID, + &i.Name, + &i.Bio, + &i.CountryCode, + pq.Array(&i.Titles), + ) + return i, err +} + const deleteAuthor = `-- name: DeleteAuthor :exec DELETE FROM authors WHERE id = $1 diff --git a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql index f5ab43058f..8edd6ae9b9 100644 --- a/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql +++ b/internal/endtoend/testdata/query_parameter_limit_to_two/postgresql/query.sql @@ -30,3 +30,6 @@ WHERE id = $1; -- name: DeleteAuthors :exec DELETE FROM authors WHERE id IN (sqlc.slice(ids)) AND name = $1; + +-- name: CreateAuthorOnlyTitles :one +INSERT INTO authors (name, titles) VALUES ($1, $2) RETURNING *;