Skip to content

Commit 319d657

Browse files
committed
fmt: print type names and DDL as the author wrote them
Two-word type names printed squashed (VARYING CHARACTER(32) came back as VARYINGCHARACTER(32)): the converter folds the spaces out of type names for catalog matching, and the printer had nothing else to show. ast.TypeName now carries the authored spelling alongside the folded name — the sqlite converter fills it for column types and casts from meyer's token-joined name — and the printer prefers it. Fixing that surfaced how much of a CREATE TABLE the reprint silently destroyed: PRIMARY KEY came back as NOT NULL, DEFAULT, UNIQUE, CHECK and every other constraint vanished, a typeless column grew an 'any', and IF NOT EXISTS was dropped. Column PRIMARY KEY, typeless columns and IF NOT EXISTS are now modeled and print faithfully. Everything the node still cannot carry — other column constraints, table constraints, table options, TEMP, AS SELECT, and a NOT NULL next to a PRIMARY KEY (which does not imply it in SQLite) — marks the statement Incomplete: it renders as nothing, no verification accepts that, and sqlc fmt keeps the statement exactly as written. ALTER TABLE ADD COLUMN gets the same treatment. TestFormat learns that an empty rendering is that signal, and the fmt endtoend case pins the new behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MTvpHqNMadH12pTtsgUq2
1 parent 4251fd7 commit 319d657

9 files changed

Lines changed: 152 additions & 20 deletions

File tree

internal/endtoend/fmt_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ func TestFormat(t *testing.T) {
169169
}
170170

171171
out := ast.Format(stmt.Raw, formatter)
172+
// An empty rendering is the formatter's signal that
173+
// the statement carries syntax the AST does not
174+
// model; sqlc fmt keeps such statements as written.
175+
if strings.TrimSpace(strings.TrimSuffix(strings.TrimSpace(out), ";")) == "" {
176+
t.Skip("statement has no faithful rendering")
177+
}
172178
actual, err := fingerprint(out)
173179
if err != nil {
174180
t.Error(err)

internal/endtoend/testdata/fmt/sqlite/query.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,17 @@ CREATE TABLE scratch (
4242
id INTEGER NOT NULL,
4343
label TEXT
4444
);
45+
46+
-- name: MakeMeasurements :exec
47+
CREATE TABLE IF NOT EXISTS measurements (
48+
id INTEGER PRIMARY KEY,
49+
label VARYING CHARACTER(120),
50+
ratio DECIMAL(10,5),
51+
note
52+
);
53+
54+
-- name: KeepAutoinc :exec
55+
CREATE TABLE counters (id INTEGER PRIMARY KEY AUTOINCREMENT, hits INTEGER DEFAULT 0);
56+
57+
-- name: CastLabel :one
58+
SELECT CAST(bio AS VARYING CHARACTER(120)) FROM authors LIMIT 1;

internal/endtoend/testdata/fmt/sqlite/stdout.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,12 @@
4646
WHERE id <> @at_param AND id <> :colon_param AND id <> $dollar_param;
4747

4848
-- name: TopAuthors :many
49+
@@ -46,7 +53,7 @@
50+
-- name: MakeMeasurements :exec
51+
CREATE TABLE IF NOT EXISTS measurements (
52+
id INTEGER PRIMARY KEY,
53+
+ label VARYING CHARACTER(120),
54+
- label VARYING CHARACTER(120),
55+
ratio DECIMAL(10,5),
56+
note
57+
);

internal/engine/sqlite/convert.go

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -214,15 +214,21 @@ func (c *cc) convertAlterTableStmt(n *meyer.AlterTableStmt) ast.Node {
214214
}
215215
name := identifier(def.Name)
216216
return &ast.AlterTableStmt{
217-
Table: parseTableName(n.Table),
217+
Table: parseTableName(n.Table),
218+
Incomplete: !representableColumn(def),
218219
Cmds: &ast.List{Items: []ast.Node{
219220
&ast.AlterTableCmd{
220221
Name: &name,
221222
Subtype: ast.AT_AddColumn,
222223
Def: &ast.ColumnDef{
223-
Colname: name,
224-
TypeName: &ast.TypeName{Name: columnTypeName(def.Type)},
225-
IsNotNull: hasNotNullConstraint(def.Constraints),
224+
Colname: name,
225+
TypeName: &ast.TypeName{
226+
Name: columnTypeName(def.Type),
227+
Spelling: typeSpelling(def.Type),
228+
},
229+
Typeless: def.Type == nil,
230+
IsNotNull: hasNotNullConstraint(def.Constraints),
231+
PrimaryKey: hasPrimaryKeyConstraint(def.Constraints),
226232
},
227233
},
228234
}},
@@ -278,13 +284,24 @@ func (c *cc) convertCreateTableStmt(n *meyer.CreateTableStmt) ast.Node {
278284
stmt := &ast.CreateTableStmt{
279285
Name: parseTableName(n.Name),
280286
IfNotExists: n.IfNotExists,
287+
// The node models none of these, so a statement carrying them has
288+
// no faithful rendering and the formatter keeps it as written.
289+
Incomplete: n.Temp || n.Select != nil || len(n.Constraints) > 0 || len(n.Options) > 0,
281290
}
282291
for _, def := range n.Columns {
292+
if !representableColumn(def) {
293+
stmt.Incomplete = true
294+
}
283295
stmt.Cols = append(stmt.Cols, &ast.ColumnDef{
284-
Colname: identifier(def.Name),
285-
IsNotNull: hasNotNullConstraint(def.Constraints),
286-
TypeName: &ast.TypeName{Name: columnTypeName(def.Type)},
287-
Location: def.Pos(),
296+
Colname: identifier(def.Name),
297+
IsNotNull: hasNotNullConstraint(def.Constraints),
298+
PrimaryKey: hasPrimaryKeyConstraint(def.Constraints),
299+
TypeName: &ast.TypeName{
300+
Name: columnTypeName(def.Type),
301+
Spelling: typeSpelling(def.Type),
302+
},
303+
Typeless: def.Type == nil,
304+
Location: def.Pos(),
288305
})
289306
}
290307
return stmt
@@ -307,7 +324,9 @@ func (c *cc) convertCreateVirtualTableFTS5(n *meyer.CreateVirtualTableStmt) ast.
307324
stmt := &ast.CreateTableStmt{
308325
Name: parseTableName(n.Name),
309326
IfNotExists: n.IfNotExists,
310-
Virtual: true,
327+
// A virtual table's module arguments are parsed away, so the
328+
// statement has no faithful rendering.
329+
Incomplete: true,
311330
}
312331

313332
// The module arguments of a virtual table are an arbitrary token
@@ -1035,6 +1054,7 @@ func (c *cc) convertCastExpr(n *meyer.CastExpr) ast.Node {
10351054
Arg: c.convert(n.X),
10361055
TypeName: &ast.TypeName{
10371056
Name: name,
1057+
Spelling: typeSpelling(n.Type),
10381058
Names: &ast.List{Items: []ast.Node{&ast.String{Str: strings.ToLower(name)}}},
10391059
ArrayBounds: &ast.List{},
10401060
},

internal/engine/sqlite/utils.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,60 @@ func typeName(n *meyer.TypeName) string {
7676
return sb.String()
7777
}
7878

79+
// typeSpelling returns the type as the author wrote it — identifier tokens
80+
// space-joined, arguments as written — for the formatter to print back.
81+
// typeName folds the spaces out for catalog matching.
82+
func typeSpelling(n *meyer.TypeName) string {
83+
if n == nil {
84+
return ""
85+
}
86+
var sb strings.Builder
87+
sb.WriteString(n.Name)
88+
if len(n.Args) > 0 {
89+
sb.WriteString("(")
90+
sb.WriteString(strings.Join(n.Args, ","))
91+
sb.WriteString(")")
92+
}
93+
return sb.String()
94+
}
95+
96+
// representableColumn reports whether a column definition round-trips
97+
// through the fields ast.ColumnDef models: a plain NOT NULL, a plain
98+
// PRIMARY KEY, or nothing. Anything else — a named constraint, a conflict
99+
// clause, ASC/DESC or AUTOINCREMENT, DEFAULT, UNIQUE, CHECK, COLLATE,
100+
// REFERENCES, GENERATED — is parsed away, so a statement printing such a
101+
// column would lose it. NOT NULL next to PRIMARY KEY is also beyond the
102+
// node: SQLite's PRIMARY KEY does not imply NOT NULL, and the printed form
103+
// carries only one of the pair.
104+
func representableColumn(def *meyer.ColumnDef) bool {
105+
var notNull, primary bool
106+
for _, con := range def.Constraints {
107+
switch {
108+
case con.Name != nil:
109+
return false
110+
case con.Kind == meyer.ColumnNotNull && con.OnConflict == meyer.ConflictDefault:
111+
notNull = true
112+
case con.Kind == meyer.ColumnPrimaryKey && con.OnConflict == meyer.ConflictDefault &&
113+
con.Order == meyer.SortDefault && !con.AutoIncrement:
114+
primary = true
115+
default:
116+
return false
117+
}
118+
}
119+
return !(notNull && primary)
120+
}
121+
122+
// hasPrimaryKeyConstraint reports whether a column definition carries a
123+
// PRIMARY KEY constraint.
124+
func hasPrimaryKeyConstraint(constraints []*meyer.ColumnConstraint) bool {
125+
for _, constraint := range constraints {
126+
if constraint.Kind == meyer.ColumnPrimaryKey {
127+
return true
128+
}
129+
}
130+
return false
131+
}
132+
79133
// hasNotNullConstraint reports whether a column definition guarantees a
80134
// value. A PRIMARY KEY implies NOT NULL for the purposes of code
81135
// generation, matching the behavior of the previous parser.

internal/sql/ast/alter_table_stmt.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ type AlterTableStmt struct {
99
Cmds *List
1010
MissingOk bool
1111
Relkind ObjectType
12+
// Incomplete marks a statement whose source carried syntax this node
13+
// does not model, such as column constraints beyond plain NOT NULL and
14+
// PRIMARY KEY on an added column. No faithful rendering exists for it.
15+
Incomplete bool
1216
}
1317

1418
func (n *AlterTableStmt) Pos() int {
@@ -19,6 +23,12 @@ func (n *AlterTableStmt) Format(buf *TrackedBuffer, d format.Dialect) {
1923
if n == nil {
2024
return
2125
}
26+
// An incomplete statement cannot be printed back: part of its source
27+
// was parsed away. Render nothing, which no verification accepts, so
28+
// the formatter keeps the statement as written.
29+
if n.Incomplete {
30+
return
31+
}
2232
buf.WriteString("ALTER TABLE ")
2333
buf.astFormat(n.Relation, d)
2434
buf.astFormat(n.Table, d)

internal/sql/ast/column_def.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ package ast
33
import "github.com/sqlc-dev/sqlc/internal/sql/format"
44

55
type ColumnDef struct {
6-
Colname string
6+
Colname string
7+
// TypeName is the column's type for the catalog. When Typeless is set
8+
// the author wrote no type at all — SQLite allows it — and the column
9+
// prints without one, whatever TypeName carries.
710
TypeName *TypeName
11+
Typeless bool
812
IsNotNull bool
913
IsUnsigned bool
1014
IsArray bool
@@ -39,8 +43,10 @@ func (n *ColumnDef) Format(buf *TrackedBuffer, d format.Dialect) {
3943
return
4044
}
4145
buf.WriteString(n.Colname)
42-
buf.WriteString(" ")
43-
buf.astFormat(n.TypeName, d)
46+
if !n.Typeless {
47+
buf.WriteString(" ")
48+
buf.astFormat(n.TypeName, d)
49+
}
4450
// Use IsArray from ColumnDef since TypeName.ArrayBounds may not be set
4551
// (for type resolution compatibility)
4652
if n.IsArray && !items(n.TypeName.ArrayBounds) {

internal/sql/ast/create_table_stmt.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ type CreateTableStmt struct {
99
ReferTable *TableName
1010
Comment string
1111
Inherits []*TableName
12-
// Virtual marks a table backed by a module (SQLite's CREATE VIRTUAL
13-
// TABLE). The statement's argument list cannot be reconstructed from
14-
// this node, so it has no faithful rendering.
15-
Virtual bool
12+
// Incomplete marks a statement whose source carried syntax this node
13+
// does not model — a virtual table's module arguments, column or table
14+
// constraints beyond plain NOT NULL and PRIMARY KEY, table options,
15+
// TEMP, or an AS SELECT body. No faithful rendering exists for it.
16+
Incomplete bool
1617
}
1718

1819
func (n *CreateTableStmt) Pos() int {
@@ -23,13 +24,16 @@ func (n *CreateTableStmt) Format(buf *TrackedBuffer, d format.Dialect) {
2324
if n == nil {
2425
return
2526
}
26-
// A virtual table cannot be printed back: its module arguments were
27-
// parsed away. Render nothing, which no verification accepts, so the
28-
// formatter keeps the statement as written.
29-
if n.Virtual {
27+
// An incomplete statement cannot be printed back: part of its source
28+
// was parsed away. Render nothing, which no verification accepts, so
29+
// the formatter keeps the statement as written.
30+
if n.Incomplete {
3031
return
3132
}
3233
buf.WriteString("CREATE TABLE ")
34+
if n.IfNotExists {
35+
buf.WriteString("IF NOT EXISTS ")
36+
}
3337
buf.astFormat(n.Name, d)
3438

3539
buf.WriteString(" (")

internal/sql/ast/type_name.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ type TypeName struct {
66
Catalog string
77
Schema string
88
Name string
9+
// Spelling is the type as the author wrote it, when that differs from
10+
// Name: SQLite folds the spaces out of multi-word type names ("VARYING
11+
// CHARACTER" resolves as "VARYINGCHARACTER" in the catalog), so the
12+
// formatter prints this back instead of the folded form.
13+
Spelling string
914

1015
// From pg.TypeName
1116
Names *List
@@ -26,6 +31,10 @@ func (n *TypeName) Format(buf *TrackedBuffer, d format.Dialect) {
2631
if n == nil {
2732
return
2833
}
34+
if n.Spelling != "" {
35+
buf.WriteString(n.Spelling)
36+
goto addMods
37+
}
2938
if items(n.Names) {
3039
// Check if this is a qualified type (e.g., pg_catalog.int4)
3140
if len(n.Names.Items) == 2 {

0 commit comments

Comments
 (0)