Skip to content

Commit d9b243b

Browse files
committed
compiler: expand stars in the query text on the core path
The core analyzer already resolved a star to the columns it covers, but only for the result set — the query it handed to codegen still said "SELECT *", so the generated SQL asked the database for whatever the table happened to hold at run time rather than the columns sqlc scanned into. Every case in the corpus that selects a star generated different code through the core than through the legacy path. The analyzer now reports each star along with the columns it stands for, sharing one list with the analyzers of the queries nested in it so a statement reports the stars in its subqueries and CTEs too. The compiler turns those into edits on the query text, which is where the engine's quoting rules and SQLite's jsonb wrapping live and where the legacy path already does the same rewrite — the two now produce byte-identical SQL across the corpus. Rewriting means reparsing to check the edit produced valid SQL, and the core path analyzes statements concurrently. A parser holds too much state for two goroutines to share one, so the compiler keeps the constructor and a goroutine builds its own. 181 of the 527 cases failing under the core context now pass.
1 parent 47bcc02 commit d9b243b

31 files changed

Lines changed: 1046 additions & 45 deletions

File tree

internal/compiler/engine.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ type Compiler struct {
3838
coreAnalysis bool
3939
coreDialect core.Option
4040

41+
// newParser builds a parser for the configured engine. The core path
42+
// analyzes statements concurrently, and a parser holds enough state that
43+
// two goroutines cannot share one, so a goroutine that has to parse
44+
// something of its own — the query text it just rewrote — builds its own.
45+
newParser func() Parser
46+
4147
schema []string
4248
}
4349

@@ -119,28 +125,29 @@ func (c *Compiler) initCore() error {
119125
var dialect core.Option
120126
switch c.conf.Engine {
121127
case config.EngineSQLite:
122-
c.parser = sqlite.NewParser()
128+
c.newParser = func() Parser { return sqlite.NewParser() }
123129
c.selector = newSQLiteSelector()
124130
dialect = sqlite.Dialect()
125131
case config.EngineMySQL:
126-
c.parser = dolphin.NewParser()
132+
c.newParser = func() Parser { return dolphin.NewParser() }
127133
c.selector = newDefaultSelector()
128134
dialect = dolphin.Dialect()
129135
case config.EnginePostgreSQL:
130-
c.parser = postgresql.NewParser()
136+
c.newParser = func() Parser { return postgresql.NewParser() }
131137
c.selector = newDefaultSelector()
132138
dialect = postgresql.Dialect()
133139
case config.EngineClickHouse:
134-
c.parser = clickhouse.NewParser()
140+
c.newParser = func() Parser { return clickhouse.NewParser() }
135141
c.selector = newDefaultSelector()
136142
dialect = clickhouse.Dialect()
137143
case config.EngineGoogleSQL:
138-
c.parser = googlesql.NewParser()
144+
c.newParser = func() Parser { return googlesql.NewParser() }
139145
c.selector = newDefaultSelector()
140146
dialect = googlesql.Dialect()
141147
default:
142148
return fmt.Errorf("unknown engine: %s", c.conf.Engine)
143149
}
150+
c.parser = c.newParser()
144151
c.coreDialect = dialect
145152
return nil
146153
}

internal/compiler/expand.go

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,38 @@ func (c *Compiler) quote(x string) string {
7878
}
7979
}
8080

81+
// starOldFunc measures how much of the query text a star reference occupies,
82+
// so an edit replaces the reference and nothing else. Each part is measured
83+
// both bare and quoted: an embed was rewritten to "table.*" in the query text,
84+
// preserving the way the user quoted the table, so it is measured the same way
85+
// as a star reference the user wrote.
86+
func (c *Compiler) starOldFunc(parts []string) func(string) int {
87+
old := make([]string, 0, len(parts))
88+
for _, p := range parts {
89+
if p == "*" {
90+
old = append(old, p)
91+
} else {
92+
old = append(old, c.quoteIdent(p))
93+
}
94+
}
95+
return func(s string) int {
96+
length := 0
97+
for i, o := range old {
98+
if hasSeparator := i > 0; hasSeparator {
99+
length++
100+
}
101+
if strings.HasPrefix(s[length:], o) {
102+
length += len(o)
103+
} else if quoted := c.quote(o); strings.HasPrefix(s[length:], quoted) {
104+
length += len(quoted)
105+
} else {
106+
length += len(o)
107+
}
108+
}
109+
return length
110+
}
111+
}
112+
81113
func (c *Compiler) expandStmt(qc *QueryCatalog, raw *ast.RawStmt, node ast.Node) ([]source.Edit, error) {
82114
tables, err := c.sourceTables(qc, node)
83115
if err != nil {
@@ -157,37 +189,9 @@ func (c *Compiler) expandStmt(qc *QueryCatalog, raw *ast.RawStmt, node ast.Node)
157189
cols = append(cols, cname)
158190
}
159191
}
160-
var old []string
161-
for _, p := range parts {
162-
if p == "*" {
163-
old = append(old, p)
164-
} else {
165-
old = append(old, c.quoteIdent(p))
166-
}
167-
}
168-
169-
// An embed was rewritten to "table.*" in the query text, so it is
170-
// measured the same way as a star reference the user wrote.
171-
oldFunc := func(s string) int {
172-
length := 0
173-
for i, o := range old {
174-
if hasSeparator := i > 0; hasSeparator {
175-
length++
176-
}
177-
if strings.HasPrefix(s[length:], o) {
178-
length += len(o)
179-
} else if quoted := c.quote(o); strings.HasPrefix(s[length:], quoted) {
180-
length += len(quoted)
181-
} else {
182-
length += len(o)
183-
}
184-
}
185-
return length
186-
}
187-
188192
edits = append(edits, source.Edit{
189193
Location: res.Location - raw.StmtLocation,
190-
OldFunc: oldFunc,
194+
OldFunc: c.starOldFunc(parts),
191195
New: strings.Join(cols, ", "),
192196
})
193197
}

internal/compiler/expand_core.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package compiler
2+
3+
import (
4+
"strings"
5+
6+
"github.com/sqlc-dev/sqlc/internal/core"
7+
"github.com/sqlc-dev/sqlc/internal/source"
8+
"github.com/sqlc-dev/sqlc/internal/sql/ast"
9+
)
10+
11+
// expandCore rewrites the stars in a query's text with the columns the core
12+
// analyzer resolved them to. The analyzer has already walked the statement and
13+
// its subqueries, so there is nothing left to look up here: what remains is
14+
// deciding how each name is written, which is the engine's business and not
15+
// the core's.
16+
func (c *Compiler) expandCore(raw *ast.RawStmt, stars []core.StarExpansion) ([]source.Edit, error) {
17+
if len(stars) == 0 {
18+
return nil, nil
19+
}
20+
edits := make([]source.Edit, 0, len(stars))
21+
seen := make(map[int]bool, len(stars))
22+
for _, star := range stars {
23+
// A statement analyzed more than once — the same CTE referenced twice,
24+
// say — reports its stars once per pass. Editing one twice would
25+
// overlap, so only the first is kept.
26+
if seen[star.Location] {
27+
continue
28+
}
29+
seen[star.Location] = true
30+
31+
// Everything before the star qualifies it: "foo.*" is scoped to foo,
32+
// while a bare "*" covers every relation in the FROM clause.
33+
scope := strings.Join(star.Fields[:len(star.Fields)-1], ".")
34+
35+
// An unqualified star that covers more than one relation may name the
36+
// same column twice, so those are written with their relation.
37+
counts := map[string]int{}
38+
if scope == "" {
39+
for _, col := range star.Columns {
40+
counts[col.Name]++
41+
}
42+
}
43+
44+
cols := make([]string, 0, len(star.Columns))
45+
for _, col := range star.Columns {
46+
cname := col.Name
47+
if star.Alias != "" {
48+
cname = star.Alias
49+
}
50+
cname = c.quoteIdent(cname)
51+
if scope != "" {
52+
cname = c.quoteIdent(scope) + "." + cname
53+
}
54+
if counts[cname] > 1 {
55+
cname = c.quoteIdent(col.Relation) + "." + cname
56+
}
57+
58+
// This is important for SQLite in particular which needs to wrap
59+
// jsonb column values with `json(colname)` so they're in a publicly
60+
// usable format (i.e. not jsonb).
61+
cols = append(cols, c.selector.ColumnExpr(cname, &Column{
62+
Name: col.Name,
63+
DataType: col.DataType,
64+
}))
65+
}
66+
67+
edits = append(edits, source.Edit{
68+
Location: star.Location - raw.StmtLocation,
69+
OldFunc: c.starOldFunc(star.Fields),
70+
New: strings.Join(cols, ", "),
71+
})
72+
}
73+
return edits, nil
74+
}

internal/compiler/parse_core.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package compiler
22

33
import (
44
"errors"
5+
"fmt"
56
"strings"
67

78
"github.com/sqlc-dev/sqlc/internal/core"
@@ -64,6 +65,21 @@ func (c *Compiler) parseQueryCore(raw *ast.RawStmt, src string, pre *preprocess.
6465
for _, p := range res.Parameters {
6566
params = append(params, Parameter{Number: p.Number, Column: coreParamColumn(p, namedParams)})
6667
}
68+
edits, err := c.expandCore(raw, res.Stars)
69+
if err != nil {
70+
return nil, err
71+
}
72+
expanded, err = source.Mutate(rawSQL, edits)
73+
if err != nil {
74+
return nil, err
75+
}
76+
}
77+
78+
// If the query string was edited, make sure the syntax is valid
79+
if expanded != rawSQL {
80+
if _, err := c.newParser().Parse(strings.NewReader(expanded)); err != nil {
81+
return nil, fmt.Errorf("edited query syntax is invalid: %w", err)
82+
}
6783
}
6884

6985
trimmed, comments, err := source.StripComments(expanded)

internal/core/analysis.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,39 @@ const (
1010
)
1111

1212
type PrepareResult struct {
13-
Command Command `json:"command,omitempty"`
14-
Columns []Column `json:"columns"`
15-
Parameters []Parameter `json:"parameters"`
13+
Command Command `json:"command,omitempty"`
14+
Columns []Column `json:"columns"`
15+
Parameters []Parameter `json:"parameters"`
16+
Stars []StarExpansion `json:"stars,omitempty"`
17+
}
18+
19+
// StarExpansion is what a star in a target list stands for. The analyzer
20+
// resolves the reference against the query's scope and reports the columns it
21+
// covers; rewriting the query text with them is the caller's to do, since only
22+
// it knows how the engine quotes an identifier.
23+
type StarExpansion struct {
24+
// Location is where the target the star belongs to starts, measured the
25+
// way the AST measures a node: from the beginning of the file the
26+
// statement was parsed from.
27+
Location int `json:"location"`
28+
29+
// Fields is the reference as it was written, with the star as its last
30+
// element: ["*"] for a bare star and ["foo", "*"] for a qualified one.
31+
Fields []string `json:"fields"`
32+
33+
// Alias is the output name the target was given, if any.
34+
Alias string `json:"alias,omitempty"`
35+
36+
Columns []StarColumn `json:"columns"`
37+
}
38+
39+
// StarColumn is a single column a star expanded to.
40+
type StarColumn struct {
41+
// Relation is the name the column's relation goes by in the query, which
42+
// is its alias when it was given one.
43+
Relation string `json:"relation,omitempty"`
44+
Name string `json:"name"`
45+
DataType string `json:"data_type,omitempty"`
1646
}
1747

1848
type ColumnSource struct {

internal/core/analyzer/analyzer.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ func Prepare(cat *core.Catalog, stmt ast.Node) (core.PrepareResult, error) {
1414
a := &analyzer{
1515
cat: cat,
1616
params: map[int]core.Parameter{},
17+
stars: &[]core.StarExpansion{},
1718
}
1819
switch s := stmt.(type) {
1920
case *ast.SelectStmt:
@@ -63,6 +64,18 @@ type analyzer struct {
6364

6465
// resolving guards against an alias that refers to itself.
6566
resolving map[string]bool
67+
68+
// stars are the expansions every star in the statement asked for, shared
69+
// with the analyzers of the queries nested in it so one statement reports
70+
// all of them.
71+
stars *[]core.StarExpansion
72+
}
73+
74+
func (a *analyzer) recordStar(s core.StarExpansion) {
75+
if a.stars == nil {
76+
return
77+
}
78+
*a.stars = append(*a.stars, s)
6679
}
6780

6881
// subquery analyzes a nested SELECT. It shares the parameter set, so a
@@ -74,6 +87,7 @@ func (a *analyzer) subquery(s *ast.SelectStmt) (*analyzer, error) {
7487
params: a.params,
7588
outer: a.scope,
7689
ctes: a.ctes,
90+
stars: a.stars,
7791
}
7892
if err := sub.analyzeSelect(s); err != nil {
7993
return nil, err
@@ -105,11 +119,15 @@ func derivedRel(alias string, cols []core.Column) scopeRel {
105119
}
106120

107121
func (a *analyzer) result() core.PrepareResult {
108-
return core.PrepareResult{
122+
res := core.PrepareResult{
109123
Command: a.command,
110124
Columns: a.columns,
111125
Parameters: orderedParams(a.params),
112126
}
127+
if a.stars != nil {
128+
res.Stars = *a.stars
129+
}
130+
return res
113131
}
114132

115133
func orderedParams(m map[int]core.Parameter) []core.Parameter {

internal/core/analyzer/projection.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func (a *analyzer) projectTarget(rt *ast.ResTarget) error {
1414
if cr, ok := rt.Val.(*ast.ColumnRef); ok {
1515
fields = flattenFields(cr.Fields)
1616
if isStar(fields) {
17-
a.emitStar(fields)
17+
a.emitStar(rt, fields)
1818
return nil
1919
}
2020
}
@@ -79,16 +79,23 @@ func isStar(fields []string) bool {
7979
return len(fields) > 0 && fields[len(fields)-1] == "*"
8080
}
8181

82-
func (a *analyzer) emitStar(fields []string) {
82+
func (a *analyzer) emitStar(rt *ast.ResTarget, fields []string) {
8383
relName := ""
8484
if len(fields) > 1 {
8585
relName = fields[0]
8686
}
87+
// The star is reported along with the columns it covers, so the query text
88+
// can be rewritten to name them.
89+
star := core.StarExpansion{Location: rt.Location, Fields: fields}
90+
if rt.Name != nil {
91+
star.Alias = *rt.Name
92+
}
8793
for _, rel := range a.scope.rels {
8894
if relName != "" && rel.alias != relName {
8995
continue
9096
}
9197
a.columns = slices.Grow(a.columns, len(rel.cols))
98+
star.Columns = slices.Grow(star.Columns, len(rel.cols))
9299
for _, c := range rel.cols {
93100
col := core.Column{
94101
Name: c.Name,
@@ -100,6 +107,12 @@ func (a *analyzer) emitStar(fields []string) {
100107
col.DataType, col.IsArray = a.typeNameOf(exprType{typeOID: c.TypeOID})
101108
a.decorateSource(&col, c.AttOID, rel.alias)
102109
a.columns = append(a.columns, col)
110+
star.Columns = append(star.Columns, core.StarColumn{
111+
Relation: rel.alias,
112+
Name: c.Name,
113+
DataType: col.DataType,
114+
})
103115
}
104116
}
117+
a.recordStar(star)
105118
}

internal/endtoend/testdata/experiment_coreanalyzer/mysql/go/query.sql.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/experiment_coreanalyzer/postgresql/stdlib/go/query.sql.go

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

0 commit comments

Comments
 (0)