From fd2dcff168b716c560312e32615ce8e76c2be715 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Tue, 23 Jun 2026 01:34:53 +0800 Subject: [PATCH 1/2] fix: order function recreation after dependent view recreation (#480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a function's RETURNS/parameter type references a view that must be recreated (DROP + CREATE) and the function's signature also changes, the function was being recreated in the create phase — before the view's DROP in the modify phase. The freshly-created function re-pinned the old view, so the subsequent DROP VIEW ... RESTRICT failed with SQLSTATE 2BP01. The function's DROP already runs in the drop phase, so deferring its CREATE to the modify phase (after generateModifyViewsSQL recreates the view) leaves the view free of dependents during its RESTRICT drop. Added functions whose type references a view with RequiresRecreate are pulled out of the create phase into functionsAwaitingRecreatedViews and emitted after view recreation. Fixes #480 Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/diff/diff.go | 49 +++++++++++++++++ .../issue_480_view_function_recreate/diff.sql | 32 +++++++++++ .../issue_480_view_function_recreate/new.sql | 25 +++++++++ .../issue_480_view_function_recreate/old.sql | 24 +++++++++ .../plan.json | 44 +++++++++++++++ .../issue_480_view_function_recreate/plan.sql | 32 +++++++++++ .../issue_480_view_function_recreate/plan.txt | 53 +++++++++++++++++++ 7 files changed, 259 insertions(+) create mode 100644 testdata/diff/dependency/issue_480_view_function_recreate/diff.sql create mode 100644 testdata/diff/dependency/issue_480_view_function_recreate/new.sql create mode 100644 testdata/diff/dependency/issue_480_view_function_recreate/old.sql create mode 100644 testdata/diff/dependency/issue_480_view_function_recreate/plan.json create mode 100644 testdata/diff/dependency/issue_480_view_function_recreate/plan.sql create mode 100644 testdata/diff/dependency/issue_480_view_function_recreate/plan.txt diff --git a/internal/diff/diff.go b/internal/diff/diff.go index a526fee6..8b33c5be 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -310,6 +310,12 @@ type ddlDiff struct { // exist when the view body is parsed (issue #414). deferredAddedViews []*ir.View functionsAwaitingDeferredViews []*ir.Function + // Newly-added functions whose return/parameter type references a view being + // recreated (DROP + CREATE) by this migration. Their old definition is dropped + // in the drop phase; creating them in the create phase (before the view's DROP) + // would re-pin the old view and block its RESTRICT drop. They are created in the + // modify phase, AFTER generateModifyViewsSQL recreates the view (issue #480). + functionsAwaitingRecreatedViews []*ir.Function // Foreign keys that depend on a unique/PK constraint being dropped or // recreated by this migration: existing ones are dropped before the table // modifications (fkPreDrops) and desired-state ones are (re)created @@ -1820,6 +1826,28 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto } } + // Functions whose return/parameter type references a view being recreated + // (DROP + CREATE in the modify phase) must be created AFTER that recreation. + // Their old definition was dropped in the drop phase; creating them now (before + // the view's DROP) would re-pin the old view and block its RESTRICT drop + // (issue #480). Pull them out of both buckets and defer to the modify phase. + recreatedViewLookup := buildRecreatedViewLookup(d.modifiedViews) + if len(recreatedViewLookup) > 0 { + deferRecreated := func(fns []*ir.Function) []*ir.Function { + var keep []*ir.Function + for _, fn := range fns { + if functionReferencesNewView(fn, recreatedViewLookup) { + d.functionsAwaitingRecreatedViews = append(d.functionsAwaitingRecreatedViews, fn) + } else { + keep = append(keep, fn) + } + } + return keep + } + functionsWithoutViewDeps = deferRecreated(functionsWithoutViewDeps) + functionsWithViewDeps = deferRecreated(functionsWithViewDeps) + } + // Create functions WITHOUT view dependencies (functions may depend on tables created above) generateCreateFunctionsSQL(functionsWithoutViewDeps, targetSchema, collector) @@ -1982,6 +2010,14 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto // Modify views - pass preDroppedViews to skip DROP for already-dropped views generateModifyViewsSQL(d.modifiedViews, targetSchema, collector, preDroppedViews, dependentViewsCtx, recreatedViews) + // Create functions deferred from generateCreateSQL because their return/parameter + // type references a view just recreated above. Emitting them now (rather than in + // the create phase) keeps the recreated view free of dependents during its + // RESTRICT drop (issue #480). + if len(d.functionsAwaitingRecreatedViews) > 0 { + generateCreateFunctionsSQL(d.functionsAwaitingRecreatedViews, targetSchema, collector) + } + // Modify functions generateModifyFunctionsSQL(d.modifiedFunctions, targetSchema, collector) @@ -2269,6 +2305,19 @@ func buildViewLookup(views []*ir.View) map[string]struct{} { return buildSchemaNameLookup(names) } +// buildRecreatedViewLookup returns case-insensitive lookup keys for views being +// recreated (DROP + CREATE) by this migration, i.e. modified views with +// RequiresRecreate set. +func buildRecreatedViewLookup(modifiedViews []*viewDiff) map[string]struct{} { + var names []struct{ schema, name string } + for _, vd := range modifiedViews { + if vd.RequiresRecreate { + names = append(names, struct{ schema, name string }{vd.New.Schema, vd.New.Name}) + } + } + return buildSchemaNameLookup(names) +} + // functionReferencesNewView determines if a function references any newly added views // in its return type or parameter types. This handles cases where functions use // view composite types (e.g., RETURNS SETOF view_name or parameter of view_name type). diff --git a/testdata/diff/dependency/issue_480_view_function_recreate/diff.sql b/testdata/diff/dependency/issue_480_view_function_recreate/diff.sql new file mode 100644 index 00000000..cc884fb9 --- /dev/null +++ b/testdata/diff/dependency/issue_480_view_function_recreate/diff.sql @@ -0,0 +1,32 @@ +DROP FUNCTION IF EXISTS fn_create_user(text); + +ALTER TABLE tb_users ADD COLUMN role text DEFAULT 'member' NOT NULL; + +DROP VIEW IF EXISTS vw_users RESTRICT; + +CREATE OR REPLACE VIEW vw_users AS + SELECT id, + email, + role, + created_at + FROM tb_users + WHERE is_deleted = false; + +CREATE OR REPLACE FUNCTION fn_create_user( + email text, + role text DEFAULT 'member' +) +RETURNS vw_users +LANGUAGE plpgsql +VOLATILE +SECURITY DEFINER +AS $$ +DECLARE + v_result vw_users; + v_new_id UUID; +BEGIN + INSERT INTO tb_users (email, role) VALUES (email, role) RETURNING id INTO v_new_id; + SELECT id, email, role, created_at INTO v_result FROM vw_users WHERE id = v_new_id; + RETURN v_result; +END; +$$; diff --git a/testdata/diff/dependency/issue_480_view_function_recreate/new.sql b/testdata/diff/dependency/issue_480_view_function_recreate/new.sql new file mode 100644 index 00000000..7348b6c8 --- /dev/null +++ b/testdata/diff/dependency/issue_480_view_function_recreate/new.sql @@ -0,0 +1,25 @@ +CREATE TABLE tb_users ( + id uuid DEFAULT gen_random_uuid(), + is_deleted boolean DEFAULT false NOT NULL, + created_at timestamptz DEFAULT now(), + email text DEFAULT 'missing@missing.com' NOT NULL, + role text DEFAULT 'member' NOT NULL, + CONSTRAINT tb_users_pkey PRIMARY KEY (id) +); + +CREATE VIEW vw_users AS +SELECT id, email, role, created_at +FROM tb_users +WHERE is_deleted = FALSE; + +CREATE FUNCTION fn_create_user(email TEXT, role TEXT DEFAULT 'member') +RETURNS vw_users AS $$ +DECLARE + v_result vw_users; + v_new_id UUID; +BEGIN + INSERT INTO tb_users (email, role) VALUES (email, role) RETURNING id INTO v_new_id; + SELECT id, email, role, created_at INTO v_result FROM vw_users WHERE id = v_new_id; + RETURN v_result; +END; +$$ LANGUAGE plpgsql SECURITY DEFINER; diff --git a/testdata/diff/dependency/issue_480_view_function_recreate/old.sql b/testdata/diff/dependency/issue_480_view_function_recreate/old.sql new file mode 100644 index 00000000..4460bae6 --- /dev/null +++ b/testdata/diff/dependency/issue_480_view_function_recreate/old.sql @@ -0,0 +1,24 @@ +CREATE TABLE tb_users ( + id uuid DEFAULT gen_random_uuid(), + is_deleted boolean DEFAULT false NOT NULL, + created_at timestamptz DEFAULT now(), + email text DEFAULT 'missing@missing.com' NOT NULL, + CONSTRAINT tb_users_pkey PRIMARY KEY (id) +); + +CREATE VIEW vw_users AS +SELECT id, email, created_at +FROM tb_users +WHERE is_deleted = FALSE; + +CREATE FUNCTION fn_create_user(email TEXT) +RETURNS vw_users AS $$ +DECLARE + v_result vw_users; + v_new_id UUID; +BEGIN + INSERT INTO tb_users (email) VALUES (email) RETURNING id INTO v_new_id; + SELECT id, email, created_at INTO v_result FROM vw_users WHERE id = v_new_id; + RETURN v_result; +END; +$$ LANGUAGE plpgsql SECURITY DEFINER; diff --git a/testdata/diff/dependency/issue_480_view_function_recreate/plan.json b/testdata/diff/dependency/issue_480_view_function_recreate/plan.json new file mode 100644 index 00000000..3232c371 --- /dev/null +++ b/testdata/diff/dependency/issue_480_view_function_recreate/plan.json @@ -0,0 +1,44 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.11.0", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "c7ede31c4d93f537192a343c3b52ae33b76c573082ad077bfd20a09419482c9d" + }, + "groups": [ + { + "steps": [ + { + "sql": "DROP FUNCTION IF EXISTS fn_create_user(text);", + "type": "function", + "operation": "drop", + "path": "public.fn_create_user" + }, + { + "sql": "ALTER TABLE tb_users ADD COLUMN role text DEFAULT 'member' NOT NULL;", + "type": "table.column", + "operation": "create", + "path": "public.tb_users.role" + }, + { + "sql": "DROP VIEW IF EXISTS vw_users RESTRICT;", + "type": "view", + "operation": "alter", + "path": "public.vw_users" + }, + { + "sql": "CREATE OR REPLACE VIEW vw_users AS\n SELECT id,\n email,\n role,\n created_at\n FROM tb_users\n WHERE is_deleted = false;", + "type": "view", + "operation": "alter", + "path": "public.vw_users" + }, + { + "sql": "CREATE OR REPLACE FUNCTION fn_create_user(\n email text,\n role text DEFAULT 'member'\n)\nRETURNS vw_users\nLANGUAGE plpgsql\nVOLATILE\nSECURITY DEFINER\nAS $$\nDECLARE\n v_result vw_users;\n v_new_id UUID;\nBEGIN\n INSERT INTO tb_users (email, role) VALUES (email, role) RETURNING id INTO v_new_id;\n SELECT id, email, role, created_at INTO v_result FROM vw_users WHERE id = v_new_id;\n RETURN v_result;\nEND;\n$$;", + "type": "function", + "operation": "create", + "path": "public.fn_create_user" + } + ] + } + ] +} diff --git a/testdata/diff/dependency/issue_480_view_function_recreate/plan.sql b/testdata/diff/dependency/issue_480_view_function_recreate/plan.sql new file mode 100644 index 00000000..cc884fb9 --- /dev/null +++ b/testdata/diff/dependency/issue_480_view_function_recreate/plan.sql @@ -0,0 +1,32 @@ +DROP FUNCTION IF EXISTS fn_create_user(text); + +ALTER TABLE tb_users ADD COLUMN role text DEFAULT 'member' NOT NULL; + +DROP VIEW IF EXISTS vw_users RESTRICT; + +CREATE OR REPLACE VIEW vw_users AS + SELECT id, + email, + role, + created_at + FROM tb_users + WHERE is_deleted = false; + +CREATE OR REPLACE FUNCTION fn_create_user( + email text, + role text DEFAULT 'member' +) +RETURNS vw_users +LANGUAGE plpgsql +VOLATILE +SECURITY DEFINER +AS $$ +DECLARE + v_result vw_users; + v_new_id UUID; +BEGIN + INSERT INTO tb_users (email, role) VALUES (email, role) RETURNING id INTO v_new_id; + SELECT id, email, role, created_at INTO v_result FROM vw_users WHERE id = v_new_id; + RETURN v_result; +END; +$$; diff --git a/testdata/diff/dependency/issue_480_view_function_recreate/plan.txt b/testdata/diff/dependency/issue_480_view_function_recreate/plan.txt new file mode 100644 index 00000000..a71c1dc4 --- /dev/null +++ b/testdata/diff/dependency/issue_480_view_function_recreate/plan.txt @@ -0,0 +1,53 @@ +Plan: 1 to add, 2 to modify, 1 to drop. + +Summary by type: + functions: 1 to add, 1 to drop + tables: 1 to modify + views: 1 to modify + +Functions: + - fn_create_user + + fn_create_user + +Tables: + ~ tb_users + + role (column) + +Views: + ~ vw_users + +DDL to be executed: +-------------------------------------------------- + +DROP FUNCTION IF EXISTS fn_create_user(text); + +ALTER TABLE tb_users ADD COLUMN role text DEFAULT 'member' NOT NULL; + +DROP VIEW IF EXISTS vw_users RESTRICT; + +CREATE OR REPLACE VIEW vw_users AS + SELECT id, + email, + role, + created_at + FROM tb_users + WHERE is_deleted = false; + +CREATE OR REPLACE FUNCTION fn_create_user( + email text, + role text DEFAULT 'member' +) +RETURNS vw_users +LANGUAGE plpgsql +VOLATILE +SECURITY DEFINER +AS $$ +DECLARE + v_result vw_users; + v_new_id UUID; +BEGIN + INSERT INTO tb_users (email, role) VALUES (email, role) RETURNING id INTO v_new_id; + SELECT id, email, role, created_at INTO v_result FROM vw_users WHERE id = v_new_id; + RETURN v_result; +END; +$$; From 1bb51619920878ddd3a65339fc8996890fce4d5f Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Tue, 23 Jun 2026 01:41:41 +0800 Subject: [PATCH 2/2] docs: clarify deferred-function comment covers new functions too (#480) Address review feedback: the functionsAwaitingRecreatedViews bucket can also hold genuinely new functions (no old definition), so qualify the "old definition dropped in the drop phase" reasoning to signature-changed functions. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/diff/diff.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 8b33c5be..c101c713 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -310,11 +310,12 @@ type ddlDiff struct { // exist when the view body is parsed (issue #414). deferredAddedViews []*ir.View functionsAwaitingDeferredViews []*ir.Function - // Newly-added functions whose return/parameter type references a view being - // recreated (DROP + CREATE) by this migration. Their old definition is dropped - // in the drop phase; creating them in the create phase (before the view's DROP) - // would re-pin the old view and block its RESTRICT drop. They are created in the - // modify phase, AFTER generateModifyViewsSQL recreates the view (issue #480). + // Added functions whose return/parameter type references a view being recreated + // (DROP + CREATE) by this migration. For a function whose signature changed, its + // old definition is dropped in the drop phase; creating the new one in the create + // phase (before the view's DROP) would re-pin the old view and block its RESTRICT + // drop. They are created in the modify phase, AFTER generateModifyViewsSQL + // recreates the view (issue #480). functionsAwaitingRecreatedViews []*ir.Function // Foreign keys that depend on a unique/PK constraint being dropped or // recreated by this migration: existing ones are dropped before the table @@ -1828,9 +1829,10 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto // Functions whose return/parameter type references a view being recreated // (DROP + CREATE in the modify phase) must be created AFTER that recreation. - // Their old definition was dropped in the drop phase; creating them now (before - // the view's DROP) would re-pin the old view and block its RESTRICT drop - // (issue #480). Pull them out of both buckets and defer to the modify phase. + // For a signature-changed function the old definition was dropped in the drop + // phase; creating the new one now (before the view's DROP) would re-pin the old + // view and block its RESTRICT drop (issue #480). Pull them out of both buckets + // and defer to the modify phase. recreatedViewLookup := buildRecreatedViewLookup(d.modifiedViews) if len(recreatedViewLookup) > 0 { deferRecreated := func(fns []*ir.Function) []*ir.Function {