Skip to content

Commit 33ee062

Browse files
committed
core: answer the second review: qualified keys, nested lookups, rewrites after aliases
An instance row's key is built from namespace-qualified names, so array(myschema.mood) and array(mood) are two rows, and a lookup that may not write still canonicalizes an expression whose nested instance is not a row, so $1::varchar(10)[] reports array(character varying(10)). The array family is seeded for every dialect rather than created on a read-only lookup. A rewrite is tried with the alias resolved, so dec(7,2) meets a rule on decimal. A bare CREATE TYPE lands in the dialect's default schema, so SQL Server's Code and dbo.Code are one row. A family in types.jsonl may name a base, which is how MySQL's unsigned families stand on their signed ones; the legacy bridge strips " unsigned" from the data type and sets the flag. Relations() carries one array dimension for an array column. Converters: a PostgreSQL cast to interval day to second decodes the field mask as a column does; a MySQL cast to float or double carries no modifier and a decimal cast's scale left out is 0; DuckDB's main schema no longer leaves a leading dot; ClickHouse's LowCardinality(Nullable(T)) is a nullable column, read as a nullable lowcardinality(T) by the converter and by goldeneye alike. goldeneye keeps the case of a MySQL enum's members and unescapes them, and relabels only the _-prefixed PostgreSQL array types, so int2vector and oidvector stay their own; the relation seeds and the goldens they feed are regenerated. The analyze_types cases cover each fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qryf7a1doPFuCz2eGT3bCr
1 parent c70c5d6 commit 33ee062

32 files changed

Lines changed: 680 additions & 199 deletions

File tree

‎internal/compiler/catalog_core.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func coreResultCatalog(c *core.Catalog) (*catalog.Catalog, error) {
4242
inner := expr.Innermost()
4343
column := &catalog.Column{
4444
Name: col.Name,
45-
Type: ast.TypeName{Name: inner.Name},
45+
Type: ast.TypeName{Name: strings.TrimSuffix(inner.Name, " unsigned")},
4646
IsNotNull: col.NotNull,
4747
IsArray: expr.IsArray(),
4848
ArrayDims: expr.ArrayDims(),

‎internal/core/analyzer/expr.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,10 @@ func (a *analyzer) typeNameOf(t exprType) (string, bool) {
575575
if e == nil {
576576
return "", false
577577
}
578-
return e.Innermost().Name, e.IsArray()
578+
// MySQL's unsigned families are their own types, but codegen reads
579+
// the signed family and an unsigned flag, which the bridge derives
580+
// from the expression.
581+
return strings.TrimSuffix(e.Innermost().Name, " unsigned"), e.IsArray()
579582
}
580583

581584
// columnExprType is the type a result column of a nested query has, as an

‎internal/core/rewrite.go‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,13 +213,33 @@ func (c *Catalog) userTypeBase(name string) (int64, error) {
213213

214214
// canonicalize applies the dialect's identifier settings and rewrites to
215215
// an expression: the first rewrite whose pattern matches is applied, and
216-
// its result is not rewritten again.
216+
// its result is not rewritten again. A rewrite names a family as the
217+
// dialect spells it, so an expression spelled with an alias — dec(10) for
218+
// a rule on decimal — is tried again with the alias resolved.
217219
func (c *Catalog) canonicalize(t *TypeExpr) (*TypeExpr, error) {
218220
r, err := c.loadRules()
219221
if err != nil {
220222
return nil, err
221223
}
222224
t = r.identify(t)
225+
if len(r.rewrites) == 0 {
226+
return t, nil
227+
}
228+
if out, ok := r.rewrite(t); ok {
229+
return out, nil
230+
}
231+
if canonical, ok := c.canonicalFamilyName(t.Name); ok && !strings.EqualFold(canonical, t.Name) {
232+
resolved := t.Clone()
233+
resolved.Name = canonical
234+
if out, ok := r.rewrite(resolved); ok {
235+
return out, nil
236+
}
237+
}
238+
return t, nil
239+
}
240+
241+
// rewrite applies the first rewrite whose pattern matches the expression.
242+
func (r *rules) rewrite(t *TypeExpr) (*TypeExpr, bool) {
223243
name := strings.ToLower(t.Name)
224244
for _, rw := range r.rewrites {
225245
if strings.ToLower(rw.pattern.Name) != name || len(rw.pattern.Args) != len(t.Args) {
@@ -231,9 +251,26 @@ func (c *Catalog) canonicalize(t *TypeExpr) (*TypeExpr, error) {
231251
}
232252
out := substitute(rw.template, bindings)
233253
out.Nullable = t.Nullable
234-
return out, nil
254+
return out, true
235255
}
236-
return t, nil
256+
return nil, false
257+
}
258+
259+
// canonicalFamilyName is the name the catalog spells a family by, with
260+
// aliases resolved, or false when the name is not a family it holds.
261+
func (c *Catalog) canonicalFamilyName(name string) (string, bool) {
262+
oid, err := c.familyOIDByQualifiedName(strings.ToLower(strings.TrimSpace(name)))
263+
if err != nil {
264+
return "", false
265+
}
266+
if oid, err = c.canonicalOID(oid); err != nil {
267+
return "", false
268+
}
269+
info, err := c.LookupType(oid)
270+
if err != nil {
271+
return "", false
272+
}
273+
return c.qualifiedName(info), true
237274
}
238275

239276
// identify turns the bare words the dialect calls identifiers into

‎internal/core/seed/seed.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ type Type struct {
162162
Name string `json:"name"`
163163
Category string `json:"category"`
164164
Aliases []string `json:"aliases,omitempty"`
165+
// Base names the family this one stands on, which must be listed
166+
// before it: MySQL's bigint unsigned is a type of its own that
167+
// resolves as a bigint where nothing takes it as itself.
168+
Base string `json:"base,omitempty"`
165169
}
166170

167171
// Operator is a single operator overload.
@@ -305,6 +309,11 @@ func apply(cat *core.Catalog, fsys fs.FS, settings Settings) error {
305309
if err := stream(fsys, TypesFile, b.addType); err != nil {
306310
return err
307311
}
312+
// Every dialect has arrays, whether or not its list names the family,
313+
// and a lookup of one against the cached catalog cannot add it then.
314+
if _, err := b.createType(core.ArrayTypeName, "A", 0); err != nil {
315+
return err
316+
}
308317
if err := b.consts(); err != nil {
309318
return err
310319
}
@@ -439,6 +448,9 @@ func Relations(fsys fs.FS, dir, schema string) ([]*catalog.Table, error) {
439448
IsNotNull: col.NotNull,
440449
IsArray: col.Array,
441450
}
451+
if col.Array {
452+
column.ArrayDims = 1
453+
}
442454
if col.Length > 0 {
443455
length := col.Length
444456
column.Length = &length
@@ -513,7 +525,15 @@ type categorized struct {
513525
}
514526

515527
func (b *builder) addType(t Type) error {
516-
oid, err := b.createType(t.Name, t.Category)
528+
var baseOID int64
529+
if t.Base != "" {
530+
oid, ok := b.oids[strings.ToLower(t.Base)]
531+
if !ok {
532+
return fmt.Errorf("type %q: base %q is not a type listed before it", t.Name, t.Base)
533+
}
534+
baseOID = oid
535+
}
536+
oid, err := b.createType(t.Name, t.Category, baseOID)
517537
if err != nil {
518538
return fmt.Errorf("type %q: %w", t.Name, err)
519539
}
@@ -553,7 +573,7 @@ func (b *builder) addAlias(name string, typeOID int64, category string) error {
553573
return nil
554574
}
555575

556-
func (b *builder) createType(name, category string) (int64, error) {
576+
func (b *builder) createType(name, category string, baseOID int64) (int64, error) {
557577
key := strings.ToLower(name)
558578
if oid, ok := b.oids[key]; ok {
559579
return oid, nil
@@ -562,6 +582,7 @@ func (b *builder) createType(name, category string) (int64, error) {
562582
Name: key,
563583
Typtype: "b",
564584
Category: category,
585+
BaseOID: baseOID,
565586
DialectOID: b.dialectOID,
566587
})
567588
if err != nil {

‎internal/core/types.go‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -362,11 +362,18 @@ func splitQualifiedName(name string) (ns, bare string) {
362362
}
363363

364364
// declaredTypeNamespace is the namespace a declared type's row goes in: the
365-
// one its name qualifies, created if the schema has not, or the default.
365+
// one its name qualifies, created if the schema has not, or the dialect's
366+
// default schema — dbo, main — so that a bare CREATE TYPE and a qualified
367+
// reference to it name one row.
366368
func (c *Catalog) declaredTypeNamespace(name string) (int64, string, error) {
367369
ns, bare := splitQualifiedName(name)
368370
if ns == "" {
369-
return 0, bare, nil
371+
if c.dialectOID != 0 {
372+
ns, _ = c.DialectFlag(c.dialectOID, FlagDefaultSchema)
373+
}
374+
if ns == "" {
375+
return 0, bare, nil
376+
}
370377
}
371378
oid, err := c.NamespaceOID(ns)
372379
if err != nil {
@@ -551,12 +558,7 @@ func (c *Catalog) TypeExprOf(oid int64) (*TypeExpr, error) {
551558
if err != nil {
552559
return nil, err
553560
}
554-
expr := &TypeExpr{Name: info.Name}
555-
// A type outside the default namespaces is named with its namespace,
556-
// as format_type prints a type off the search path.
557-
if ns, err := c.namespaceName(info.NamespaceOID); err == nil && ns != "" && !slices.Contains(c.DefaultNamespaces(), ns) {
558-
expr.Name = ns + "." + info.Name
559-
}
561+
expr := &TypeExpr{Name: c.qualifiedName(info)}
560562
if !info.IsFamily() {
561563
rows, err := c.q.TypeArgs(context.Background(), oid)
562564
if err != nil {
@@ -592,6 +594,18 @@ func (c *Catalog) TypeExprOf(oid int64) (*TypeExpr, error) {
592594
return expr.Clone(), nil
593595
}
594596

597+
// qualifiedName is the name a type row is known by in an expression: its
598+
// name, qualified with its namespace when that is not one of the dialect's
599+
// defaults, as format_type prints a type off the search path. An
600+
// instance's key is built from these, so the array of one schema's mood is
601+
// a row apart from the array of another's.
602+
func (c *Catalog) qualifiedName(info TypeInfo) string {
603+
if ns, err := c.namespaceName(info.NamespaceOID); err == nil && ns != "" && !slices.Contains(c.DefaultNamespaces(), ns) {
604+
return ns + "." + info.Name
605+
}
606+
return info.Name
607+
}
608+
595609
// ResolveTypeExpr interns the type an expression names and returns its row:
596610
// the family for a bare name, the instance for a family applied to
597611
// arguments, each argument type interned first. Names are canonicalized, so
@@ -680,7 +694,7 @@ func (c *Catalog) internType(t *TypeExpr, newFamily func(name string) (int64, er
680694
name := strings.ToLower(strings.TrimSpace(t.Name))
681695
familyOID, err := c.familyOIDByQualifiedName(name)
682696
if err != nil {
683-
if name == ArrayTypeName {
697+
if name == ArrayTypeName && write {
684698
// Every dialect has arrays, whether or not its seed lists the
685699
// family; one that does not gets it as an array type rather
686700
// than a user type.
@@ -700,27 +714,36 @@ func (c *Catalog) internType(t *TypeExpr, newFamily func(name string) (int64, er
700714
return 0, nil, err
701715
}
702716
if len(t.Args) == 0 {
703-
return familyOID, &TypeExpr{Name: family.Name}, nil
717+
return familyOID, &TypeExpr{Name: c.qualifiedName(family)}, nil
704718
}
705719

706720
// The instance's canonical spelling is the family applied to its
707721
// arguments as the catalog spells them, so each argument type is
708-
// resolved first and read back.
709-
canonical := &TypeExpr{Name: family.Name, Args: make([]TypeArg, len(t.Args))}
722+
// resolved first and read back. An argument whose own instance is not
723+
// a row, in a lookup that may not write, still has a canonical
724+
// spelling, which the whole expression's is built from.
725+
canonical := &TypeExpr{Name: c.qualifiedName(family), Args: make([]TypeArg, len(t.Args))}
710726
argOIDs := make([]int64, len(t.Args))
727+
unknown := false
711728
for i, a := range t.Args {
712729
canonical.Args[i] = a
713730
if a.Type == nil {
714731
continue
715732
}
716733
oid, argExpr, err := c.internType(a.Type, newFamily, write)
717734
if err != nil {
718-
return 0, nil, err
735+
if !errors.Is(err, errUnknownType) || argExpr == nil {
736+
return 0, nil, err
737+
}
738+
unknown = true
719739
}
720740
argOIDs[i] = oid
721741
argExpr.Nullable = a.Type.Nullable
722742
canonical.Args[i].Type = argExpr
723743
}
744+
if unknown {
745+
return 0, canonical, errUnknownType
746+
}
724747
key := canonical.Key()
725748
ctx := context.Background()
726749
if oid, err := c.q.TypeOIDByExprInNamespace(ctx, catalogdb.TypeOIDByExprInNamespaceParams{

‎internal/core/types.md‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,9 @@ and details settled on the way:
658658
`float($1)` to `real` where `$1 <= 24`, `decimal` to `decimal(18, 0)`,
659659
`Decimal32($1)` to `Decimal(9, $1)`, `sysname` to `nvarchar(128)` — which
660660
the seed loads into `sql_type_rewrite` and the catalog applies before
661-
interning, first match winning; `idents` and `ident_args`, the words and
661+
interning, first match winning, tried once with the name as spelled and
662+
once with its alias resolved so that `dec` meets a rule on `decimal`;
663+
`idents` and `ident_args`, the words and
662664
argument positions that are identifiers rather than types, kept as dialect
663665
flags; and `affinity`, SQLite's ordered rule for a family the seed does
664666
not list, loaded into `sql_type_affinity` and asked when a schema
@@ -672,7 +674,22 @@ and details settled on the way:
672674
- A bare type name resolves in the default namespaces only — the catalog's
673675
own, `pg_catalog` and the dialect's default schema — and `CREATE TYPE`
674676
deduplicates within the namespace it names, so `foo.mood` and `mood` are
675-
two types and a bare `mood` never binds to `foo.mood`.
677+
two types and a bare `mood` never binds to `foo.mood`. A bare `CREATE
678+
TYPE` lands in the dialect's default schema when it has one, so SQL
679+
Server's `PhoneNumber` and `dbo.PhoneNumber` are one row. A type outside
680+
the default namespaces is spelled with its namespace wherever the catalog
681+
spells it, including inside an instance's key, so `array(foo.mood)` and
682+
`array(mood)` are two rows.
683+
- A lookup that may not write — the analyzer's, against the cached catalog
684+
— still canonicalizes an expression whose instance is not a row, however
685+
deep the missing instance sits, so `$1::varchar(10)[]` reports
686+
`array(character varying(10))` against the `array` family. The seed
687+
gives every dialect the `array` family for that reason, whether or not
688+
its `types.jsonl` lists it.
689+
- A family in `types.jsonl` may name a `base` listed before it, which is
690+
how MySQL's unsigned families stand on their signed ones: `bigint
691+
unsigned` is a type of its own, and resolves as a `bigint` where nothing
692+
takes it as itself.
676693
- SQLite's `dialect.json` says `"alias": "base"`, which makes each alias in
677694
its `types.jsonl` a type of its own standing on the type it aliases,
678695
rather than another spelling of it.
@@ -690,7 +707,13 @@ and details settled on the way:
690707
placeholder gives itself.
691708
- A cast is NULL when its operand is, or when its type says so, as
692709
`Nullable(String)` does; a cast of a placeholder types the placeholder
693-
and takes its name and source from what it is compared with.
710+
and takes its name and source from what it is compared with. A cast to
711+
`interval day to second` decodes the field mask the parser reports the
712+
way a column definition does.
713+
- ClickHouse's `LowCardinality(Nullable(T))`, the only order it accepts,
714+
is a nullable column, and the converter and `goldeneye` both read it as
715+
`lowcardinality(T)` with the nullability on the outside, where the
716+
column's nullability lives.
694717
- MySQL types `CAST(x AS CHAR(10))` as `varchar(10)` and `CAST(x AS
695718
BINARY(8))` as `varbinary(8)`, which is what its metadata and a view over
696719
the cast both report, rather than the `char(10)` the table above
@@ -705,7 +728,10 @@ and details settled on the way:
705728
not its labels, since the canonicalizer cannot see the catalog. A
706729
GoogleSQL array or struct constructor in a select list is still untyped.
707730
- PostgreSQL's `relations.jsonl` spells an array column as its element with
708-
the array flag, which `goldeneye` now writes from `typelem`.
731+
the array flag, which `goldeneye` now writes from `typelem` for the
732+
`_`-prefixed array types alone; `int2vector` and `oidvector` share the
733+
array category but stay types of their own. MySQL's keeps the case of an
734+
enum's members, which are values.
709735

710736
## Order of work
711737

‎internal/endtoend/testdata/analyze_system_catalog/mysql/stdout.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,13 @@
7575
"name": "enum",
7676
"args": [
7777
{
78-
"string": "base table"
78+
"string": "BASE TABLE"
7979
},
8080
{
81-
"string": "view"
81+
"string": "VIEW"
8282
},
8383
{
84-
"string": "system view"
84+
"string": "SYSTEM VIEW"
8585
}
8686
]
8787
},

‎internal/endtoend/testdata/analyze_types/clickhouse/stdout.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,11 @@
8686
"name": "kind",
8787
"type": {
8888
"name": "lowcardinality",
89+
"nullable": true,
8990
"args": [
9091
{
9192
"type": {
92-
"name": "string",
93-
"nullable": true
93+
"name": "string"
9494
}
9595
}
9696
]
@@ -456,11 +456,11 @@
456456
"name": "kind",
457457
"type": {
458458
"name": "lowcardinality",
459+
"nullable": true,
459460
"args": [
460461
{
461462
"type": {
462-
"name": "string",
463-
"nullable": true
463+
"name": "string"
464464
}
465465
}
466466
]

‎internal/endtoend/testdata/analyze_types/duckdb/query.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ SELECT
99
$4::mood AS d,
1010
CAST($5 AS VARCHAR(5)) AS e,
1111
$6::MAP(VARCHAR, INTEGER) AS f,
12-
$7::INTEGER[3] AS g
12+
$7::INTEGER[3] AS g,
13+
$8::main.mood AS h
1314
FROM things;
1415

1516
-- name: Params :one

‎internal/endtoend/testdata/analyze_types/duckdb/schema.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ CREATE TABLE things (
1111
title VARCHAR(10),
1212
kind ENUM('a','b'),
1313
m mood,
14+
mm main.mood,
1415
big HUGEINT,
1516
ubig UHUGEINT,
1617
data BLOB,

0 commit comments

Comments
 (0)