From dd5122cc2888634aeea9e6e3a904956ccb42ad13 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Wed, 25 Feb 2026 21:55:17 -0800 Subject: [PATCH] fix: DROP function before CREATE when return type or param names change (#326) PostgreSQL does not allow CREATE OR REPLACE FUNCTION to change the return type, parameter names, or OUT/INOUT parameter types of an existing function. When these change (but input parameter types stay the same, keeping the same function signature key), pgschema now generates DROP FUNCTION followed by CREATE FUNCTION instead of just CREATE OR REPLACE FUNCTION. Also extracts generateDropFunctionSQL helper to share DROP statement generation between the recreate path and the existing drop path. Co-Authored-By: Claude Opus 4.6 --- internal/diff/function.go | 77 ++++++++++++++++--- .../issue_326_param_name_change/diff.sql | 10 +++ .../issue_326_param_name_change/new.sql | 5 ++ .../issue_326_param_name_change/old.sql | 5 ++ .../issue_326_param_name_change/plan.json | 26 +++++++ .../issue_326_param_name_change/plan.sql | 10 +++ .../issue_326_param_name_change/plan.txt | 21 +++++ .../issue_326_param_type_change/diff.sql | 10 +++ .../issue_326_param_type_change/new.sql | 5 ++ .../issue_326_param_type_change/old.sql | 5 ++ .../issue_326_param_type_change/plan.json | 26 +++++++ .../issue_326_param_type_change/plan.sql | 10 +++ .../issue_326_param_type_change/plan.txt | 22 ++++++ .../issue_326_return_type_change/diff.sql | 10 +++ .../issue_326_return_type_change/new.sql | 5 ++ .../issue_326_return_type_change/old.sql | 5 ++ .../issue_326_return_type_change/plan.json | 26 +++++++ .../issue_326_return_type_change/plan.sql | 10 +++ .../issue_326_return_type_change/plan.txt | 21 +++++ 19 files changed, 298 insertions(+), 11 deletions(-) create mode 100644 testdata/diff/create_function/issue_326_param_name_change/diff.sql create mode 100644 testdata/diff/create_function/issue_326_param_name_change/new.sql create mode 100644 testdata/diff/create_function/issue_326_param_name_change/old.sql create mode 100644 testdata/diff/create_function/issue_326_param_name_change/plan.json create mode 100644 testdata/diff/create_function/issue_326_param_name_change/plan.sql create mode 100644 testdata/diff/create_function/issue_326_param_name_change/plan.txt create mode 100644 testdata/diff/create_function/issue_326_param_type_change/diff.sql create mode 100644 testdata/diff/create_function/issue_326_param_type_change/new.sql create mode 100644 testdata/diff/create_function/issue_326_param_type_change/old.sql create mode 100644 testdata/diff/create_function/issue_326_param_type_change/plan.json create mode 100644 testdata/diff/create_function/issue_326_param_type_change/plan.sql create mode 100644 testdata/diff/create_function/issue_326_param_type_change/plan.txt create mode 100644 testdata/diff/create_function/issue_326_return_type_change/diff.sql create mode 100644 testdata/diff/create_function/issue_326_return_type_change/new.sql create mode 100644 testdata/diff/create_function/issue_326_return_type_change/old.sql create mode 100644 testdata/diff/create_function/issue_326_return_type_change/plan.json create mode 100644 testdata/diff/create_function/issue_326_return_type_change/plan.sql create mode 100644 testdata/diff/create_function/issue_326_return_type_change/plan.txt diff --git a/internal/diff/function.go b/internal/diff/function.go index f1c11646..4fa51ab6 100644 --- a/internal/diff/function.go +++ b/internal/diff/function.go @@ -100,6 +100,31 @@ func generateModifyFunctionsSQL(diffs []*functionDiff, targetSchema string, coll if oldFunc.Comment != newFunc.Comment { generateFunctionComment(newFunc, targetSchema, DiffTypeFunction, DiffOperationAlter, collector) } + } else if functionRequiresRecreate(oldFunc, newFunc) { + // Return type, OUT parameters, or parameter names changed - must DROP then CREATE + // PostgreSQL does not allow CREATE OR REPLACE to change these. + // See https://github.com/pgplex/pgschema/issues/326 + dropSQL := generateDropFunctionSQL(oldFunc, targetSchema) + createSQL := generateFunctionSQL(newFunc, targetSchema) + + alterContext := &diffContext{ + Type: DiffTypeFunction, + Operation: DiffOperationAlter, + Path: fmt.Sprintf("%s.%s", newFunc.Schema, newFunc.Name), + Source: diff, + CanRunInTransaction: true, + } + + statements := []SQLStatement{ + {SQL: dropSQL, CanRunInTransaction: true}, + {SQL: createSQL, CanRunInTransaction: true}, + } + collector.collectStatements(alterContext, statements) + + // Check if comment also changed alongside body changes + if oldFunc.Comment != newFunc.Comment { + generateFunctionComment(newFunc, targetSchema, DiffTypeFunction, DiffOperationAlter, collector) + } } else { // Function body or other attributes changed - use CREATE OR REPLACE sql := generateFunctionSQL(newFunc, targetSchema) @@ -129,17 +154,7 @@ func generateDropFunctionsSQL(functions []*ir.Function, targetSchema string, col sortedFunctions := reverseSlice(topologicallySortFunctions(functions)) for _, function := range sortedFunctions { - functionName := qualifyEntityName(function.Schema, function.Name, targetSchema) - var sql string - - // Build argument list for DROP statement using GetArguments() - argsList := function.GetArguments() - - if argsList != "" { - sql = fmt.Sprintf("DROP FUNCTION IF EXISTS %s(%s);", functionName, argsList) - } else { - sql = fmt.Sprintf("DROP FUNCTION IF EXISTS %s();", functionName) - } + sql := generateDropFunctionSQL(function, targetSchema) // Create context for this statement context := &diffContext{ @@ -154,6 +169,16 @@ func generateDropFunctionsSQL(functions []*ir.Function, targetSchema string, col } } +// generateDropFunctionSQL generates a DROP FUNCTION IF EXISTS statement +func generateDropFunctionSQL(function *ir.Function, targetSchema string) string { + functionName := qualifyEntityName(function.Schema, function.Name, targetSchema) + argsList := function.GetArguments() + if argsList != "" { + return fmt.Sprintf("DROP FUNCTION IF EXISTS %s(%s);", functionName, argsList) + } + return fmt.Sprintf("DROP FUNCTION IF EXISTS %s();", functionName) +} + // generateFunctionSQL generates CREATE OR REPLACE FUNCTION SQL for a function func generateFunctionSQL(function *ir.Function, targetSchema string) string { var stmt strings.Builder @@ -460,6 +485,36 @@ func functionsEqualExceptComment(old, new *ir.Function) bool { return parametersEqual(oldInputParams, newInputParams) } +// functionRequiresRecreate checks if a function modification requires DROP+CREATE +// instead of CREATE OR REPLACE. PostgreSQL does not allow CREATE OR REPLACE to change +// the return type or parameter names of an existing function. +func functionRequiresRecreate(old, new *ir.Function) bool { + if old.ReturnType != new.ReturnType { + return true + } + // Check parameter changes that CREATE OR REPLACE cannot handle. + // Input parameter types are the same (same map key), but names, OUT/INOUT + // parameter types/modes, or parameter count differences require DROP+CREATE. + oldParams := filterNonTableParameters(old.Parameters) + newParams := filterNonTableParameters(new.Parameters) + if len(oldParams) != len(newParams) { + return true + } + for i := range oldParams { + if oldParams[i].Name != newParams[i].Name { + return true + } + // OUT/INOUT parameter type or mode changes also require DROP+CREATE + if oldParams[i].Mode == "OUT" || oldParams[i].Mode == "INOUT" || + newParams[i].Mode == "OUT" || newParams[i].Mode == "INOUT" { + if !parameterEqual(oldParams[i], newParams[i]) { + return true + } + } + } + return false +} + // filterNonTableParameters filters out TABLE mode parameters // TABLE parameters are output columns in RETURNS TABLE() and shouldn't be compared as input parameters func filterNonTableParameters(params []*ir.Parameter) []*ir.Parameter { diff --git a/testdata/diff/create_function/issue_326_param_name_change/diff.sql b/testdata/diff/create_function/issue_326_param_name_change/diff.sql new file mode 100644 index 00000000..f23a2d19 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_name_change/diff.sql @@ -0,0 +1,10 @@ +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + new_name text +) +RETURNS text +LANGUAGE sql +VOLATILE +AS $$ SELECT new_name; +$$; diff --git a/testdata/diff/create_function/issue_326_param_name_change/new.sql b/testdata/diff/create_function/issue_326_param_name_change/new.sql new file mode 100644 index 00000000..5ab93b42 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_name_change/new.sql @@ -0,0 +1,5 @@ +CREATE OR REPLACE FUNCTION somefunction( + new_name text +) RETURNS text +LANGUAGE sql +AS $$ SELECT new_name; $$; diff --git a/testdata/diff/create_function/issue_326_param_name_change/old.sql b/testdata/diff/create_function/issue_326_param_name_change/old.sql new file mode 100644 index 00000000..1bbc87d9 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_name_change/old.sql @@ -0,0 +1,5 @@ +CREATE OR REPLACE FUNCTION somefunction( + old_name text +) RETURNS text +LANGUAGE sql +AS $$ SELECT old_name; $$; diff --git a/testdata/diff/create_function/issue_326_param_name_change/plan.json b/testdata/diff/create_function/issue_326_param_name_change/plan.json new file mode 100644 index 00000000..9d3886c8 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_name_change/plan.json @@ -0,0 +1,26 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.2", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "d87f2cfffc1d1273ca588466e14557b2698607c55dc7c8a0e44317046e3c95a9" + }, + "groups": [ + { + "steps": [ + { + "sql": "DROP FUNCTION IF EXISTS somefunction(text);", + "type": "function", + "operation": "alter", + "path": "public.somefunction" + }, + { + "sql": "CREATE OR REPLACE FUNCTION somefunction(\n new_name text\n)\nRETURNS text\nLANGUAGE sql\nVOLATILE\nAS $$ SELECT new_name;\n$$;", + "type": "function", + "operation": "alter", + "path": "public.somefunction" + } + ] + } + ] +} diff --git a/testdata/diff/create_function/issue_326_param_name_change/plan.sql b/testdata/diff/create_function/issue_326_param_name_change/plan.sql new file mode 100644 index 00000000..f23a2d19 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_name_change/plan.sql @@ -0,0 +1,10 @@ +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + new_name text +) +RETURNS text +LANGUAGE sql +VOLATILE +AS $$ SELECT new_name; +$$; diff --git a/testdata/diff/create_function/issue_326_param_name_change/plan.txt b/testdata/diff/create_function/issue_326_param_name_change/plan.txt new file mode 100644 index 00000000..f40a5796 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_name_change/plan.txt @@ -0,0 +1,21 @@ +Plan: 1 to modify. + +Summary by type: + functions: 1 to modify + +Functions: + ~ somefunction + +DDL to be executed: +-------------------------------------------------- + +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + new_name text +) +RETURNS text +LANGUAGE sql +VOLATILE +AS $$ SELECT new_name; +$$; diff --git a/testdata/diff/create_function/issue_326_param_type_change/diff.sql b/testdata/diff/create_function/issue_326_param_type_change/diff.sql new file mode 100644 index 00000000..ff9f4641 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_type_change/diff.sql @@ -0,0 +1,10 @@ +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + param2 uuid +) +RETURNS uuid +LANGUAGE sql +VOLATILE +AS $$ SELECT param2; +$$; diff --git a/testdata/diff/create_function/issue_326_param_type_change/new.sql b/testdata/diff/create_function/issue_326_param_type_change/new.sql new file mode 100644 index 00000000..9f1b337b --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_type_change/new.sql @@ -0,0 +1,5 @@ +CREATE OR REPLACE FUNCTION somefunction( + param2 uuid +) RETURNS uuid +LANGUAGE sql +AS $$ SELECT param2; $$; diff --git a/testdata/diff/create_function/issue_326_param_type_change/old.sql b/testdata/diff/create_function/issue_326_param_type_change/old.sql new file mode 100644 index 00000000..7c212103 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_type_change/old.sql @@ -0,0 +1,5 @@ +CREATE OR REPLACE FUNCTION somefunction( + param1 text +) RETURNS text +LANGUAGE sql +AS $$ SELECT param1; $$; diff --git a/testdata/diff/create_function/issue_326_param_type_change/plan.json b/testdata/diff/create_function/issue_326_param_type_change/plan.json new file mode 100644 index 00000000..afc57d99 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_type_change/plan.json @@ -0,0 +1,26 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.2", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "59a96fc0ed0cbfa32f92a9d869bbbc6e38359d50afcd5ea6eb4f388717c48135" + }, + "groups": [ + { + "steps": [ + { + "sql": "DROP FUNCTION IF EXISTS somefunction(text);", + "type": "function", + "operation": "drop", + "path": "public.somefunction" + }, + { + "sql": "CREATE OR REPLACE FUNCTION somefunction(\n param2 uuid\n)\nRETURNS uuid\nLANGUAGE sql\nVOLATILE\nAS $$ SELECT param2;\n$$;", + "type": "function", + "operation": "create", + "path": "public.somefunction" + } + ] + } + ] +} diff --git a/testdata/diff/create_function/issue_326_param_type_change/plan.sql b/testdata/diff/create_function/issue_326_param_type_change/plan.sql new file mode 100644 index 00000000..ff9f4641 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_type_change/plan.sql @@ -0,0 +1,10 @@ +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + param2 uuid +) +RETURNS uuid +LANGUAGE sql +VOLATILE +AS $$ SELECT param2; +$$; diff --git a/testdata/diff/create_function/issue_326_param_type_change/plan.txt b/testdata/diff/create_function/issue_326_param_type_change/plan.txt new file mode 100644 index 00000000..5900c097 --- /dev/null +++ b/testdata/diff/create_function/issue_326_param_type_change/plan.txt @@ -0,0 +1,22 @@ +Plan: 1 to add, 1 to drop. + +Summary by type: + functions: 1 to add, 1 to drop + +Functions: + - somefunction + + somefunction + +DDL to be executed: +-------------------------------------------------- + +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + param2 uuid +) +RETURNS uuid +LANGUAGE sql +VOLATILE +AS $$ SELECT param2; +$$; diff --git a/testdata/diff/create_function/issue_326_return_type_change/diff.sql b/testdata/diff/create_function/issue_326_return_type_change/diff.sql new file mode 100644 index 00000000..277bef82 --- /dev/null +++ b/testdata/diff/create_function/issue_326_return_type_change/diff.sql @@ -0,0 +1,10 @@ +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + param1 text +) +RETURNS integer +LANGUAGE sql +VOLATILE +AS $$ SELECT length(param1); +$$; diff --git a/testdata/diff/create_function/issue_326_return_type_change/new.sql b/testdata/diff/create_function/issue_326_return_type_change/new.sql new file mode 100644 index 00000000..edbbb0ef --- /dev/null +++ b/testdata/diff/create_function/issue_326_return_type_change/new.sql @@ -0,0 +1,5 @@ +CREATE OR REPLACE FUNCTION somefunction( + param1 text +) RETURNS integer +LANGUAGE sql +AS $$ SELECT length(param1); $$; diff --git a/testdata/diff/create_function/issue_326_return_type_change/old.sql b/testdata/diff/create_function/issue_326_return_type_change/old.sql new file mode 100644 index 00000000..7c212103 --- /dev/null +++ b/testdata/diff/create_function/issue_326_return_type_change/old.sql @@ -0,0 +1,5 @@ +CREATE OR REPLACE FUNCTION somefunction( + param1 text +) RETURNS text +LANGUAGE sql +AS $$ SELECT param1; $$; diff --git a/testdata/diff/create_function/issue_326_return_type_change/plan.json b/testdata/diff/create_function/issue_326_return_type_change/plan.json new file mode 100644 index 00000000..44032415 --- /dev/null +++ b/testdata/diff/create_function/issue_326_return_type_change/plan.json @@ -0,0 +1,26 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.2", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "59a96fc0ed0cbfa32f92a9d869bbbc6e38359d50afcd5ea6eb4f388717c48135" + }, + "groups": [ + { + "steps": [ + { + "sql": "DROP FUNCTION IF EXISTS somefunction(text);", + "type": "function", + "operation": "alter", + "path": "public.somefunction" + }, + { + "sql": "CREATE OR REPLACE FUNCTION somefunction(\n param1 text\n)\nRETURNS integer\nLANGUAGE sql\nVOLATILE\nAS $$ SELECT length(param1);\n$$;", + "type": "function", + "operation": "alter", + "path": "public.somefunction" + } + ] + } + ] +} diff --git a/testdata/diff/create_function/issue_326_return_type_change/plan.sql b/testdata/diff/create_function/issue_326_return_type_change/plan.sql new file mode 100644 index 00000000..277bef82 --- /dev/null +++ b/testdata/diff/create_function/issue_326_return_type_change/plan.sql @@ -0,0 +1,10 @@ +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + param1 text +) +RETURNS integer +LANGUAGE sql +VOLATILE +AS $$ SELECT length(param1); +$$; diff --git a/testdata/diff/create_function/issue_326_return_type_change/plan.txt b/testdata/diff/create_function/issue_326_return_type_change/plan.txt new file mode 100644 index 00000000..6930e8ae --- /dev/null +++ b/testdata/diff/create_function/issue_326_return_type_change/plan.txt @@ -0,0 +1,21 @@ +Plan: 1 to modify. + +Summary by type: + functions: 1 to modify + +Functions: + ~ somefunction + +DDL to be executed: +-------------------------------------------------- + +DROP FUNCTION IF EXISTS somefunction(text); + +CREATE OR REPLACE FUNCTION somefunction( + param1 text +) +RETURNS integer +LANGUAGE sql +VOLATILE +AS $$ SELECT length(param1); +$$;