Skip to content

Commit df6c041

Browse files
committed
Move the analyze checks into goldeneye
goldeneye already generates and checks the dialect seeds and reserved a place for the analysis checks, so the testcheck module folds into it: the case loader becomes goldeneye/endtoend, reusing goldeneye's diff, the ClickHouse analysis joins goldeneye's clickhouse package next to the dialect generator it shares a binary with, and `check` verifies both the committed dialect and the analyze cases. `go test ./...` in internal/goldeneye runs the same checks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6XkyWnx7iJFEb8q3AnYps
1 parent 47cce48 commit df6c041

17 files changed

Lines changed: 123 additions & 577 deletions

File tree

‎CLAUDE.md‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,11 @@ the run early. Run a subset to get past one (`-run 'TestReplay/core/^select'`).
147147

148148
The dialect seeds under `/internal/engine/<engine>/dialect/` are generated
149149
from a live database by `/internal/goldeneye`, a nested module, and its tests
150-
verify the committed files against one byte for byte. Engines whose database
151-
is not available skip.
150+
verify the committed files against one byte for byte. The same module checks
151+
the `analyze_*` cases under `/internal/endtoend/testdata/` against what the
152+
database itself reports for them, so a `fixture.sql` next to a case's schema
153+
gives the queries rows to run against. Engines whose database is not
154+
available skip.
152155

153156
```bash
154157
cd internal/goldeneye
@@ -240,16 +243,15 @@ MYSQL_SERVER_URI="root:mysecretpassword@tcp(127.0.0.1:3306)/mysql?multiStatement
240243
JSONL read by `/internal/core/seed`; the generated parts come from
241244
`/internal/goldeneye`
242245
- `/internal/goldeneye/` - Nested module that generates the dialect seeds
243-
under `/internal/engine/<engine>/dialect/` from a live database and checks
244-
the committed ones against it, one package per engine; see its README
246+
under `/internal/engine/<engine>/dialect/` from a live database, checks
247+
the committed ones against it, and checks the analyze cases under
248+
`/internal/endtoend/testdata/` against what the database reports, one
249+
package per engine; see its README
245250
- `/internal/core/` - The analysis core: catalog, analyzer and dialect seeds
246251
- `/internal/compiler/` - Query compilation logic
247252
- `/internal/codegen/` - Code generation for different languages
248253
- `/internal/config/` - Configuration file parsing
249254
- `/internal/endtoend/` - End-to-end tests
250-
- `/internal/testcheck/` - Nested module that verifies the analyze cases under
251-
`/internal/endtoend/testdata/` against a real database, one package per
252-
engine; see its README
253255
- `/internal/sqltest/` - Test database setup (Docker, native, local detection)
254256
- `/examples/` - Example projects for testing
255257

‎internal/goldeneye/README.md‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,29 @@ the hand-written files alone, and the checks do not look at them.
8787
- `dialect/` — the record types the files are made of, mirrored from
8888
`internal/core/seed`, and the helpers that write a generated set of files
8989
into an engine directory or diff it against what is committed.
90+
- `endtoend/` — finds the analyze cases and compares an engine's answer with
91+
a case's committed output.
9092
- `postgresql/`, `duckdb/`, `clickhouse/`, `sqlite/` — one package per
91-
engine, each exposing `Locate`, `Version` and `Generate`, and a test that
92-
runs the check.
93+
engine, each exposing `Locate`, `Version` and `Generate`, `Analyze` where
94+
the engine has an analysis check, and tests that run the checks.
9395
- `cmd/goldeneye/` — the command.
9496

95-
The analysis checks — verifying the `analyze_*` cases under
96-
`internal/endtoend/testdata` against what each database itself reports — are
97-
meant to live here too, alongside the dialect checks.
97+
## Analysis checks
98+
99+
`check` also verifies the `analyze_*` cases under `internal/endtoend/testdata`
100+
against what the database itself reports. A case is an
101+
`analyze_<name>/<engine>` directory whose `exec.json` runs the analyze
102+
command; `endtoend/` finds them. The engine package loads the case's
103+
`schema.sql` and optional `fixture.sql` into the database, runs `query.sql`
104+
there, prints what the database reports in the JSON shape `sqlc analyze`
105+
prints, and compares it with the committed `output.json` byte for byte. A
106+
difference means sqlc's analysis disagrees with the database. A case that
107+
asks for `--ast` is skipped, since only sqlc can print that.
108+
109+
- **`clickhouse`** runs each case in an ephemeral `clickhouse local` process.
110+
Column types come from the executed query's result header, provenance from
111+
`EXPLAIN QUERY TREE`, and parameters from sentinel constants substituted for
112+
`?`, `sqlc.arg()` and `sqlc.narg()`, since ClickHouse itself never sees a
113+
placeholder; `INSERT ... VALUES` parameters map onto `DESCRIBE TABLE`.
114+
115+
The other engines have no analysis check yet.
Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,3 @@
1-
// Package clickhouse verifies the ClickHouse analyze cases under
2-
// internal/endtoend/testdata against what ClickHouse itself reports.
3-
//
4-
// Each case's schema and fixture are loaded into an ephemeral
5-
// `clickhouse local` process and its queries are run there. Result column
6-
// types come from the executed query's result header, provenance from
7-
// EXPLAIN QUERY TREE, and parameters from sentinel constants substituted
8-
// for the placeholders, since ClickHouse itself never sees a ?. The answer
9-
// is printed in the JSON shape sqlc analyze prints and compared with the
10-
// case's committed output.json byte for byte.
111
package clickhouse
122

133
import (
@@ -17,12 +7,9 @@ import (
177
"fmt"
188
"os"
199

20-
"github.com/sqlc-dev/sqlc/internal/testcheck/endtoend"
10+
"github.com/sqlc-dev/sqlc/internal/goldeneye/endtoend"
2111
)
2212

23-
// Engine is the name of this engine's directory under each analyze case.
24-
const Engine = "clickhouse"
25-
2613
// Analyze runs a case's queries through the clickhouse binary and returns
2714
// the analysis in the JSON shape sqlc analyze prints.
2815
func Analyze(ctx context.Context, binary string, c endtoend.Case) ([]byte, error) {

‎internal/goldeneye/clickhouse/clickhouse.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@
99
// system.functions carries no signatures — so functions.jsonl is written by
1010
// hand and is not this package's business.
1111
//
12+
// The package also verifies the ClickHouse analyze cases under
13+
// internal/endtoend/testdata against the same binary: each case's schema and
14+
// fixture are loaded into a `clickhouse local` process and its queries run
15+
// there. Result column types come from the executed query's result header,
16+
// provenance from EXPLAIN QUERY TREE, and parameters from sentinel constants
17+
// substituted for the placeholders, since ClickHouse itself never sees a ?.
18+
// The answer is printed in the JSON shape sqlc analyze prints and compared
19+
// with the case's committed output.json byte for byte.
20+
//
1221
// The binary is downloaded once per pinned version by Install, or supplied
1322
// through the CLICKHOUSE environment variable.
1423
package clickhouse

‎internal/goldeneye/clickhouse/clickhouse_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/sqlc-dev/sqlc/internal/goldeneye/dialect"
8+
"github.com/sqlc-dev/sqlc/internal/goldeneye/endtoend"
89
)
910

1011
// TestDialect verifies the committed ClickHouse dialect against what the
@@ -35,3 +36,31 @@ func TestDialect(t *testing.T) {
3536
t.Errorf("%s does not match what %s reports:\n%s", dir, version, report)
3637
}
3738
}
39+
40+
// TestAnalyzeCases verifies every ClickHouse analyze case under
41+
// internal/endtoend/testdata against what ClickHouse reports. It skips
42+
// unless the binary is installed.
43+
func TestAnalyzeCases(t *testing.T) {
44+
binary, err := Locate()
45+
if err != nil {
46+
t.Skip(err)
47+
}
48+
cases, err := endtoend.Cases(Engine)
49+
if err != nil {
50+
t.Fatal(err)
51+
}
52+
if len(cases) == 0 {
53+
t.Fatal("no clickhouse analyze cases found")
54+
}
55+
for _, c := range cases {
56+
t.Run(c.Name, func(t *testing.T) {
57+
diff, err := Check(context.Background(), binary, c)
58+
if err != nil {
59+
t.Fatal(err)
60+
}
61+
if diff != "" {
62+
t.Errorf("%s does not match what ClickHouse reports (-committed +clickhouse):\n%s", c.Output, diff)
63+
}
64+
})
65+
}
66+
}

‎internal/goldeneye/cmd/goldeneye/main.go‎

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
// Command goldeneye generates the dialect seeds under
2-
// internal/engine/<engine>/dialect from a live database, and checks the
3-
// committed ones against it.
2+
// internal/engine/<engine>/dialect from a live database, checks the
3+
// committed ones against it, and checks the analyze cases under
4+
// internal/endtoend/testdata against what the database itself reports.
45
//
56
// Usage, from internal/goldeneye:
67
//
78
// go run ./cmd/goldeneye install clickhouse # download the pinned clickhouse binary
89
// go run ./cmd/goldeneye install sqlite # build the pinned sqlite3 shells from source
910
// go run ./cmd/goldeneye generate [engine] # rewrite the generated files from the database
10-
// go run ./cmd/goldeneye check [engine] # compare the committed files with the database
11+
// go run ./cmd/goldeneye check [engine] # compare the committed files and analyze cases with the database
1112
//
1213
// Without an engine, generate and check cover every engine whose database
1314
// is available and say which ones they skipped. `go test ./...` runs the
@@ -22,10 +23,12 @@ import (
2223
"io"
2324
"os"
2425
"runtime"
26+
"strings"
2527

2628
"github.com/sqlc-dev/sqlc/internal/goldeneye/clickhouse"
2729
"github.com/sqlc-dev/sqlc/internal/goldeneye/dialect"
2830
"github.com/sqlc-dev/sqlc/internal/goldeneye/duckdb"
31+
"github.com/sqlc-dev/sqlc/internal/goldeneye/endtoend"
2932
"github.com/sqlc-dev/sqlc/internal/goldeneye/postgresql"
3033
"github.com/sqlc-dev/sqlc/internal/goldeneye/sqlite"
3134
)
@@ -44,7 +47,7 @@ const usage = `usage:
4447
goldeneye generate [engine]
4548
rewrite the generated dialect files from the database, for every available engine or one
4649
goldeneye check [engine]
47-
compare the committed dialect files with the database, for every available engine or one
50+
compare the committed dialect files and analyze cases with the database, for every available engine or one
4851
4952
engines: clickhouse, duckdb, postgresql, sqlite`
5053

@@ -58,13 +61,16 @@ type engine struct {
5861
version func(context.Context, string) (string, error)
5962
// generate reads the dialect from the database.
6063
generate func(context.Context, string) (dialect.Files, error)
64+
// analyze asks the database what it reports for an analyze case, in
65+
// the shape sqlc analyze prints. Nil for an engine without one yet.
66+
analyze func(context.Context, string, endtoend.Case) ([]byte, error)
6167
}
6268

6369
var engines = []engine{
64-
{clickhouse.Engine, clickhouse.Locate, clickhouse.Version, clickhouse.Generate},
65-
{duckdb.Engine, duckdb.Locate, duckdb.Version, duckdb.Generate},
66-
{postgresql.Engine, postgresql.Locate, postgresql.Version, postgresql.Generate},
67-
{sqlite.Engine, sqlite.Locate, sqlite.Version, sqlite.Generate},
70+
{clickhouse.Engine, clickhouse.Locate, clickhouse.Version, clickhouse.Generate, clickhouse.Analyze},
71+
{duckdb.Engine, duckdb.Locate, duckdb.Version, duckdb.Generate, nil},
72+
{postgresql.Engine, postgresql.Locate, postgresql.Version, postgresql.Generate, nil},
73+
{sqlite.Engine, sqlite.Locate, sqlite.Version, sqlite.Generate, nil},
6874
}
6975

7076
// installer puts the binary an engine is read through in place, for the
@@ -204,5 +210,37 @@ func check(ctx context.Context, e engine, handle string, stderr io.Writer) error
204210
return fmt.Errorf("%s does not match the database\n%s", dir, report)
205211
}
206212
fmt.Fprintf(stderr, "%s: ok, %d file(s) match\n", e.name, len(files))
213+
return checkAnalyzeCases(ctx, e, handle, stderr)
214+
}
215+
216+
// checkAnalyzeCases compares what the database reports for each of the
217+
// engine's analyze cases with the case's committed output.
218+
func checkAnalyzeCases(ctx context.Context, e engine, handle string, stderr io.Writer) error {
219+
if e.analyze == nil {
220+
return nil
221+
}
222+
cases, err := endtoend.Cases(e.name)
223+
if err != nil {
224+
return err
225+
}
226+
var report strings.Builder
227+
for _, c := range cases {
228+
got, err := e.analyze(ctx, handle, c)
229+
if err != nil {
230+
fmt.Fprintf(&report, "%s: %v\n", c.Name, err)
231+
continue
232+
}
233+
diff, err := c.Compare(got)
234+
if err != nil {
235+
return err
236+
}
237+
if diff != "" {
238+
fmt.Fprintf(&report, "%s (-committed +database)\n%s", c.Name, diff)
239+
}
240+
}
241+
if report.Len() > 0 {
242+
return fmt.Errorf("analyze cases do not match the database\n%s", report.String())
243+
}
244+
fmt.Fprintf(stderr, "%s: ok, %d analyze case(s) match\n", e.name, len(cases))
207245
return nil
208246
}

0 commit comments

Comments
 (0)