Skip to content

Commit 1f66df0

Browse files
PadenZachclaude
andauthored
fix: order CREATE VIEW after ALTER TABLE ADD COLUMN within a plan (#414) (#417)
* fix: order CREATE VIEW after ALTER TABLE ADD COLUMN within a plan (#414) When a single plan both adds a column to an existing table and creates a new view that references that column, pgschema previously emitted the CREATE VIEW before the ALTER TABLE ADD COLUMN. Both ended up in the same implicit transaction group, so PostgreSQL aborted with `42703 column "<col>" does not exist`. Defer creation of newly-added views (and any functions whose view dependency targets one of them) whose definition references a newly-added column on a modified table. Deferred views are emitted immediately after generateModifyTablesSQL, so the columns exist by the time the view body is parsed. Mirrors the existing `tablesWithDeps`/`functionsWithViewDeps` deferral pattern. Fixes #414 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup! fix: order CREATE VIEW after ALTER TABLE ADD COLUMN within a plan (#414) ammended previous commit, creating doc only change to trigger greptile --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 86079e6 commit 1f66df0

20 files changed

Lines changed: 430 additions & 2 deletions

File tree

internal/diff/diff.go

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,11 @@ type ddlDiff struct {
296296
addedColumnPrivileges []*ir.ColumnPrivilege
297297
droppedColumnPrivileges []*ir.ColumnPrivilege
298298
modifiedColumnPrivileges []*columnPrivilegeDiff
299+
// Newly-added views that reference newly-added columns on modified tables.
300+
// Created in the modify phase, AFTER generateModifyTablesSQL, so the columns
301+
// exist when the view body is parsed (issue #414).
302+
deferredAddedViews []*ir.View
303+
functionsAwaitingDeferredViews []*ir.Function
299304
}
300305

301306
// schemaDiff represents changes to a schema
@@ -1615,8 +1620,61 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto
16151620
// Note: We need to create triggers for ALL tables, not just the original d.addedTables
16161621
generateCreateTriggersFromTables(d.addedTables, targetSchema, collector)
16171622

1618-
// Create views
1619-
generateCreateViewsSQL(d.addedViews, targetSchema, collector)
1623+
// Create views, deferring any whose body references a newly-added column on a
1624+
// modified table. Those columns are emitted by generateModifyTablesSQL during
1625+
// the modify phase, so deferred views are created there (issue #414)
1626+
addedColLookup := buildModifiedTableAddedColumnLookup(d.modifiedTables)
1627+
viewsToCreateNow := d.addedViews
1628+
if len(addedColLookup) > 0 {
1629+
viewsToCreateNow = nil
1630+
for _, v := range d.addedViews {
1631+
if viewReferencesAddedColumn(v, addedColLookup) {
1632+
d.deferredAddedViews = append(d.deferredAddedViews, v)
1633+
} else {
1634+
viewsToCreateNow = append(viewsToCreateNow, v)
1635+
}
1636+
}
1637+
1638+
// Transitive closure: also defer any view whose body references a view
1639+
// already in deferredAddedViews. Iterate to fixpoint so chains of any
1640+
// length (V3 -> V2 -> V1 -> added column) move together. Walking
1641+
// viewsToCreateNow in order preserves topological ordering on each pass.
1642+
// Each iteration reads d.deferredAddedViews fresh, so a view appended
1643+
// during this pass is visible to the very next sibling examined — that
1644+
// is what lets a topo-sorted chain drain in a single pass.
1645+
for {
1646+
var stillNow []*ir.View
1647+
added := false
1648+
for _, v := range viewsToCreateNow {
1649+
if viewReferencesAnyDeferredView(v, d.deferredAddedViews) {
1650+
d.deferredAddedViews = append(d.deferredAddedViews, v)
1651+
added = true
1652+
} else {
1653+
stillNow = append(stillNow, v)
1654+
}
1655+
}
1656+
viewsToCreateNow = stillNow
1657+
if !added {
1658+
break
1659+
}
1660+
}
1661+
}
1662+
generateCreateViewsSQL(viewsToCreateNow, targetSchema, collector)
1663+
1664+
// If any views were deferred, also defer functions whose view dependency is
1665+
// on those deferred views — they must be created after the views exist.
1666+
if len(d.deferredAddedViews) > 0 {
1667+
deferredViewLookup := buildViewLookup(d.deferredAddedViews)
1668+
var keepNow []*ir.Function
1669+
for _, fn := range functionsWithViewDeps {
1670+
if functionReferencesNewView(fn, deferredViewLookup) {
1671+
d.functionsAwaitingDeferredViews = append(d.functionsAwaitingDeferredViews, fn)
1672+
} else {
1673+
keepNow = append(keepNow, fn)
1674+
}
1675+
}
1676+
functionsWithViewDeps = keepNow
1677+
}
16201678

16211679
// Create functions WITH view dependencies (now that views exist)
16221680
// These functions reference views in their return type or parameter types (issue #300)
@@ -1652,6 +1710,16 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto
16521710
// Modify tables
16531711
generateModifyTablesSQL(d.modifiedTables, d.droppedTables, targetSchema, collector)
16541712

1713+
// Create views deferred from generateCreateSQL — their bodies reference
1714+
// columns just added by ALTER TABLE above (issue #414). Likewise, emit
1715+
// any functions whose view dependency was on those deferred views.
1716+
if len(d.deferredAddedViews) > 0 {
1717+
generateCreateViewsSQL(d.deferredAddedViews, targetSchema, collector)
1718+
}
1719+
if len(d.functionsAwaitingDeferredViews) > 0 {
1720+
generateCreateFunctionsSQL(d.functionsAwaitingDeferredViews, targetSchema, collector)
1721+
}
1722+
16551723
// Find views that depend on views being recreated (issue #268, #308)
16561724
// Handles both materialized views and regular views with RequiresRecreate
16571725
// Exclude newly added views - they will be created in CREATE phase after recreated views

internal/diff/view.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,67 @@ func viewDependsOnTable(view *ir.View, tableSchema, tableName string) bool {
769769
return false
770770
}
771771

772+
// buildModifiedTableAddedColumnLookup returns a map of lowercased schema.tableName
773+
// to a set of lowercased column names being added by ALTER TABLE on that table.
774+
func buildModifiedTableAddedColumnLookup(modifiedTables []*tableDiff) map[string]map[string]struct{} {
775+
lookup := make(map[string]map[string]struct{})
776+
for _, td := range modifiedTables {
777+
if len(td.AddedColumns) == 0 {
778+
continue
779+
}
780+
key := strings.ToLower(td.Table.Schema + "." + td.Table.Name)
781+
cols := make(map[string]struct{}, len(td.AddedColumns))
782+
for _, c := range td.AddedColumns {
783+
cols[strings.ToLower(c.Name)] = struct{}{}
784+
}
785+
lookup[key] = cols
786+
}
787+
return lookup
788+
}
789+
790+
// viewReferencesAnyDeferredView reports whether the view's body references any
791+
// of the provided deferred views by name. Used for transitive deferral so that
792+
// view chains (V2 -> V1 -> added column) move together to the modify phase.
793+
func viewReferencesAnyDeferredView(view *ir.View, deferred []*ir.View) bool {
794+
if view == nil || view.Definition == "" || len(deferred) == 0 {
795+
return false
796+
}
797+
for _, dv := range deferred {
798+
if viewDependsOnView(view, dv.Name) {
799+
return true
800+
}
801+
if dv.Schema != "" && viewDependsOnView(view, dv.Schema+"."+dv.Name) {
802+
return true
803+
}
804+
}
805+
return false
806+
}
807+
808+
// viewReferencesAddedColumn reports whether the view's definition references
809+
// any modified table AND at least one of the columns being added to that table.
810+
// Both checks are required to avoid deferring views that simply happen to
811+
// mention a column name being added to an unrelated table.
812+
func viewReferencesAddedColumn(view *ir.View, addedCols map[string]map[string]struct{}) bool {
813+
if view == nil || view.Definition == "" || len(addedCols) == 0 {
814+
return false
815+
}
816+
for tableKey, cols := range addedCols {
817+
parts := strings.SplitN(tableKey, ".", 2)
818+
if len(parts) != 2 {
819+
continue
820+
}
821+
if !viewDependsOnTable(view, parts[0], parts[1]) {
822+
continue
823+
}
824+
for col := range cols {
825+
if containsIdentifier(view.Definition, col) {
826+
return true
827+
}
828+
}
829+
}
830+
return false
831+
}
832+
772833
// dependentViewsContext tracks views that depend on views being recreated
773834
type dependentViewsContext struct {
774835
// dependents maps view key (schema.name) to list of dependent views
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
ALTER TABLE foo ADD COLUMN run_id uuid;
2+
3+
CREATE OR REPLACE VIEW foo_base AS
4+
SELECT id,
5+
run_id
6+
FROM foo
7+
WHERE run_id IS NOT NULL;
8+
9+
CREATE OR REPLACE VIEW foo_summary AS
10+
SELECT id
11+
FROM foo_base;
12+
13+
CREATE OR REPLACE FUNCTION get_foo_summary()
14+
RETURNS SETOF foo_summary
15+
LANGUAGE sql
16+
STABLE
17+
AS $$ SELECT * FROM foo_summary
18+
$$;
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
CREATE TABLE foo (
2+
id bigint PRIMARY KEY,
3+
run_id uuid
4+
);
5+
6+
CREATE OR REPLACE VIEW foo_base AS
7+
SELECT id, run_id FROM foo WHERE run_id IS NOT NULL;
8+
9+
CREATE OR REPLACE VIEW foo_summary AS
10+
SELECT id FROM foo_base;
11+
12+
CREATE OR REPLACE FUNCTION get_foo_summary()
13+
RETURNS SETOF foo_summary
14+
LANGUAGE sql STABLE
15+
AS $$ SELECT * FROM foo_summary $$;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
CREATE TABLE foo (
2+
id bigint PRIMARY KEY
3+
);
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"version": "1.0.0",
3+
"pgschema_version": "1.9.0",
4+
"created_at": "1970-01-01T00:00:00Z",
5+
"source_fingerprint": {
6+
"hash": "e1fb0e7b8fda0362df6ecdbc88f6910f1faaa4d896de95796aa412b913e18858"
7+
},
8+
"groups": [
9+
{
10+
"steps": [
11+
{
12+
"sql": "ALTER TABLE foo ADD COLUMN run_id uuid;",
13+
"type": "table.column",
14+
"operation": "create",
15+
"path": "public.foo.run_id"
16+
},
17+
{
18+
"sql": "CREATE OR REPLACE VIEW foo_base AS\n SELECT id,\n run_id\n FROM foo\n WHERE run_id IS NOT NULL;",
19+
"type": "view",
20+
"operation": "create",
21+
"path": "public.foo_base"
22+
},
23+
{
24+
"sql": "CREATE OR REPLACE VIEW foo_summary AS\n SELECT id\n FROM foo_base;",
25+
"type": "view",
26+
"operation": "create",
27+
"path": "public.foo_summary"
28+
},
29+
{
30+
"sql": "CREATE OR REPLACE FUNCTION get_foo_summary()\nRETURNS SETOF foo_summary\nLANGUAGE sql\nSTABLE\nAS $$ SELECT * FROM foo_summary\n$$;",
31+
"type": "function",
32+
"operation": "create",
33+
"path": "public.get_foo_summary"
34+
}
35+
]
36+
}
37+
]
38+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
ALTER TABLE foo ADD COLUMN run_id uuid;
2+
3+
CREATE OR REPLACE VIEW foo_base AS
4+
SELECT id,
5+
run_id
6+
FROM foo
7+
WHERE run_id IS NOT NULL;
8+
9+
CREATE OR REPLACE VIEW foo_summary AS
10+
SELECT id
11+
FROM foo_base;
12+
13+
CREATE OR REPLACE FUNCTION get_foo_summary()
14+
RETURNS SETOF foo_summary
15+
LANGUAGE sql
16+
STABLE
17+
AS $$ SELECT * FROM foo_summary
18+
$$;
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
Plan: 3 to add, 1 to modify.
2+
3+
Summary by type:
4+
functions: 1 to add
5+
tables: 1 to modify
6+
views: 2 to add
7+
8+
Functions:
9+
+ get_foo_summary
10+
11+
Tables:
12+
~ foo
13+
+ run_id (column)
14+
15+
Views:
16+
+ foo_base
17+
+ foo_summary
18+
19+
DDL to be executed:
20+
--------------------------------------------------
21+
22+
ALTER TABLE foo ADD COLUMN run_id uuid;
23+
24+
CREATE OR REPLACE VIEW foo_base AS
25+
SELECT id,
26+
run_id
27+
FROM foo
28+
WHERE run_id IS NOT NULL;
29+
30+
CREATE OR REPLACE VIEW foo_summary AS
31+
SELECT id
32+
FROM foo_base;
33+
34+
CREATE OR REPLACE FUNCTION get_foo_summary()
35+
RETURNS SETOF foo_summary
36+
LANGUAGE sql
37+
STABLE
38+
AS $$ SELECT * FROM foo_summary
39+
$$;
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
ALTER TABLE foo ADD COLUMN run_id uuid;
2+
3+
CREATE OR REPLACE VIEW foo_base AS
4+
SELECT id,
5+
run_id
6+
FROM foo
7+
WHERE run_id IS NOT NULL;
8+
9+
CREATE OR REPLACE VIEW foo_summary AS
10+
SELECT id
11+
FROM foo_base;
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
CREATE TABLE foo (
2+
id bigint PRIMARY KEY,
3+
run_id uuid
4+
);
5+
6+
CREATE OR REPLACE VIEW foo_base AS
7+
SELECT id, run_id FROM foo WHERE run_id IS NOT NULL;
8+
9+
CREATE OR REPLACE VIEW foo_summary AS
10+
SELECT id FROM foo_base;

0 commit comments

Comments
 (0)