Check analyze cases against a real ClickHouse in goldeneye, and make sqlc agree with it (#4603)

Author: kyleconroy

CLAUDE.md

@@ -120,8 +120,8 @@ A case is a directory holding the inputs and the expected output. `exec.json`
 names the command and its arguments — omit it and the case runs `generate`,
 comparing the generated files against the ones committed alongside; give it
 `{"command": "analyze", "args": [...]}` and the case compares the command's
-stdout against `stdout.txt`. A case that is expected to fail commits its
-`stderr.txt`. Regenerate a golden by running the command in its directory and
+stdout against `stdout.json` (or `stdout.txt` for a command that does not
+print JSON). A case that is expected to fail commits its `stderr.txt`. Regenerate a golden by running the command in its directory and
 writing the output back over the committed file.
 
 `TestReplay` runs the whole corpus once per *context*. `base` runs each case as
@@ -147,8 +147,11 @@ the run early. Run a subset to get past one (`-run 'TestReplay/core/^select'`).
 
 The dialect seeds under `/internal/engine/<engine>/dialect/` are generated
 from a live database by `/internal/goldeneye`, a nested module, and its tests
-verify the committed files against one byte for byte. Engines whose database
-is not available skip.
+verify the committed files against one byte for byte. The same module checks
+the `analyze_*` cases under `/internal/endtoend/testdata/` against what the
+database itself reports for them, so a `fixture.sql` next to a case's schema
+gives the queries rows to run against. Engines whose database is not
+available skip.
 
 ```bash
 cd internal/goldeneye
@@ -240,8 +243,10 @@ MYSQL_SERVER_URI="root:mysecretpassword@tcp(127.0.0.1:3306)/mysql?multiStatement
     JSONL read by `/internal/core/seed`; the generated parts come from
     `/internal/goldeneye`
 - `/internal/goldeneye/` - Nested module that generates the dialect seeds
-  under `/internal/engine/<engine>/dialect/` from a live database and checks
-  the committed ones against it, one package per engine; see its README
+  under `/internal/engine/<engine>/dialect/` from a live database, checks
+  the committed ones against it, and checks the analyze cases under
+  `/internal/endtoend/testdata/` against what the database reports, one
+  package per engine; see its README
 - `/internal/core/` - The analysis core: catalog, analyzer and dialect seeds
 - `/internal/compiler/` - Query compilation logic
 - `/internal/codegen/` - Code generation for different languages

docs/howto/analyze.md

@@ -70,23 +70,24 @@ reports the result columns and parameters:
     "columns": [
       {
         "name": "id",
-        "data_type": "bigserial",
-        "not_null": true,
-        "is_array": false,
+        "type": {
+          "name": "bigserial"
+        },
         "table": "authors"
       },
       {
         "name": "name",
-        "data_type": "text",
-        "not_null": true,
-        "is_array": false,
+        "type": {
+          "name": "text"
+        },
         "table": "authors"
       },
       {
         "name": "bio",
-        "data_type": "text",
-        "not_null": false,
-        "is_array": false,
+        "type": {
+          "name": "text",
+          "nullable": true
+        },
         "table": "authors"
       }
     ],
@@ -95,9 +96,9 @@ reports the result columns and parameters:
         "number": 1,
         "column": {
           "name": "id",
-          "data_type": "bigserial",
-          "not_null": true,
-          "is_array": false,
+          "type": {
+            "name": "bigserial"
+          },
           "table": "authors"
         }
       }
@@ -106,6 +107,13 @@ reports the result columns and parameters:
 ]
 ```
 
+A column's `type` is written as a call expression: a `name` applied to
+`args`, each of which carries an optional `label` and exactly one of `type`,
+`int`, `bool` or `string`, with `nullable` set at whatever depth it applies.
+An array of text is `array` applied to `text`; a `Map(String, Nullable(UInt8))`
+in ClickHouse is `map` applied to `string` and a nullable `uint8`. Names are
+recorded as the engine reports them.
+
 Pass `--ast` to also include each statement's parsed AST under an `ast` key. It
 has the same shape as the output of [`parse`](parse.md), with every node tagged
 by type.

internal/cmd/analyze.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/sqlc-dev/sqlc/internal/compiler"
 	"github.com/sqlc-dev/sqlc/internal/config"
+	"github.com/sqlc-dev/sqlc/internal/core"
 	"github.com/sqlc-dev/sqlc/internal/multierr"
 	"github.com/sqlc-dev/sqlc/internal/opts"
 	"github.com/sqlc-dev/sqlc/internal/sql/ast"
@@ -204,11 +205,9 @@ type analyzedQuery struct {
 }
 
 type analyzedColumn struct {
-	Name     string `json:"name"`
-	DataType string `json:"data_type"`
-	NotNull  bool   `json:"not_null"`
-	IsArray  bool   `json:"is_array"`
-	Table    string `json:"table,omitempty"`
+	Name  string         `json:"name"`
+	Type  *core.TypeExpr `json:"type,omitempty"`
+	Table string         `json:"table,omitempty"`
 }
 
 type analyzedParam struct {
@@ -243,13 +242,34 @@ func newAnalyzedColumn(col *compiler.Column) analyzedColumn {
 		return analyzedColumn{}
 	}
 	ac := analyzedColumn{
-		Name:     col.Name,
-		DataType: col.DataType,
-		NotNull:  col.NotNull,
-		IsArray:  col.IsArray,
+		Name: col.Name,
+		Type: newAnalyzedType(col),
 	}
 	if col.Table != nil {
 		ac.Table = col.Table.Name
 	}
 	return ac
 }
+
+// newAnalyzedType is the column's type as an expression: the one the
+// analysis core wrote when it did, otherwise the flat description the
+// compiler holds, which is the data type wrapped in one array node per
+// dimension with the column's nullability on the outermost node.
+func newAnalyzedType(col *compiler.Column) *core.TypeExpr {
+	if col.TypeExpr != nil {
+		return col.TypeExpr
+	}
+	if col.DataType == "" {
+		return nil
+	}
+	t := core.ParseTypeExpr(col.DataType)
+	dims := col.ArrayDims
+	if col.IsArray && dims == 0 {
+		dims = 1
+	}
+	for i := 0; i < dims; i++ {
+		t = &core.TypeExpr{Name: "array", Args: []core.TypeArg{{Type: t}}}
+	}
+	t.Nullable = !col.NotNull
+	return t
+}

internal/compiler/parse_core.go

@@ -105,6 +105,7 @@ func coreColumn(c core.Column) *Column {
 		DataType: c.DataType,
 		NotNull:  c.NotNull,
 		IsArray:  c.IsArray,
+		TypeExpr: c.Type,
 	}
 	// The core reports arrays without dimensions, and codegen renders one
 	// "[]" per dimension.
@@ -129,6 +130,7 @@ func coreParamColumn(p core.Parameter, params *named.ParamSet) *Column {
 		DataType: p.DataType,
 		NotNull:  p.NotNull,
 		IsArray:  p.IsArray,
+		TypeExpr: p.Type,
 	}
 	if p.IsArray {
 		col.ArrayDims = 1

internal/compiler/query.go

@@ -1,6 +1,7 @@
 package compiler
 
 import (
+	"github.com/sqlc-dev/sqlc/internal/core"
 	"github.com/sqlc-dev/sqlc/internal/metadata"
 	"github.com/sqlc-dev/sqlc/internal/sql/ast"
 	"github.com/sqlc-dev/sqlc/internal/sql/catalog"
@@ -37,6 +38,11 @@ type Column struct {
 	Type       *ast.TypeName
 	EmbedTable *ast.TableName
 
+	// TypeExpr is the type as the analysis core wrote it, with the
+	// arguments and nesting DataType and IsArray flatten away. It is unset
+	// on the legacy path.
+	TypeExpr *core.TypeExpr
+
 	IsSqlcSlice bool // is this sqlc.slice()
 
 	skipTableRequiredCheck bool

internal/core/analysis.go

@@ -53,8 +53,11 @@ type ColumnSource struct {
 }
 
 type Column struct {
-	Name               string        `json:"name"`
-	DataType           string        `json:"data_type"`
+	Name     string `json:"name"`
+	DataType string `json:"data_type"`
+	// Type is the column's type as an expression, carrying what DataType
+	// and IsArray flatten away: arguments, nesting and inner nullability.
+	Type               *TypeExpr     `json:"type,omitempty"`
 	TypeOID            int64         `json:"type_oid,omitempty"`
 	NotNull            bool          `json:"not_null"`
 	IsArray            bool          `json:"is_array,omitempty"`
@@ -73,6 +76,7 @@ type Parameter struct {
 	Number   int           `json:"number"`
 	Name     string        `json:"name,omitempty"`
 	DataType string        `json:"data_type,omitempty"`
+	Type     *TypeExpr     `json:"type,omitempty"`
 	TypeOID  int64         `json:"type_oid,omitempty"`
 	NotNull  bool          `json:"not_null"`
 	IsArray  bool          `json:"is_array,omitempty"`

internal/core/analyzer/analyzer.go

@@ -119,6 +119,20 @@ func derivedRel(alias string, cols []core.Column) scopeRel {
 }
 
 func (a *analyzer) result() core.PrepareResult {
+	// A placeholder nothing constrained takes the dialect's type for one, when
+	// the dialect has such a type.
+	if oid, ok := a.cat.UntypedTypeOID(); ok {
+		for n, p := range a.params {
+			if p.TypeOID == 0 && p.DataType == "" {
+				t := exprType{typeOID: oid, nullable: true}
+				p.TypeOID = oid
+				p.DataType, p.IsArray = a.typeNameOf(t)
+				p.NotNull = false
+				p.Type = a.typeExprOf(t, "")
+				a.params[n] = p
+			}
+		}
+	}
 	res := core.PrepareResult{
 		Command:    a.command,
 		Columns:    a.columns,
@@ -213,9 +227,39 @@ func (a *analyzer) analyzeSelect(s *ast.SelectStmt) error {
 			return err
 		}
 	}
+	for _, item := range listItems(s.SortClause) {
+		if sb, ok := item.(*ast.SortBy); ok {
+			if _, err := a.typeExpr(sb.Node); err != nil {
+				return fmt.Errorf("order by: %w", err)
+			}
+		}
+	}
+	for _, n := range []ast.Node{s.LimitCount, s.LimitOffset} {
+		if err := a.typeLimit(n); err != nil {
+			return fmt.Errorf("limit: %w", err)
+		}
+	}
 	return nil
 }
 
+// typeLimit types a LIMIT or OFFSET count. A bare placeholder there holds
+// whatever the dialect counts rows in.
+func (a *analyzer) typeLimit(n ast.Node) error {
+	if n == nil {
+		return nil
+	}
+	if pr, ok := n.(*ast.ParamRef); ok {
+		oid, err := a.cat.LimitTypeOID()
+		if err != nil {
+			return err
+		}
+		a.inferParam(pr.Number, exprType{typeOID: oid})
+		return nil
+	}
+	_, err := a.typeExpr(n)
+	return err
+}
+
 func (a *analyzer) typeValuesLists(l *ast.List) error {
 	for _, row := range listItems(l) {
 		if _, err := a.typeExpr(row); err != nil {