From 5bd0257b6ec3cd5e9347555f29846d437df6c578 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Thu, 27 Jul 2023 15:31:45 -0400 Subject: [PATCH 1/2] fix(compiler): correctly validate alias in order/group by clauses for joins Resolves #1886 Resolves #2398 Resolves #2399 --- internal/compiler/output_columns.go | 18 ++--- .../postgresql/stdlib/go/db.go | 31 +++++++++ .../postgresql/stdlib/go/models.go | 11 +++ .../postgresql/stdlib/go/query.sql.go | 68 +++++++++++++++++++ .../postgresql/stdlib/query.sql | 11 +++ .../postgresql/stdlib/sqlc.json | 12 ++++ 6 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/db.go create mode 100644 internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/models.go create mode 100644 internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go create mode 100644 internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql create mode 100644 internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/sqlc.json diff --git a/internal/compiler/output_columns.go b/internal/compiler/output_columns.go index 665e8892fe..07395e8f17 100644 --- a/internal/compiler/output_columns.go +++ b/internal/compiler/output_columns.go @@ -69,7 +69,7 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er if n.GroupClause != nil { for _, item := range n.GroupClause.Items { - if err := findColumnForNode(item, tables, n); err != nil { + if err := findColumnForNode(item, tables, targets); err != nil { return nil, err } } @@ -85,7 +85,7 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er if !ok { continue } - if err := findColumnForNode(sb.Node, tables, n); err != nil { + if err := findColumnForNode(sb.Node, tables, targets); err != nil { return nil, fmt.Errorf("%v: if you want to skip this validation, set 'strict_order_by' to false", err) } } @@ -101,7 +101,7 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er if !ok { continue } - if err := findColumnForNode(caseExpr.Xpr, tables, n); err != nil { + if err := findColumnForNode(caseExpr.Xpr, tables, targets); err != nil { return nil, fmt.Errorf("%v: if you want to skip this validation, set 'strict_order_by' to false", err) } } @@ -650,15 +650,15 @@ func outputColumnRefs(res *ast.ResTarget, tables []*Table, node *ast.ColumnRef) return cols, nil } -func findColumnForNode(item ast.Node, tables []*Table, n *ast.SelectStmt) error { +func findColumnForNode(item ast.Node, tables []*Table, targetList *ast.List) error { ref, ok := item.(*ast.ColumnRef) if !ok { return nil } - return findColumnForRef(ref, tables, n) + return findColumnForRef(ref, tables, targetList) } -func findColumnForRef(ref *ast.ColumnRef, tables []*Table, selectStatement *ast.SelectStmt) error { +func findColumnForRef(ref *ast.ColumnRef, tables []*Table, targetList *ast.List) error { parts := stringSlice(ref.Fields) var alias, name string if len(parts) == 1 { @@ -686,9 +686,11 @@ func findColumnForRef(ref *ast.ColumnRef, tables []*Table, selectStatement *ast. if foundColumn { continue } + } - // Find matching alias - for _, c := range selectStatement.TargetList.Items { + // Find matching alias if necessary + if found == 0 { + for _, c := range targetList.Items { resTarget, ok := c.(*ast.ResTarget) if !ok { continue diff --git a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/db.go b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/db.go new file mode 100644 index 0000000000..fb6ae669f6 --- /dev/null +++ b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/db.go @@ -0,0 +1,31 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.19.1 + +package querytest + +import ( + "context" + "database/sql" +) + +type DBTX interface { + ExecContext(context.Context, string, ...interface{}) (sql.Result, error) + PrepareContext(context.Context, string) (*sql.Stmt, error) + QueryContext(context.Context, string, ...interface{}) (*sql.Rows, error) + QueryRowContext(context.Context, string, ...interface{}) *sql.Row +} + +func New(db DBTX) *Queries { + return &Queries{db: db} +} + +type Queries struct { + db DBTX +} + +func (q *Queries) WithTx(tx *sql.Tx) *Queries { + return &Queries{ + db: tx, + } +} diff --git a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/models.go b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/models.go new file mode 100644 index 0000000000..031cba11cf --- /dev/null +++ b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/models.go @@ -0,0 +1,11 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.19.1 + +package querytest + +import () + +type Foo struct { + Email string +} diff --git a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go new file mode 100644 index 0000000000..5b0bb09502 --- /dev/null +++ b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go @@ -0,0 +1,68 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.19.1 +// source: query.sql + +package querytest + +import ( + "context" +) + +const columnAsGroupBy = `-- name: ColumnAsGroupBy :many +SELECT a.email AS id +FROM foo a JOIN foo b ON a.email = b.email +GROUP BY id +` + +func (q *Queries) ColumnAsGroupBy(ctx context.Context) ([]string, error) { + rows, err := q.db.QueryContext(ctx, columnAsGroupBy) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + items = append(items, id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const columnAsOrderBy = `-- name: ColumnAsOrderBy :many +SELECT a.email AS id +FROM foo a JOIN foo b ON a.email = b.email +ORDER BY id +` + +func (q *Queries) ColumnAsOrderBy(ctx context.Context) ([]string, error) { + rows, err := q.db.QueryContext(ctx, columnAsOrderBy) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + items = append(items, id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql new file mode 100644 index 0000000000..8888e83abc --- /dev/null +++ b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql @@ -0,0 +1,11 @@ +CREATE TABLE foo (email text not null); + +-- name: ColumnAsOrderBy :many +SELECT a.email AS id +FROM foo a JOIN foo b ON a.email = b.email +ORDER BY id; + +-- name: ColumnAsGroupBy :many +SELECT a.email AS id +FROM foo a JOIN foo b ON a.email = b.email +GROUP BY id; diff --git a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/sqlc.json b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/sqlc.json new file mode 100644 index 0000000000..de427d069f --- /dev/null +++ b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/sqlc.json @@ -0,0 +1,12 @@ +{ + "version": "1", + "packages": [ + { + "engine": "postgresql", + "path": "go", + "name": "querytest", + "schema": "query.sql", + "queries": "query.sql" + } + ] +} From 91e6734d2d4ed016b29c1471570075c68991431b Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Thu, 27 Jul 2023 16:58:09 -0400 Subject: [PATCH 2/2] remove dead code and split up test --- internal/compiler/output_columns.go | 7 +--- .../postgresql/stdlib/go/db.go | 31 +++++++++++++++ .../postgresql/stdlib/go/models.go | 11 ++++++ .../postgresql/stdlib/go/query.sql.go | 39 +++++++++++++++++++ .../postgresql/stdlib/query.sql | 6 +++ .../postgresql/stdlib/sqlc.json | 12 ++++++ .../postgresql/stdlib/go/query.sql.go | 29 -------------- .../postgresql/stdlib/query.sql | 5 --- 8 files changed, 100 insertions(+), 40 deletions(-) create mode 100644 internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/db.go create mode 100644 internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/models.go create mode 100644 internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/query.sql.go create mode 100644 internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/query.sql create mode 100644 internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/sqlc.json diff --git a/internal/compiler/output_columns.go b/internal/compiler/output_columns.go index 07395e8f17..ef39219f3e 100644 --- a/internal/compiler/output_columns.go +++ b/internal/compiler/output_columns.go @@ -675,17 +675,12 @@ func findColumnForRef(ref *ast.ColumnRef, tables []*Table, targetList *ast.List) } // Find matching column - var foundColumn bool for _, c := range t.Columns { if c.Name == name { found++ - foundColumn = true + break } } - - if foundColumn { - continue - } } // Find matching alias if necessary diff --git a/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/db.go b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/db.go new file mode 100644 index 0000000000..fb6ae669f6 --- /dev/null +++ b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/db.go @@ -0,0 +1,31 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.19.1 + +package querytest + +import ( + "context" + "database/sql" +) + +type DBTX interface { + ExecContext(context.Context, string, ...interface{}) (sql.Result, error) + PrepareContext(context.Context, string) (*sql.Stmt, error) + QueryContext(context.Context, string, ...interface{}) (*sql.Rows, error) + QueryRowContext(context.Context, string, ...interface{}) *sql.Row +} + +func New(db DBTX) *Queries { + return &Queries{db: db} +} + +type Queries struct { + db DBTX +} + +func (q *Queries) WithTx(tx *sql.Tx) *Queries { + return &Queries{ + db: tx, + } +} diff --git a/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/models.go b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/models.go new file mode 100644 index 0000000000..031cba11cf --- /dev/null +++ b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/models.go @@ -0,0 +1,11 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.19.1 + +package querytest + +import () + +type Foo struct { + Email string +} diff --git a/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/query.sql.go b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/query.sql.go new file mode 100644 index 0000000000..3fe70f51b6 --- /dev/null +++ b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/go/query.sql.go @@ -0,0 +1,39 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.19.1 +// source: query.sql + +package querytest + +import ( + "context" +) + +const columnAsGroupBy = `-- name: ColumnAsGroupBy :many +SELECT a.email AS id +FROM foo a JOIN foo b ON a.email = b.email +GROUP BY id +` + +func (q *Queries) ColumnAsGroupBy(ctx context.Context) ([]string, error) { + rows, err := q.db.QueryContext(ctx, columnAsGroupBy) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + items = append(items, id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/query.sql b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/query.sql new file mode 100644 index 0000000000..c0f410b505 --- /dev/null +++ b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/query.sql @@ -0,0 +1,6 @@ +CREATE TABLE foo (email text not null); + +-- name: ColumnAsGroupBy :many +SELECT a.email AS id +FROM foo a JOIN foo b ON a.email = b.email +GROUP BY id; diff --git a/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/sqlc.json b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/sqlc.json new file mode 100644 index 0000000000..de427d069f --- /dev/null +++ b/internal/endtoend/testdata/join_group_by_alias/postgresql/stdlib/sqlc.json @@ -0,0 +1,12 @@ +{ + "version": "1", + "packages": [ + { + "engine": "postgresql", + "path": "go", + "name": "querytest", + "schema": "query.sql", + "queries": "query.sql" + } + ] +} diff --git a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go index 5b0bb09502..c312dfa7e1 100644 --- a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go +++ b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/go/query.sql.go @@ -9,35 +9,6 @@ import ( "context" ) -const columnAsGroupBy = `-- name: ColumnAsGroupBy :many -SELECT a.email AS id -FROM foo a JOIN foo b ON a.email = b.email -GROUP BY id -` - -func (q *Queries) ColumnAsGroupBy(ctx context.Context) ([]string, error) { - rows, err := q.db.QueryContext(ctx, columnAsGroupBy) - if err != nil { - return nil, err - } - defer rows.Close() - var items []string - for rows.Next() { - var id string - if err := rows.Scan(&id); err != nil { - return nil, err - } - items = append(items, id) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const columnAsOrderBy = `-- name: ColumnAsOrderBy :many SELECT a.email AS id FROM foo a JOIN foo b ON a.email = b.email diff --git a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql index 8888e83abc..9abf85e687 100644 --- a/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql +++ b/internal/endtoend/testdata/join_order_by_alias/postgresql/stdlib/query.sql @@ -4,8 +4,3 @@ CREATE TABLE foo (email text not null); SELECT a.email AS id FROM foo a JOIN foo b ON a.email = b.email ORDER BY id; - --- name: ColumnAsGroupBy :many -SELECT a.email AS id -FROM foo a JOIN foo b ON a.email = b.email -GROUP BY id;