Skip to content

Commit baf228d

Browse files
committed
fix: check column references in ORDER BY (sqlc-dev#1411)
1 parent 4837b07 commit baf228d

File tree

5 files changed

+156
-2
lines changed

5 files changed

+156
-2
lines changed

internal/compiler/output_columns.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,41 @@ func outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, error) {
6767

6868
if n.GroupClause != nil {
6969
for _, item := range n.GroupClause.Items {
70-
ref, ok := item.(*ast.ColumnRef)
70+
if err := findColumnForNode(item, tables, n); err != nil {
71+
return nil, err
72+
}
73+
}
74+
}
75+
if n.SortClause != nil {
76+
for _, item := range n.SortClause.Items {
77+
sb, ok := item.(*ast.SortBy)
7178
if !ok {
7279
continue
7380
}
7481

75-
if err := findColumnForRef(ref, tables, n); err != nil {
82+
if err := findColumnForNode(sb.Node, tables, n); err != nil {
7683
return nil, err
7784
}
7885
}
7986
}
87+
if n.WindowClause != nil {
88+
for _, item := range n.WindowClause.Items {
89+
sb, ok := item.(*ast.List)
90+
if !ok {
91+
continue
92+
}
93+
for _, single := range sb.Items {
94+
caseExpr, ok := single.(*ast.CaseExpr)
95+
if !ok {
96+
continue
97+
}
98+
99+
if err := findColumnForNode(caseExpr.Xpr, tables, n); err != nil {
100+
return nil, err
101+
}
102+
}
103+
}
104+
}
80105

81106
// For UNION queries, targets is empty and we need to look for the
82107
// columns in Largs.
@@ -516,6 +541,14 @@ func outputColumnRefs(res *ast.ResTarget, tables []*Table, node *ast.ColumnRef)
516541
return cols, nil
517542
}
518543

544+
func findColumnForNode(item ast.Node, tables []*Table, n *ast.SelectStmt) error {
545+
ref, ok := item.(*ast.ColumnRef)
546+
if !ok {
547+
return nil
548+
}
549+
return findColumnForRef(ref, tables, n)
550+
}
551+
519552
func findColumnForRef(ref *ast.ColumnRef, tables []*Table, selectStatement *ast.SelectStmt) error {
520553
parts := stringSlice(ref.Fields)
521554
var alias, name string

internal/compiler/parse_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
package compiler
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
"testing"
7+
8+
"github.com/kyleconroy/sqlc/internal/config"
9+
"github.com/kyleconroy/sqlc/internal/engine/dolphin"
10+
"github.com/kyleconroy/sqlc/internal/engine/postgresql"
11+
"github.com/kyleconroy/sqlc/internal/engine/sqlite"
12+
"github.com/kyleconroy/sqlc/internal/opts"
13+
)
14+
15+
func Test_ParseQueryErrors(t *testing.T) {
16+
for _, tc := range []struct {
17+
name string
18+
engine config.Engine
19+
createStmt string
20+
selectStmt string
21+
parser Parser
22+
wantErr error
23+
}{
24+
{
25+
name: "unreferenced order column postgresql",
26+
engine: config.EnginePostgreSQL,
27+
createStmt: `CREATE TABLE authors (id INT);`,
28+
selectStmt: `SELECT id FROM authors ORDER BY foo;`,
29+
parser: postgresql.NewParser(),
30+
wantErr: fmt.Errorf(`column reference "foo" not found`),
31+
},
32+
{
33+
name: "unreferenced order column mysql",
34+
engine: config.EngineMySQL,
35+
createStmt: `CREATE TABLE authors (id INT);`,
36+
selectStmt: `SELECT id FROM authors ORDER BY foo;`,
37+
parser: dolphin.NewParser(),
38+
wantErr: fmt.Errorf(`column reference "foo" not found`),
39+
},
40+
{
41+
name: "unreferenced order column sqlite",
42+
engine: config.EngineSQLite,
43+
createStmt: `CREATE TABLE authors (id INT);`,
44+
selectStmt: `SELECT id FROM authors ORDER BY foo;`,
45+
parser: sqlite.NewParser(),
46+
wantErr: fmt.Errorf(`column reference "foo" not found`),
47+
},
48+
{
49+
name: "unreferenced group column postgresql",
50+
engine: config.EnginePostgreSQL,
51+
createStmt: `CREATE TABLE authors ( id INT );`,
52+
selectStmt: `SELECT id FROM authors GROUP BY foo;`,
53+
parser: postgresql.NewParser(),
54+
wantErr: fmt.Errorf(`column reference "foo" not found`),
55+
},
56+
{
57+
name: "unreferenced group column mysql",
58+
engine: config.EngineMySQL,
59+
createStmt: `CREATE TABLE authors (id INT);`,
60+
selectStmt: `SELECT id FROM authors GROUP BY foo;`,
61+
parser: dolphin.NewParser(),
62+
wantErr: fmt.Errorf(`column reference "foo" not found`),
63+
},
64+
{
65+
name: "unreferenced group column sqlite",
66+
engine: config.EngineSQLite,
67+
createStmt: `CREATE TABLE authors (id INT);`,
68+
selectStmt: `SELECT id FROM authors GROUP BY foo;`,
69+
parser: sqlite.NewParser(),
70+
wantErr: fmt.Errorf(`column reference "foo" not found`),
71+
},
72+
} {
73+
t.Run(tc.name, func(t *testing.T) {
74+
conf := config.SQL{
75+
Engine: tc.engine,
76+
}
77+
combo := config.CombinedSettings{}
78+
comp := NewCompiler(conf, combo)
79+
stmts, err := tc.parser.Parse(strings.NewReader(tc.createStmt))
80+
if err != nil {
81+
t.Fatalf("cannot parse test catalog: %v", err)
82+
}
83+
err = comp.catalog.Update(stmts[0], comp)
84+
if err != nil {
85+
t.Fatalf("cannot update test catalog: %v", err)
86+
}
87+
stmts, err = tc.parser.Parse(strings.NewReader(tc.selectStmt))
88+
if err != nil {
89+
t.Errorf("Parse failed: %v", err)
90+
}
91+
if len(stmts) != 1 {
92+
t.Errorf("expected one statement, got %d", len(stmts))
93+
}
94+
95+
_, err = comp.parseQuery(stmts[0].Raw, tc.selectStmt, opts.Parser{})
96+
if err == nil {
97+
t.Fatalf("expected parseQuery to return an error, got nil")
98+
}
99+
if err.Error() != tc.wantErr.Error() {
100+
t.Errorf("error message: want %s, got %s", tc.wantErr, err.Error())
101+
}
102+
})
103+
}
104+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-- Example queries for sqlc
2+
CREATE TABLE authors (
3+
id INT
4+
);
5+
6+
-- name: ListAuthors :many
7+
SELECT id FROM authors
8+
ORDER BY adfadsf;
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
version: 1
2+
packages:
3+
- path: "go"
4+
name: "querytest"
5+
engine: "postgresql"
6+
schema: "query.sql"
7+
queries: "query.sql"
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# package querytest
2+
query.sql:8:10: column reference "adfadsf" not found

0 commit comments

Comments
 (0)