From 9d44c0054d643d3ec93f811f00bc61ea14d79ec2 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Mon, 31 Aug 2026 15:30:50 +1000 Subject: [PATCH 1/2] Add PostgreSQL ADD COLUMN synthesis to the DDL parser Pure seam for startup schema convergence: lift the ColumnDef from the desired CREATE TABLE into a deparsed ALTER TABLE ... ADD COLUMN, so the converger never hand-maintains a second copy of column DDL. Nothing calls it yet; the tripwire-to-convergence flip follows separately. --- pkg/ddl/postgres_parser.go | 45 +++++++++++++++++++++ pkg/ddl/postgres_parser_test.go | 70 +++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/pkg/ddl/postgres_parser.go b/pkg/ddl/postgres_parser.go index ac02fe1f8..c6595ea27 100644 --- a/pkg/ddl/postgres_parser.go +++ b/pkg/ddl/postgres_parser.go @@ -116,6 +116,51 @@ func (postgresStatementParser) CreateTableColumns(stmt string) ([]string, error) return columns, nil } +// SynthesizePostgresAddColumn builds an ALTER TABLE statement from a column +// declaration in exactly one PostgreSQL CREATE TABLE statement. +func SynthesizePostgresAddColumn(createTableDDL, columnName string) (string, error) { + result, err := pgquery.Parse(createTableDDL) + if err != nil { + return "", fmt.Errorf("parse CREATE TABLE %q: %w", statementPreview(createTableDDL), err) + } + stmts := result.GetStmts() + if len(stmts) != 1 { + return "", fmt.Errorf("expected one CREATE TABLE statement, got %d", len(stmts)) + } + createNode, ok := stmts[0].GetStmt().GetNode().(*pgproto.Node_CreateStmt) + if !ok { + return "", fmt.Errorf("expected CREATE TABLE statement") + } + + var columnNode *pgproto.Node + for _, element := range createNode.CreateStmt.GetTableElts() { + column, ok := element.GetNode().(*pgproto.Node_ColumnDef) + if ok && column.ColumnDef.GetColname() == columnName { + columnNode = element + break + } + } + if columnNode == nil { + return "", fmt.Errorf("column %q not found in CREATE TABLE statement", columnName) + } + + result.Stmts = []*pgproto.RawStmt{{ + Stmt: &pgproto.Node{Node: &pgproto.Node_AlterTableStmt{AlterTableStmt: &pgproto.AlterTableStmt{ + Relation: createNode.CreateStmt.GetRelation(), + Cmds: []*pgproto.Node{{Node: &pgproto.Node_AlterTableCmd{AlterTableCmd: &pgproto.AlterTableCmd{ + Subtype: pgproto.AlterTableType_AT_AddColumn, + Def: columnNode, + }}}}, + Objtype: pgproto.ObjectType_OBJECT_TABLE, + }}}, + }} + ddl, err := pgquery.Deparse(result) + if err != nil { + return "", fmt.Errorf("deparse ADD COLUMN for %q: %w", columnName, err) + } + return ddl, nil +} + // CreateIndex implements StatementParser using the parsed IndexStmt. Any // standalone CREATE INDEX statement reports its index and table names, with // unique carrying the UNIQUE declaration; other parsed statement types return diff --git a/pkg/ddl/postgres_parser_test.go b/pkg/ddl/postgres_parser_test.go index ab24fefb1..bfd7e54d5 100644 --- a/pkg/ddl/postgres_parser_test.go +++ b/pkg/ddl/postgres_parser_test.go @@ -1,11 +1,13 @@ package ddl import ( + "io/fs" "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + pgquery "github.com/wasilibs/go-pgquery" "github.com/block/schemabot/pkg/schema" ) @@ -208,6 +210,74 @@ func TestPostgresParserCreateTableColumns(t *testing.T) { require.ErrorContains(t, err, "expected CREATE TABLE statement") } +func TestSynthesizePostgresAddColumn(t *testing.T) { + tests := []struct { + name string + createDDL string + columnName string + want string + }{ + {"plain column", "CREATE TABLE users (id bigint, email text)", "email", "ALTER TABLE users ADD COLUMN email text"}, + {"not null and default", "CREATE TABLE users (id bigint, enabled boolean NOT NULL DEFAULT true)", "enabled", "ALTER TABLE users ADD COLUMN enabled boolean NOT NULL DEFAULT true"}, + {"type modifier", "CREATE TABLE users (name varchar(255))", "name", "ALTER TABLE users ADD COLUMN name varchar(255)"}, + {"timestamp function default", "CREATE TABLE users (updated_at timestamptz DEFAULT now())", "updated_at", "ALTER TABLE users ADD COLUMN updated_at timestamptz DEFAULT now()"}, + {"schema-qualified table", "CREATE TABLE app.users (id bigint)", "id", "ALTER TABLE app.users ADD COLUMN id bigint"}, + {"quoted identifiers", `CREATE TABLE "App"."UserProfiles" ("DisplayName" varchar(255) NOT NULL)`, "DisplayName", `ALTER TABLE "App"."UserProfiles" ADD COLUMN "DisplayName" varchar(255) NOT NULL`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := SynthesizePostgresAddColumn(tc.createDDL, tc.columnName) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } + + t.Run("column not found", func(t *testing.T) { + _, err := SynthesizePostgresAddColumn("CREATE TABLE users (id bigint)", "email") + require.ErrorContains(t, err, `column "email" not found`) + }) + + t.Run("multiple statements", func(t *testing.T) { + _, err := SynthesizePostgresAddColumn("CREATE TABLE users (id bigint); CREATE TABLE teams (id bigint)", "id") + require.ErrorContains(t, err, "expected one CREATE TABLE statement, got 2") + }) + + t.Run("not a CREATE TABLE", func(t *testing.T) { + _, err := SynthesizePostgresAddColumn("ALTER TABLE users ADD COLUMN email text", "email") + require.ErrorContains(t, err, "expected CREATE TABLE statement") + }) + + t.Run("parse failure", func(t *testing.T) { + _, err := SynthesizePostgresAddColumn("CREATE TABLE users (", "email") + require.ErrorContains(t, err, "parse CREATE TABLE") + }) +} + +func TestSynthesizePostgresAddColumn_EmbeddedSchema(t *testing.T) { + files, err := fs.Glob(schema.PostgresFS, "postgres/*.sql") + require.NoError(t, err) + require.NotEmpty(t, files) + p := postgresStatementParser{} + + for _, file := range files { + content, err := schema.PostgresFS.ReadFile(file) + require.NoError(t, err, "read %s", file) + statements, err := p.Split(string(content)) + require.NoError(t, err, "split %s", file) + require.NotEmpty(t, statements, "schema file %s", file) + columns, err := p.CreateTableColumns(statements[0]) + require.NoError(t, err, "columns in %s", file) + + for _, column := range columns { + ddl, err := SynthesizePostgresAddColumn(statements[0], column) + require.NoError(t, err, "%s column %s", file, column) + parsed, err := pgquery.Parse(ddl) + require.NoError(t, err, "%s column %s: %s", file, column, ddl) + require.Len(t, parsed.GetStmts(), 1, "%s column %s", file, column) + } + } +} + func TestPostgresParserCreateIndex(t *testing.T) { p := postgresStatementParser{} From 615b4f1d4cf4f79f2eca242de0c230db9c0b37dd Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Wed, 2 Sep 2026 08:26:00 +1000 Subject: [PATCH 2/2] refactor(postgres): move ADD COLUMN synthesis behind the parser seam Dialect-specific parsing behavior must be reachable only through ParserForDialect, matching CreateIndex. Pin the seam's contract in its docs: synthesis is faithful and judges applicability nowhere, and the column name is the parser-folded spelling CreateTableColumns returns. The corpus test iterates every CREATE TABLE in the embedded schema and proves proto-equality round-trips, nothing more. --- pkg/ddl/parser.go | 25 ++++++ pkg/ddl/parser_test.go | 4 + pkg/ddl/postgres_parser.go | 10 ++- pkg/ddl/postgres_parser_test.go | 130 ++++++++++++++++++++++++++++---- 4 files changed, 150 insertions(+), 19 deletions(-) diff --git a/pkg/ddl/parser.go b/pkg/ddl/parser.go index 1eea51f87..7f507acc9 100644 --- a/pkg/ddl/parser.go +++ b/pkg/ddl/parser.go @@ -46,6 +46,24 @@ type StatementParser interface { // name without an error. CreateIndex(stmt string) (indexName, tableName string, unique bool, err error) + // SynthesizeAddColumn builds an ALTER TABLE ... ADD COLUMN statement for + // the named column of exactly one CREATE TABLE statement, carrying the + // column's declaration verbatim at the parse-tree level: its type and all + // column-level constraints. Table-level constraints (PRIMARY KEY (...), + // UNIQUE (...), CHECK (...)) are not part of a column declaration and are + // not carried. The result is the parser's normalized rendering of the + // synthesized statement, not a textual slice of the input DDL. + // + // The seam is faithful by design and judges nothing: a NOT NULL column + // without a DEFAULT synthesizes exactly as declared even though PostgreSQL + // rejects that ALTER on a populated table. Whether the synthesized + // statement can be applied is the caller's decision. + // + // columnName is matched against the parser-folded column name — the + // values CreateTableColumns returns — so an unquoted "Email" in the DDL + // is found as "email", and only a quoted identifier keeps its case. + SynthesizeAddColumn(createTableDDL, columnName string) (string, error) + // Canonicalize normalizes a single DDL statement's formatting, returning // the input unchanged when it cannot be parsed. Canonicalize(ddl string) string @@ -137,6 +155,13 @@ func (tidbStatementParser) CreateIndex(string) (string, string, bool, error) { return "", "", false, fmt.Errorf("CREATE INDEX inspection is not supported by the MySQL statement parser") } +// SynthesizeAddColumn implements StatementParser. This synthesis operation is +// currently needed only by the PostgreSQL storage bootstrapper; the MySQL +// bootstrapper diffs schemas with Spirit instead. +func (tidbStatementParser) SynthesizeAddColumn(string, string) (string, error) { + return "", fmt.Errorf("ADD COLUMN synthesis is not supported by the MySQL statement parser") +} + // statementTypeFromSpirit translates Spirit's parser-owned statement type into // the pkg/ddl-owned vocabulary at the seam boundary, so Spirit's type never // escapes the TiDB implementation. diff --git a/pkg/ddl/parser_test.go b/pkg/ddl/parser_test.go index beaab582d..99b3305a4 100644 --- a/pkg/ddl/parser_test.go +++ b/pkg/ddl/parser_test.go @@ -46,6 +46,10 @@ func (f fakeStatementParser) CreateIndex(string) (string, string, bool, error) { return "", "", false, nil } +func (f fakeStatementParser) SynthesizeAddColumn(string, string) (string, error) { + return "", nil +} + func (f fakeStatementParser) Canonicalize(string) string { return f.canonicalized } diff --git a/pkg/ddl/postgres_parser.go b/pkg/ddl/postgres_parser.go index c6595ea27..decab3755 100644 --- a/pkg/ddl/postgres_parser.go +++ b/pkg/ddl/postgres_parser.go @@ -116,9 +116,13 @@ func (postgresStatementParser) CreateTableColumns(stmt string) ([]string, error) return columns, nil } -// SynthesizePostgresAddColumn builds an ALTER TABLE statement from a column -// declaration in exactly one PostgreSQL CREATE TABLE statement. -func SynthesizePostgresAddColumn(createTableDDL, columnName string) (string, error) { +// SynthesizeAddColumn implements StatementParser. It grafts the column's +// ColumnDef parse node — type and column-level constraints intact — from the +// CREATE TABLE tree into a fresh ALTER TABLE ... ADD COLUMN tree and deparses +// it, so the output is libpg_query's normalized rendering rather than a +// textual slice of the input. Table-level constraints are separate TableElts +// nodes, not part of the ColumnDef, and are not carried. +func (postgresStatementParser) SynthesizeAddColumn(createTableDDL, columnName string) (string, error) { result, err := pgquery.Parse(createTableDDL) if err != nil { return "", fmt.Errorf("parse CREATE TABLE %q: %w", statementPreview(createTableDDL), err) diff --git a/pkg/ddl/postgres_parser_test.go b/pkg/ddl/postgres_parser_test.go index bfd7e54d5..338f98d18 100644 --- a/pkg/ddl/postgres_parser_test.go +++ b/pkg/ddl/postgres_parser_test.go @@ -5,9 +5,12 @@ import ( "strings" "testing" + pgproto "github.com/pganalyze/pg_query_go/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" pgquery "github.com/wasilibs/go-pgquery" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoreflect" "github.com/block/schemabot/pkg/schema" ) @@ -210,7 +213,9 @@ func TestPostgresParserCreateTableColumns(t *testing.T) { require.ErrorContains(t, err, "expected CREATE TABLE statement") } -func TestSynthesizePostgresAddColumn(t *testing.T) { +func TestPostgresParserSynthesizeAddColumn(t *testing.T) { + p := postgresStatementParser{} + tests := []struct { name string createDDL string @@ -223,37 +228,56 @@ func TestSynthesizePostgresAddColumn(t *testing.T) { {"timestamp function default", "CREATE TABLE users (updated_at timestamptz DEFAULT now())", "updated_at", "ALTER TABLE users ADD COLUMN updated_at timestamptz DEFAULT now()"}, {"schema-qualified table", "CREATE TABLE app.users (id bigint)", "id", "ALTER TABLE app.users ADD COLUMN id bigint"}, {"quoted identifiers", `CREATE TABLE "App"."UserProfiles" ("DisplayName" varchar(255) NOT NULL)`, "DisplayName", `ALTER TABLE "App"."UserProfiles" ADD COLUMN "DisplayName" varchar(255) NOT NULL`}, + {"collation", `CREATE TABLE users (name text COLLATE "C")`, "name", `ALTER TABLE users ADD COLUMN name text COLLATE "C"`}, + {"identity", "CREATE TABLE users (id bigint GENERATED BY DEFAULT AS IDENTITY)", "id", "ALTER TABLE users ADD COLUMN id bigint GENERATED BY DEFAULT AS IDENTITY"}, + {"generated stored", "CREATE TABLE items (qty integer, price numeric, total numeric GENERATED ALWAYS AS (qty * price) STORED)", "total", "ALTER TABLE items ADD COLUMN total numeric GENERATED ALWAYS AS (qty * price) STORED"}, + {"array type", "CREATE TABLE users (tags text[])", "tags", "ALTER TABLE users ADD COLUMN tags text[]"}, + {"storage mode", "CREATE TABLE users (payload bytea STORAGE EXTERNAL)", "payload", "ALTER TABLE users ADD COLUMN payload bytea STORAGE external"}, + {"compression method", "CREATE TABLE users (document text COMPRESSION lz4)", "document", "ALTER TABLE users ADD COLUMN document text COMPRESSION lz4"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got, err := SynthesizePostgresAddColumn(tc.createDDL, tc.columnName) + got, err := p.SynthesizeAddColumn(tc.createDDL, tc.columnName) require.NoError(t, err) assert.Equal(t, tc.want, got) }) } + t.Run("table-level constraints are not carried", func(t *testing.T) { + got, err := p.SynthesizeAddColumn("CREATE TABLE users (id bigint, email text, PRIMARY KEY (id), UNIQUE (email), CHECK (id > 0))", "id") + require.NoError(t, err) + assert.Equal(t, "ALTER TABLE users ADD COLUMN id bigint", got) + }) + t.Run("column not found", func(t *testing.T) { - _, err := SynthesizePostgresAddColumn("CREATE TABLE users (id bigint)", "email") + _, err := p.SynthesizeAddColumn("CREATE TABLE users (id bigint)", "email") require.ErrorContains(t, err, `column "email" not found`) }) t.Run("multiple statements", func(t *testing.T) { - _, err := SynthesizePostgresAddColumn("CREATE TABLE users (id bigint); CREATE TABLE teams (id bigint)", "id") + _, err := p.SynthesizeAddColumn("CREATE TABLE users (id bigint); CREATE TABLE teams (id bigint)", "id") require.ErrorContains(t, err, "expected one CREATE TABLE statement, got 2") }) t.Run("not a CREATE TABLE", func(t *testing.T) { - _, err := SynthesizePostgresAddColumn("ALTER TABLE users ADD COLUMN email text", "email") + _, err := p.SynthesizeAddColumn("ALTER TABLE users ADD COLUMN email text", "email") require.ErrorContains(t, err, "expected CREATE TABLE statement") }) t.Run("parse failure", func(t *testing.T) { - _, err := SynthesizePostgresAddColumn("CREATE TABLE users (", "email") + _, err := p.SynthesizeAddColumn("CREATE TABLE users (", "email") require.ErrorContains(t, err, "parse CREATE TABLE") }) } -func TestSynthesizePostgresAddColumn_EmbeddedSchema(t *testing.T) { +// Every column of every CREATE TABLE in the embedded storage schema must +// round-trip through synthesis with its declaration intact: the ColumnDef +// carried by the synthesized ALTER must equal the ColumnDef declared in the +// CREATE TABLE, node for node, so no type, default, NOT NULL, identity, or +// collation clause can be silently dropped. This proves faithfulness only; it +// says nothing about whether PostgreSQL would accept the statement on a +// populated table, which is the caller's judgment. +func TestPostgresParserSynthesizeAddColumn_EmbeddedSchemaRoundTrips(t *testing.T) { files, err := fs.Glob(schema.PostgresFS, "postgres/*.sql") require.NoError(t, err) require.NotEmpty(t, files) @@ -265,19 +289,93 @@ func TestSynthesizePostgresAddColumn_EmbeddedSchema(t *testing.T) { statements, err := p.Split(string(content)) require.NoError(t, err, "split %s", file) require.NotEmpty(t, statements, "schema file %s", file) - columns, err := p.CreateTableColumns(statements[0]) - require.NoError(t, err, "columns in %s", file) - - for _, column := range columns { - ddl, err := SynthesizePostgresAddColumn(statements[0], column) - require.NoError(t, err, "%s column %s", file, column) - parsed, err := pgquery.Parse(ddl) - require.NoError(t, err, "%s column %s: %s", file, column, ddl) - require.Len(t, parsed.GetStmts(), 1, "%s column %s", file, column) + + createTables := 0 + for _, stmt := range statements { + kind, _, err := p.Classify(stmt) + require.NoError(t, err, "classify in %s", file) + if kind != StatementCreateTable { + continue + } + createTables++ + columns, err := p.CreateTableColumns(stmt) + require.NoError(t, err, "columns in %s", file) + + for _, column := range columns { + ddl, err := p.SynthesizeAddColumn(stmt, column) + require.NoError(t, err, "%s column %s", file, column) + want := columnDefFromCreateTable(t, stmt, column) + got := columnDefFromAddColumn(t, ddl) + clearParseLocations(want.ProtoReflect()) + clearParseLocations(got.ProtoReflect()) + assert.True(t, proto.Equal(want, got), + "%s column %s: synthesized ColumnDef diverges from the CREATE TABLE declaration\nsynthesized: %s\nwant: %v\ngot: %v", + file, column, ddl, want, got) + } } + require.Positive(t, createTables, "schema file %s declares no CREATE TABLE", file) } } +// columnDefFromCreateTable returns the named column's ColumnDef parse node +// from a single CREATE TABLE statement. +func columnDefFromCreateTable(t *testing.T, createDDL, column string) *pgproto.ColumnDef { + t.Helper() + parsed, err := pgquery.Parse(createDDL) + require.NoError(t, err) + require.Len(t, parsed.GetStmts(), 1) + create, ok := parsed.GetStmts()[0].GetStmt().GetNode().(*pgproto.Node_CreateStmt) + require.True(t, ok, "expected CREATE TABLE, got %q", createDDL) + for _, element := range create.CreateStmt.GetTableElts() { + def, ok := element.GetNode().(*pgproto.Node_ColumnDef) + if ok && def.ColumnDef.GetColname() == column { + return def.ColumnDef + } + } + t.Fatalf("column %q not found in %q", column, createDDL) + return nil +} + +// columnDefFromAddColumn returns the ColumnDef parse node carried by a +// single-command ALTER TABLE ... ADD COLUMN statement. +func columnDefFromAddColumn(t *testing.T, alterDDL string) *pgproto.ColumnDef { + t.Helper() + parsed, err := pgquery.Parse(alterDDL) + require.NoError(t, err, "parse %q", alterDDL) + require.Len(t, parsed.GetStmts(), 1, "statement %q", alterDDL) + alter, ok := parsed.GetStmts()[0].GetStmt().GetNode().(*pgproto.Node_AlterTableStmt) + require.True(t, ok, "expected ALTER TABLE, got %q", alterDDL) + require.Len(t, alter.AlterTableStmt.GetCmds(), 1, "statement %q", alterDDL) + cmd, ok := alter.AlterTableStmt.GetCmds()[0].GetNode().(*pgproto.Node_AlterTableCmd) + require.True(t, ok, "expected ALTER TABLE command in %q", alterDDL) + require.Equal(t, pgproto.AlterTableType_AT_AddColumn, cmd.AlterTableCmd.GetSubtype(), "statement %q", alterDDL) + def, ok := cmd.AlterTableCmd.GetDef().GetNode().(*pgproto.Node_ColumnDef) + require.True(t, ok, "expected a ColumnDef in %q", alterDDL) + return def.ColumnDef +} + +// clearParseLocations recursively zeroes every source-text offset field in a +// parse tree, so trees parsed from different statement texts compare equal on +// structure alone. +func clearParseLocations(m protoreflect.Message) { + m.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool { + switch { + case fd.IsList(): + if fd.Kind() == protoreflect.MessageKind { + list := v.List() + for i := 0; i < list.Len(); i++ { + clearParseLocations(list.Get(i).Message()) + } + } + case fd.Kind() == protoreflect.MessageKind: + clearParseLocations(v.Message()) + case strings.Contains(string(fd.Name()), "location"): + m.Clear(fd) + } + return true + }) +} + func TestPostgresParserCreateIndex(t *testing.T) { p := postgresStatementParser{}