Skip to content

Commit 2599966

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 dd37cbc commit 2599966

17 files changed

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

‎internal/goldeneye/README.md‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,29 @@ the hand-written files alone, and the checks do not look at them.
6060
- `dialect/` — the record types the files are made of, mirrored from
6161
`internal/core/seed`, and the helpers that write a generated set of files
6262
into an engine directory or diff it against what is committed.
63+
- `endtoend/` — finds the analyze cases and compares an engine's answer with
64+
a case's committed output.
6365
- `postgresql/`, `duckdb/`, `clickhouse/` — one package per engine, each
64-
exposing `Locate`, `Version` and `Generate`, and a test that runs the check.
66+
exposing `Locate`, `Version` and `Generate`, `Analyze` where the engine has
67+
an analysis check, and tests that run the checks.
6568
- `cmd/goldeneye/` — the command.
6669

67-
The analysis checks — verifying the `analyze_*` cases under
68-
`internal/endtoend/testdata` against what each database itself reports — are
69-
meant to live here too, alongside the dialect checks.
70+
## Analysis checks
71+
72+
`check` also verifies the `analyze_*` cases under `internal/endtoend/testdata`
73+
against what the database itself reports. A case is an
74+
`analyze_<name>/<engine>` directory whose `exec.json` runs the analyze
75+
command; `endtoend/` finds them. The engine package loads the case's
76+
`schema.sql` and optional `fixture.sql` into the database, runs `query.sql`
77+
there, prints what the database reports in the JSON shape `sqlc analyze`
78+
prints, and compares it with the committed `output.json` byte for byte. A
79+
difference means sqlc's analysis disagrees with the database. A case that
80+
asks for `--ast` is skipped, since only sqlc can print that.
81+
82+
- **`clickhouse`** runs each case in an ephemeral `clickhouse local` process.
83+
Column types come from the executed query's result header, provenance from
84+
`EXPLAIN QUERY TREE`, and parameters from sentinel constants substituted for
85+
`?`, `sqlc.arg()` and `sqlc.narg()`, since ClickHouse itself never sees a
86+
placeholder; `INSERT ... VALUES` parameters map onto `DESCRIBE TABLE`.
87+
88+
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: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
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 generate [engine] # rewrite the generated files from the database
9-
// go run ./cmd/goldeneye check [engine] # compare the committed files with the database
10+
// go run ./cmd/goldeneye check [engine] # compare the committed files and analyze cases with the database
1011
//
1112
// Without an engine, generate and check cover every engine whose database
1213
// is available and say which ones they skipped. `go test ./...` runs the
@@ -21,10 +22,12 @@ import (
2122
"io"
2223
"os"
2324
"runtime"
25+
"strings"
2426

2527
"github.com/sqlc-dev/sqlc/internal/goldeneye/clickhouse"
2628
"github.com/sqlc-dev/sqlc/internal/goldeneye/dialect"
2729
"github.com/sqlc-dev/sqlc/internal/goldeneye/duckdb"
30+
"github.com/sqlc-dev/sqlc/internal/goldeneye/endtoend"
2831
"github.com/sqlc-dev/sqlc/internal/goldeneye/postgresql"
2932
)
3033

@@ -41,7 +44,7 @@ const usage = `usage:
4144
goldeneye generate [engine]
4245
rewrite the generated dialect files from the database, for every available engine or one
4346
goldeneye check [engine]
44-
compare the committed dialect files with the database, for every available engine or one
47+
compare the committed dialect files and analyze cases with the database, for every available engine or one
4548
4649
engines: clickhouse, duckdb, postgresql`
4750

@@ -55,12 +58,15 @@ type engine struct {
5558
version func(context.Context, string) (string, error)
5659
// generate reads the dialect from the database.
5760
generate func(context.Context, string) (dialect.Files, error)
61+
// analyze asks the database what it reports for an analyze case, in
62+
// the shape sqlc analyze prints. Nil for an engine without one yet.
63+
analyze func(context.Context, string, endtoend.Case) ([]byte, error)
5864
}
5965

6066
var engines = []engine{
61-
{clickhouse.Engine, clickhouse.Locate, clickhouse.Version, clickhouse.Generate},
62-
{duckdb.Engine, duckdb.Locate, duckdb.Version, duckdb.Generate},
63-
{postgresql.Engine, postgresql.Locate, postgresql.Version, postgresql.Generate},
67+
{clickhouse.Engine, clickhouse.Locate, clickhouse.Version, clickhouse.Generate, clickhouse.Analyze},
68+
{duckdb.Engine, duckdb.Locate, duckdb.Version, duckdb.Generate, nil},
69+
{postgresql.Engine, postgresql.Locate, postgresql.Version, postgresql.Generate, nil},
6470
}
6571

6672
func run(ctx context.Context, args []string, stdout, stderr io.Writer) error {
@@ -184,5 +190,37 @@ func check(ctx context.Context, e engine, handle string, stderr io.Writer) error
184190
return fmt.Errorf("%s does not match the database\n%s", dir, report)
185191
}
186192
fmt.Fprintf(stderr, "%s: ok, %d file(s) match\n", e.name, len(files))
193+
return checkAnalyzeCases(ctx, e, handle, stderr)
194+
}
195+
196+
// checkAnalyzeCases compares what the database reports for each of the
197+
// engine's analyze cases with the case's committed output.
198+
func checkAnalyzeCases(ctx context.Context, e engine, handle string, stderr io.Writer) error {
199+
if e.analyze == nil {
200+
return nil
201+
}
202+
cases, err := endtoend.Cases(e.name)
203+
if err != nil {
204+
return err
205+
}
206+
var report strings.Builder
207+
for _, c := range cases {
208+
got, err := e.analyze(ctx, handle, c)
209+
if err != nil {
210+
fmt.Fprintf(&report, "%s: %v\n", c.Name, err)
211+
continue
212+
}
213+
diff, err := c.Compare(got)
214+
if err != nil {
215+
return err
216+
}
217+
if diff != "" {
218+
fmt.Fprintf(&report, "%s (-committed +database)\n%s", c.Name, diff)
219+
}
220+
}
221+
if report.Len() > 0 {
222+
return fmt.Errorf("analyze cases do not match the database\n%s", report.String())
223+
}
224+
fmt.Fprintf(stderr, "%s: ok, %d analyze case(s) match\n", e.name, len(cases))
187225
return nil
188226
}

0 commit comments

Comments
 (0)