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); +$$;