Skip to content

Commit 7fcad42

Browse files
committed
preprocess: add a side-table golden to each testdata case
The rewritten SQL only shows half of what the preprocessor produces. The other half — parameter names and nullability, embed and slice spans, placeholder numbering and the offset map back to the original text — was covered by a handful of hand-written Go tests that each poked at one case. Record it as a golden instead. Every testdata directory now holds a side_table.json alongside its output.sql, rendered by reading the result back through the same API the compiler uses, so all 59 cases exercise the whole side table rather than the six that had a bespoke test. The hand-written tests are gone. Two supporting changes: - Result.Statements() exposes the statements in source order, so the side table can be walked without looking each one up by offset - the whitespace trailing the final semicolon is no longer reported as a statement; it is still written to the output, it is just not something a query can be found in Renaming the errors helper to cover ParamErr as well as Err turned up two cases whose input also tripped the numbering-gap check. Their inputs are now valid so each case tests one thing, and the gap and mixed-style errors get cases of their own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015BSM9xGoosq58r1Mz6B7YP
1 parent b31f7ce commit 7fcad42

74 files changed

Lines changed: 1133 additions & 170 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

internal/sql/preprocess/CLAUDE.md

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,33 @@ original source, so errors point at what the user wrote.
6363

6464
## Invariants
6565

66-
- **Line structure is preserved.** A rewrite never adds or removes a newline, so
67-
line numbers survive even before `Origin` is applied.
66+
- **A rewrite never adds a line.** Replacements are single-line, so a line
67+
number taken from the rewritten text is never past the end of the original. It
68+
can *lose* lines, when the sqlc call itself spanned several — map offsets
69+
through `Origin` rather than trusting line numbers.
6870
- **An invalid statement is copied through untouched.** The engine still parses
6971
what the user wrote and the error is reported per statement, not per file.
7072
- **Nothing inside a comment or literal is rewritten.** Query annotations like
7173
`-- name: GetAuthor :one` are comments, so they are always safe.
7274

7375
## Tests
7476

75-
`testdata/<engine>/<case>/` holds an `input.sql`, the expected `output.sql` and,
76-
for invalid input, a `stderr.txt`. `TestRewrite` runs one subtest per directory.
77-
Regenerate the goldens with:
77+
`testdata/<engine>/<case>/` holds the input and the expected results:
78+
79+
| file | contents |
80+
| --- | --- |
81+
| `input.sql` | the query file |
82+
| `output.sql` | the rewritten SQL |
83+
| `side_table.json` | everything the preprocessor recorded |
84+
| `stderr.txt` | reported errors, only when the input is invalid |
85+
86+
`TestRewrite` runs one subtest per directory. `side_table.json` is rendered by
87+
reading the result back through the same API the compiler uses, so it covers
88+
parameter names and nullability, embed and slice spans, placeholder numbering
89+
and the offset map — a parameter's `location` is its offset in the rewritten
90+
text and its `origin` is where it came from.
91+
92+
Regenerate every golden with:
7893

7994
```bash
8095
go test ./internal/sql/preprocess -update

internal/sql/preprocess/preprocess.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,11 @@ type edit struct {
106106
newStart, newEnd int
107107
}
108108

109+
// Statements returns every statement in the rewritten text, in source order.
110+
func (r *Result) Statements() []*Statement {
111+
return r.stmts
112+
}
113+
109114
// Statement returns the preprocessing results for the statement covering the
110115
// given offset in the rewritten text. It never returns nil.
111116
func (r *Result) Statement(location int) *Statement {
@@ -176,8 +181,18 @@ func Dialected(d Dialect, src string) (*Result, error) {
176181
var b strings.Builder
177182
b.Grow(len(src))
178183

179-
for _, span := range l.statements() {
184+
spans := l.statements()
185+
for i, span := range spans {
180186
start, end := span[0], span[1]
187+
188+
// Whatever trails the last semicolon is only whitespace in a
189+
// well-formed file. It still belongs in the output, but it is not a
190+
// statement.
191+
if i == len(spans)-1 && strings.TrimSpace(src[start:end]) == "" {
192+
b.WriteString(src[start:end])
193+
continue
194+
}
195+
181196
occs := l.scan(start, end)
182197
stmt := &Statement{Start: b.Len()}
183198

Lines changed: 131 additions & 160 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package preprocess_test
22

33
import (
4+
"encoding/json"
45
"flag"
56
"fmt"
67
"os"
@@ -21,12 +22,14 @@ import (
2122
var update = flag.Bool("update", false, "update the testdata golden files")
2223

2324
// TestRewrite runs every case under testdata. Each case is a directory holding
24-
// an input.sql, the expected output.sql and, when the input is invalid, a
25-
// stderr.txt with the reported error. Cases are grouped by engine:
25+
// the input and the expected results, grouped by engine:
2626
//
27-
// testdata/<engine>/<case>/input.sql
28-
// testdata/<engine>/<case>/output.sql
29-
// testdata/<engine>/<case>/stderr.txt (optional)
27+
// testdata/<engine>/<case>/input.sql the query file
28+
// testdata/<engine>/<case>/output.sql the rewritten SQL
29+
// testdata/<engine>/<case>/side_table.json what the preprocessor recorded
30+
// testdata/<engine>/<case>/stderr.txt reported errors (only when invalid)
31+
//
32+
// Regenerate the golden files with `go test ./internal/sql/preprocess -update`.
3033
func TestRewrite(t *testing.T) {
3134
for _, dir := range cases(t) {
3235
t.Run(filepath.ToSlash(dir), func(t *testing.T) {
@@ -40,11 +43,134 @@ func TestRewrite(t *testing.T) {
4043
}
4144

4245
compare(t, filepath.Join(path, "output.sql"), res.Text)
46+
compare(t, filepath.Join(path, "side_table.json"), sideTable(res))
4347
compare(t, filepath.Join(path, "stderr.txt"), errors(input, res))
48+
49+
// A rewrite only ever replaces text with something shorter or
50+
// equal in lines, so a line number taken from the rewritten text is
51+
// never past the end of the original.
52+
if want, got := strings.Count(input, "\n"), strings.Count(res.Text, "\n"); got > want {
53+
t.Errorf("rewrite added lines: got %d, want at most %d", got, want)
54+
}
4455
})
4556
}
4657
}
4758

59+
// The JSON shape of the side table the preprocessor records alongside the
60+
// rewritten SQL. Locations are offsets into the rewritten text; origins are the
61+
// offsets they map back to in the original source.
62+
type statement struct {
63+
Start int `json:"start"`
64+
End int `json:"end"`
65+
Dollar bool `json:"dollar"`
66+
Params []parameter `json:"params,omitempty"`
67+
Embeds []embed `json:"embeds,omitempty"`
68+
Error string `json:"error,omitempty"`
69+
ErrorAt int `json:"error_at,omitempty"`
70+
ParamError string `json:"param_error,omitempty"`
71+
}
72+
73+
type parameter struct {
74+
Number int `json:"number"`
75+
Location int `json:"location"`
76+
Origin int `json:"origin"`
77+
Name string `json:"name,omitempty"`
78+
// Named is false for a placeholder the user wrote themselves, which the
79+
// preprocessor numbers but does not name.
80+
Named bool `json:"named"`
81+
// Nullable is true when sqlc.narg() forced the parameter nullable, even
82+
// though the schema says it is NOT NULL.
83+
Nullable bool `json:"nullable,omitempty"`
84+
Slice bool `json:"slice,omitempty"`
85+
}
86+
87+
type embed struct {
88+
Table string `json:"table"`
89+
Location int `json:"location"`
90+
Origin int `json:"origin"`
91+
Orig string `json:"orig"`
92+
}
93+
94+
// sideTable renders everything the preprocessor recorded, reading it back
95+
// through the same API the compiler uses.
96+
func sideTable(res *preprocess.Result) string {
97+
out := make([]statement, 0, len(res.Statements()))
98+
for _, stmt := range res.Statements() {
99+
s := statement{Start: stmt.Start, End: stmt.End, Dollar: stmt.Dollar}
100+
if stmt.Err != nil {
101+
s.Error = stmt.Err.Error()
102+
if e, ok := stmt.Err.(*sqlerr.Error); ok {
103+
s.ErrorAt = e.Location
104+
}
105+
}
106+
if stmt.ParamErr != nil {
107+
s.ParamError = stmt.ParamErr.Error()
108+
}
109+
110+
locations := make([]int, 0, len(stmt.Numbers))
111+
for loc := range stmt.Numbers {
112+
locations = append(locations, loc)
113+
}
114+
sort.Ints(locations)
115+
for _, loc := range locations {
116+
number := stmt.Numbers[loc]
117+
name, _ := stmt.Params.NameFor(number)
118+
// Merging against a NOT NULL parameter is how the compiler asks
119+
// whether the user overrode nullability with sqlc.narg().
120+
p, isNamed := stmt.Params.FetchMerge(number, named.NewInferredParam(name, true))
121+
_, isSlice := stmt.Slices[loc]
122+
s.Params = append(s.Params, parameter{
123+
Number: number,
124+
Location: loc,
125+
Origin: res.Origin(loc),
126+
Name: name,
127+
Named: isNamed,
128+
Nullable: isNamed && !p.NotNull(),
129+
Slice: isSlice,
130+
})
131+
}
132+
133+
for _, e := range stmt.Embeds {
134+
s.Embeds = append(s.Embeds, embed{
135+
Table: e.Table.Name,
136+
Location: e.Location,
137+
Origin: res.Origin(e.Location),
138+
Orig: e.Orig(),
139+
})
140+
}
141+
out = append(out, s)
142+
}
143+
144+
blob, err := json.MarshalIndent(out, "", " ")
145+
if err != nil {
146+
panic(err)
147+
}
148+
return string(blob) + "\n"
149+
}
150+
151+
// errors renders every validation error the preprocessor recorded, one per
152+
// line, as "line:column: message" against the original source.
153+
func errors(input string, res *preprocess.Result) string {
154+
var out []string
155+
for _, stmt := range res.Statements() {
156+
for _, err := range []error{stmt.Err, stmt.ParamErr} {
157+
if err == nil {
158+
continue
159+
}
160+
loc := stmt.Start
161+
if e, ok := err.(*sqlerr.Error); ok && e.Location != 0 {
162+
loc = e.Location
163+
}
164+
line, column := source.LineNumber(input, res.Origin(loc))
165+
out = append(out, fmt.Sprintf("%d:%d: %s", line, column, err))
166+
}
167+
}
168+
if len(out) == 0 {
169+
return ""
170+
}
171+
return strings.Join(out, "\n") + "\n"
172+
}
173+
48174
// cases returns every testdata directory that holds an input.sql, relative to
49175
// testdata and in a stable order.
50176
func cases(t *testing.T) []string {
@@ -74,32 +200,6 @@ func cases(t *testing.T) []string {
74200
return dirs
75201
}
76202

77-
// errors renders every validation error the preprocessor recorded, one per
78-
// line, as "line:column: message" against the original source.
79-
func errors(input string, res *preprocess.Result) string {
80-
var out []string
81-
for offset := 0; offset < len(res.Text); {
82-
stmt := res.Statement(offset)
83-
if stmt.End <= offset {
84-
break
85-
}
86-
offset = stmt.End
87-
if stmt.Err == nil {
88-
continue
89-
}
90-
loc := 0
91-
if e, ok := stmt.Err.(*sqlerr.Error); ok {
92-
loc = res.Origin(e.Location)
93-
}
94-
line, column := source.LineNumber(input, loc)
95-
out = append(out, fmt.Sprintf("%d:%d: %s", line, column, stmt.Err))
96-
}
97-
if len(out) == 0 {
98-
return ""
99-
}
100-
return strings.Join(out, "\n") + "\n"
101-
}
102-
103203
func readFile(t *testing.T, path string) string {
104204
t.Helper()
105205
blob, err := os.ReadFile(path)
@@ -133,132 +233,3 @@ func compare(t *testing.T, path, actual string) {
133233
t.Errorf("%s differed (-want +got):\n%s", filepath.Base(path), diff)
134234
}
135235
}
136-
137-
func mustRewrite(t *testing.T, engine config.Engine, src string) *preprocess.Result {
138-
t.Helper()
139-
res, err := preprocess.File(engine, src)
140-
if err != nil {
141-
t.Fatalf("preprocess.File(%s): %s", engine, err)
142-
}
143-
return res
144-
}
145-
146-
func TestParamSet(t *testing.T) {
147-
res := mustRewrite(t, config.EnginePostgreSQL,
148-
"SELECT * FROM users WHERE a = sqlc.arg(alpha) AND b = sqlc.narg(beta) AND c = ANY(sqlc.slice(gamma));")
149-
stmt := res.Statement(0)
150-
151-
for num, want := range map[int]string{1: "alpha", 2: "beta", 3: "gamma"} {
152-
got, ok := stmt.Params.NameFor(num)
153-
if !ok {
154-
t.Fatalf("no name for parameter %d", num)
155-
}
156-
if got != want {
157-
t.Errorf("parameter %d: got %q, want %q", num, got, want)
158-
}
159-
}
160-
161-
// sqlc.narg() marks the parameter nullable even when inference says
162-
// otherwise.
163-
beta, _ := stmt.Params.FetchMerge(2, named.NewInferredParam("beta", true))
164-
if beta.NotNull() {
165-
t.Error("sqlc.narg parameter should be nullable")
166-
}
167-
gamma, _ := stmt.Params.FetchMerge(3, named.NewParam("gamma"))
168-
if !gamma.IsSqlcSlice() {
169-
t.Error("sqlc.slice parameter should be marked as a slice")
170-
}
171-
}
172-
173-
func TestEmbedSpans(t *testing.T) {
174-
src := "SELECT sqlc.embed(a), b.* FROM a, b;"
175-
res := mustRewrite(t, config.EnginePostgreSQL, src)
176-
stmt := res.Statement(0)
177-
if len(stmt.Embeds) != 1 {
178-
t.Fatalf("expected 1 embed, got %d", len(stmt.Embeds))
179-
}
180-
e := stmt.Embeds[0]
181-
if e.Table.Name != "a" {
182-
t.Errorf("embed table: got %q, want %q", e.Table.Name, "a")
183-
}
184-
if want := strings.Index(res.Text, "a.*"); e.Location != want {
185-
t.Errorf("embed location: got %d, want %d", e.Location, want)
186-
}
187-
// A star reference the user wrote must not be mistaken for an embed.
188-
if _, ok := stmt.Embeds.Find(strings.Index(res.Text, "b.*")); ok {
189-
t.Error("user-written b.* was reported as an embed")
190-
}
191-
if got, want := e.Orig(), "sqlc.embed(a)"; got != want {
192-
t.Errorf("Orig: got %q, want %q", got, want)
193-
}
194-
}
195-
196-
func TestSliceSpans(t *testing.T) {
197-
src := "SELECT * FROM t WHERE a IN (sqlc.slice(ids)) AND b = sqlc.arg(x);"
198-
res := mustRewrite(t, config.EngineMySQL, src)
199-
stmt := res.Statement(0)
200-
if len(stmt.Slices) != 1 {
201-
t.Fatalf("expected 1 slice, got %d", len(stmt.Slices))
202-
}
203-
// The recorded offset points at the placeholder itself, past the marker,
204-
// which is where the engine reports the parameter node.
205-
want := strings.Index(res.Text, "*/?") + len("*/")
206-
if got, ok := stmt.Slices[want]; !ok {
207-
t.Errorf("slice offsets %v do not contain %d", stmt.Slices, want)
208-
} else if got != "ids" {
209-
t.Errorf("slice name: got %q, want %q", got, "ids")
210-
}
211-
}
212-
213-
func TestOriginMapping(t *testing.T) {
214-
src := "SELECT * FROM t WHERE a = sqlc.arg(alpha) AND b = sqlc.arg(beta);"
215-
res := mustRewrite(t, config.EnginePostgreSQL, src)
216-
217-
// Text before the first rewrite maps to itself.
218-
if got := res.Origin(3); got != 3 {
219-
t.Errorf("Origin(3): got %d, want 3", got)
220-
}
221-
// Text after a rewrite maps back across the length change.
222-
newB, oldB := strings.Index(res.Text, "b ="), strings.Index(src, "b =")
223-
if got := res.Origin(newB); got != oldB {
224-
t.Errorf("Origin(%d): got %d, want %d", newB, got, oldB)
225-
}
226-
// An offset inside a placeholder maps to the start of what it replaced.
227-
newParam := strings.Index(res.Text, "$2")
228-
oldParam := strings.Index(src, "sqlc.arg(beta)")
229-
if got := res.Origin(newParam); got != oldParam {
230-
t.Errorf("Origin(%d): got %d, want %d", newParam, got, oldParam)
231-
}
232-
}
233-
234-
func TestLinesArePreserved(t *testing.T) {
235-
// Downstream error reporting counts lines in the rewritten text, so a
236-
// rewrite must never add or remove one.
237-
src := "-- name: Get :one\nSELECT *\nFROM users\nWHERE id = sqlc.arg(id)\n AND name = @name;\n"
238-
res := mustRewrite(t, config.EnginePostgreSQL, src)
239-
if want, got := strings.Count(src, "\n"), strings.Count(res.Text, "\n"); want != got {
240-
t.Errorf("line count changed: got %d, want %d", got, want)
241-
}
242-
}
243-
244-
func TestStatementLookup(t *testing.T) {
245-
src := "SELECT sqlc.arg(a);\nSELECT sqlc.arg(b), sqlc.arg(c);\n"
246-
res := mustRewrite(t, config.EnginePostgreSQL, src)
247-
first := res.Statement(0)
248-
second := res.Statement(strings.Index(res.Text, "$1, $2"))
249-
if first == second {
250-
t.Fatal("expected two distinct statements")
251-
}
252-
if name, _ := first.Params.NameFor(1); name != "a" {
253-
t.Errorf("first statement parameter 1: got %q, want %q", name, "a")
254-
}
255-
if name, _ := second.Params.NameFor(2); name != "c" {
256-
t.Errorf("second statement parameter 2: got %q, want %q", name, "c")
257-
}
258-
}
259-
260-
func TestUnknownEngine(t *testing.T) {
261-
if _, err := preprocess.File(config.Engine("nope"), "SELECT 1;"); err == nil {
262-
t.Fatal("expected an error for an unknown engine")
263-
}
264-
}

0 commit comments

Comments
 (0)