From e2a43eea2c8e10c8e018c2f2b8ec86d4421bae36 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 7 Nov 2025 10:06:16 -0800 Subject: [PATCH] fix: drop trigger/function order --- internal/diff/diff.go | 3 ++ internal/diff/table.go | 18 ++------- internal/diff/trigger.go | 33 ++++++++++++++++ .../dependency/function_to_trigger/diff.sql | 21 ++++------ .../dependency/function_to_trigger/new.sql | 16 ++++---- .../dependency/function_to_trigger/old.sql | 24 +++++++++++- .../dependency/function_to_trigger/plan.json | 24 +++++++----- .../dependency/function_to_trigger/plan.sql | 22 ++++------- .../dependency/function_to_trigger/plan.txt | 38 +++++++++---------- 9 files changed, 118 insertions(+), 81 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index c17d7e75..0f121f23 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -963,6 +963,9 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto // generateDropSQL generates DROP statements in reverse dependency order func (d *ddlDiff) generateDropSQL(targetSchema string, collector *diffCollector) { + // Drop triggers from modified tables first (triggers depend on functions) + generateDropTriggersFromModifiedTables(d.modifiedTables, targetSchema, collector) + // Drop functions generateDropFunctionsSQL(d.droppedFunctions, targetSchema, collector) diff --git a/internal/diff/table.go b/internal/diff/table.go index 1be87df7..2073a397 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -491,6 +491,8 @@ func generateTableSQL(table *ir.Table, targetSchema string) string { } // generateAlterTableStatements generates SQL statements for table modifications +// Note: DroppedTriggers are skipped here because they are already processed in the DROP phase +// (see generateDropTriggersFromModifiedTables in trigger.go) func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector *diffCollector) { // Drop constraints first (before dropping columns) - already sorted by the Diff operation for _, constraint := range td.DroppedConstraints { @@ -821,20 +823,8 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector collector.collect(context, sql) } - // Drop triggers - already sorted by the Diff operation - for _, trigger := range td.DroppedTriggers { - tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema) - sql := fmt.Sprintf("DROP TRIGGER IF EXISTS %s ON %s;", trigger.Name, tableName) - - context := &diffContext{ - Type: DiffTypeTableTrigger, - Operation: DiffOperationDrop, - Path: fmt.Sprintf("%s.%s.%s", td.Table.Schema, td.Table.Name, trigger.Name), - Source: trigger, - CanRunInTransaction: true, - } - collector.collect(context, sql) - } + // Drop triggers - skipped here because they are already dropped in the DROP phase + // (see generateDropTriggersFromModifiedTables in trigger.go) // Add triggers - already sorted by the Diff operation for _, trigger := range td.AddedTriggers { diff --git a/internal/diff/trigger.go b/internal/diff/trigger.go index 859b7764..7873a54e 100644 --- a/internal/diff/trigger.go +++ b/internal/diff/trigger.go @@ -141,6 +141,39 @@ func generateCreateTriggersSQL(triggers []*ir.Trigger, targetSchema string, coll } } +// generateDropTriggersFromModifiedTables collects and drops all triggers from modified tables +// This ensures triggers are dropped before their associated functions +func generateDropTriggersFromModifiedTables(tables []*tableDiff, targetSchema string, collector *diffCollector) { + var allTriggers []*ir.Trigger + + // Collect all dropped triggers from modified tables + for _, tableDiff := range tables { + for _, trigger := range tableDiff.DroppedTriggers { + allTriggers = append(allTriggers, trigger) + } + } + + // Sort all triggers by name for consistent ordering + sort.Slice(allTriggers, func(i, j int) bool { + return allTriggers[i].Name < allTriggers[j].Name + }) + + // Generate DROP TRIGGER statements for all collected triggers + for _, trigger := range allTriggers { + tableName := getTableNameWithSchema(trigger.Schema, trigger.Table, targetSchema) + sql := fmt.Sprintf("DROP TRIGGER IF EXISTS %s ON %s;", trigger.Name, tableName) + + context := &diffContext{ + Type: DiffTypeTableTrigger, + Operation: DiffOperationDrop, + Path: fmt.Sprintf("%s.%s.%s", trigger.Schema, trigger.Table, trigger.Name), + Source: trigger, + CanRunInTransaction: true, + } + collector.collect(context, sql) + } +} + // generateTriggerSQLWithMode generates CREATE [OR REPLACE] TRIGGER or CREATE CONSTRAINT TRIGGER statement func generateTriggerSQLWithMode(trigger *ir.Trigger, targetSchema string) string { // Build event list in standard order: INSERT, UPDATE, DELETE, TRUNCATE diff --git a/testdata/diff/dependency/function_to_trigger/diff.sql b/testdata/diff/dependency/function_to_trigger/diff.sql index 118ef64e..30881a81 100644 --- a/testdata/diff/dependency/function_to_trigger/diff.sql +++ b/testdata/diff/dependency/function_to_trigger/diff.sql @@ -1,26 +1,19 @@ -CREATE TABLE IF NOT EXISTS users ( - id SERIAL, - name text NOT NULL, - email text, - created_at timestamp DEFAULT CURRENT_TIMESTAMP, - updated_at timestamp DEFAULT CURRENT_TIMESTAMP, - CONSTRAINT users_pkey PRIMARY KEY (id), - CONSTRAINT users_email_key UNIQUE (email) -); +DROP TRIGGER IF EXISTS update_users_modified_time ON users; +DROP FUNCTION IF EXISTS update_modified_time(); -CREATE OR REPLACE FUNCTION update_modified_time() +CREATE OR REPLACE FUNCTION log_user_changes() RETURNS trigger LANGUAGE plpgsql SECURITY INVOKER VOLATILE AS $$ BEGIN - NEW.updated_at = CURRENT_TIMESTAMP; + RAISE NOTICE 'User record changed: %', NEW.id; RETURN NEW; END; $$; -CREATE OR REPLACE TRIGGER update_users_modified_time - BEFORE UPDATE ON users +CREATE OR REPLACE TRIGGER log_users_trigger + AFTER INSERT OR UPDATE ON users FOR EACH ROW - EXECUTE FUNCTION update_modified_time(); + EXECUTE FUNCTION log_user_changes(); diff --git a/testdata/diff/dependency/function_to_trigger/new.sql b/testdata/diff/dependency/function_to_trigger/new.sql index 2a3a3262..e78c30c6 100644 --- a/testdata/diff/dependency/function_to_trigger/new.sql +++ b/testdata/diff/dependency/function_to_trigger/new.sql @@ -1,4 +1,4 @@ --- Table with a trigger that depends on the function +-- Table (unchanged) CREATE TABLE public.users ( id serial PRIMARY KEY, name text NOT NULL, @@ -7,17 +7,17 @@ CREATE TABLE public.users ( updated_at timestamp DEFAULT CURRENT_TIMESTAMP ); --- Function that will be used by the trigger -CREATE OR REPLACE FUNCTION public.update_modified_time() +-- Different function for logging changes +CREATE OR REPLACE FUNCTION public.log_user_changes() RETURNS trigger AS $$ BEGIN - NEW.updated_at = CURRENT_TIMESTAMP; + RAISE NOTICE 'User record changed: %', NEW.id; RETURN NEW; END; $$ LANGUAGE plpgsql; --- Trigger that depends on the function -CREATE TRIGGER update_users_modified_time - BEFORE UPDATE ON public.users +-- Different trigger that depends on the log function +CREATE TRIGGER log_users_trigger + AFTER INSERT OR UPDATE ON public.users FOR EACH ROW - EXECUTE FUNCTION public.update_modified_time(); \ No newline at end of file + EXECUTE FUNCTION public.log_user_changes(); diff --git a/testdata/diff/dependency/function_to_trigger/old.sql b/testdata/diff/dependency/function_to_trigger/old.sql index 89dd9564..60cdf48f 100644 --- a/testdata/diff/dependency/function_to_trigger/old.sql +++ b/testdata/diff/dependency/function_to_trigger/old.sql @@ -1 +1,23 @@ --- Empty schema (no objects) \ No newline at end of file +-- Table with a trigger that depends on the function +CREATE TABLE public.users ( + id serial PRIMARY KEY, + name text NOT NULL, + email text UNIQUE, + created_at timestamp DEFAULT CURRENT_TIMESTAMP, + updated_at timestamp DEFAULT CURRENT_TIMESTAMP +); + +-- Function that will be used by the trigger +CREATE OR REPLACE FUNCTION public.update_modified_time() +RETURNS trigger AS $$ +BEGIN + NEW.updated_at = CURRENT_TIMESTAMP; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +-- Trigger that depends on the function +CREATE TRIGGER update_users_modified_time + BEFORE UPDATE ON public.users + FOR EACH ROW + EXECUTE FUNCTION public.update_modified_time(); diff --git a/testdata/diff/dependency/function_to_trigger/plan.json b/testdata/diff/dependency/function_to_trigger/plan.json index 3a035898..4aafeb21 100644 --- a/testdata/diff/dependency/function_to_trigger/plan.json +++ b/testdata/diff/dependency/function_to_trigger/plan.json @@ -3,28 +3,34 @@ "pgschema_version": "1.4.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "965b1131737c955e24c7f827c55bd78e4cb49a75adfd04229e0ba297376f5085" + "hash": "838be5bff5331655f93ff85bf9e620d007f6c664b57889a1efb31343310534c8" }, "groups": [ { "steps": [ { - "sql": "CREATE TABLE IF NOT EXISTS users (\n id SERIAL,\n name text NOT NULL,\n email text,\n created_at timestamp DEFAULT CURRENT_TIMESTAMP,\n updated_at timestamp DEFAULT CURRENT_TIMESTAMP,\n CONSTRAINT users_pkey PRIMARY KEY (id),\n CONSTRAINT users_email_key UNIQUE (email)\n);", - "type": "table", - "operation": "create", - "path": "public.users" + "sql": "DROP TRIGGER IF EXISTS update_users_modified_time ON users;", + "type": "table.trigger", + "operation": "drop", + "path": "public.users.update_users_modified_time" }, { - "sql": "CREATE OR REPLACE FUNCTION update_modified_time()\nRETURNS trigger\nLANGUAGE plpgsql\nSECURITY INVOKER\nVOLATILE\nAS $$\nBEGIN\n NEW.updated_at = CURRENT_TIMESTAMP;\n RETURN NEW;\nEND;\n$$;", + "sql": "DROP FUNCTION IF EXISTS update_modified_time();", "type": "function", - "operation": "create", + "operation": "drop", "path": "public.update_modified_time" }, { - "sql": "CREATE OR REPLACE TRIGGER update_users_modified_time\n BEFORE UPDATE ON users\n FOR EACH ROW\n EXECUTE FUNCTION update_modified_time();", + "sql": "CREATE OR REPLACE FUNCTION log_user_changes()\nRETURNS trigger\nLANGUAGE plpgsql\nSECURITY INVOKER\nVOLATILE\nAS $$\nBEGIN\n RAISE NOTICE 'User record changed: %', NEW.id;\n RETURN NEW;\nEND;\n$$;", + "type": "function", + "operation": "create", + "path": "public.log_user_changes" + }, + { + "sql": "CREATE OR REPLACE TRIGGER log_users_trigger\n AFTER INSERT OR UPDATE ON users\n FOR EACH ROW\n EXECUTE FUNCTION log_user_changes();", "type": "table.trigger", "operation": "create", - "path": "public.users.update_users_modified_time" + "path": "public.users.log_users_trigger" } ] } diff --git a/testdata/diff/dependency/function_to_trigger/plan.sql b/testdata/diff/dependency/function_to_trigger/plan.sql index 118ef64e..1f0084bd 100644 --- a/testdata/diff/dependency/function_to_trigger/plan.sql +++ b/testdata/diff/dependency/function_to_trigger/plan.sql @@ -1,26 +1,20 @@ -CREATE TABLE IF NOT EXISTS users ( - id SERIAL, - name text NOT NULL, - email text, - created_at timestamp DEFAULT CURRENT_TIMESTAMP, - updated_at timestamp DEFAULT CURRENT_TIMESTAMP, - CONSTRAINT users_pkey PRIMARY KEY (id), - CONSTRAINT users_email_key UNIQUE (email) -); +DROP TRIGGER IF EXISTS update_users_modified_time ON users; -CREATE OR REPLACE FUNCTION update_modified_time() +DROP FUNCTION IF EXISTS update_modified_time(); + +CREATE OR REPLACE FUNCTION log_user_changes() RETURNS trigger LANGUAGE plpgsql SECURITY INVOKER VOLATILE AS $$ BEGIN - NEW.updated_at = CURRENT_TIMESTAMP; + RAISE NOTICE 'User record changed: %', NEW.id; RETURN NEW; END; $$; -CREATE OR REPLACE TRIGGER update_users_modified_time - BEFORE UPDATE ON users +CREATE OR REPLACE TRIGGER log_users_trigger + AFTER INSERT OR UPDATE ON users FOR EACH ROW - EXECUTE FUNCTION update_modified_time(); + EXECUTE FUNCTION log_user_changes(); diff --git a/testdata/diff/dependency/function_to_trigger/plan.txt b/testdata/diff/dependency/function_to_trigger/plan.txt index dafd2521..f3c34cf2 100644 --- a/testdata/diff/dependency/function_to_trigger/plan.txt +++ b/testdata/diff/dependency/function_to_trigger/plan.txt @@ -1,42 +1,38 @@ -Plan: 2 to add. +Plan: 1 to add, 1 to modify, 1 to drop. Summary by type: - functions: 1 to add - tables: 1 to add + functions: 1 to add, 1 to drop + tables: 1 to modify Functions: - + update_modified_time + + log_user_changes + - update_modified_time Tables: - + users - + update_users_modified_time (trigger) + ~ users + + log_users_trigger (trigger) + - update_users_modified_time (trigger) DDL to be executed: -------------------------------------------------- -CREATE TABLE IF NOT EXISTS users ( - id SERIAL, - name text NOT NULL, - email text, - created_at timestamp DEFAULT CURRENT_TIMESTAMP, - updated_at timestamp DEFAULT CURRENT_TIMESTAMP, - CONSTRAINT users_pkey PRIMARY KEY (id), - CONSTRAINT users_email_key UNIQUE (email) -); - -CREATE OR REPLACE FUNCTION update_modified_time() +DROP TRIGGER IF EXISTS update_users_modified_time ON users; + +DROP FUNCTION IF EXISTS update_modified_time(); + +CREATE OR REPLACE FUNCTION log_user_changes() RETURNS trigger LANGUAGE plpgsql SECURITY INVOKER VOLATILE AS $$ BEGIN - NEW.updated_at = CURRENT_TIMESTAMP; + RAISE NOTICE 'User record changed: %', NEW.id; RETURN NEW; END; $$; -CREATE OR REPLACE TRIGGER update_users_modified_time - BEFORE UPDATE ON users +CREATE OR REPLACE TRIGGER log_users_trigger + AFTER INSERT OR UPDATE ON users FOR EACH ROW - EXECUTE FUNCTION update_modified_time(); + EXECUTE FUNCTION log_user_changes();