Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 128 additions & 4 deletions internal/diff/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,27 @@ func generateDropFunctionsSQL(functions []*ir.Function, targetSchema string, col
for _, function := range sortedFunctions {
functionName := qualifyEntityName(function.Schema, function.Name, targetSchema)
var sql string
if function.Arguments != "" {
sql = fmt.Sprintf("DROP FUNCTION IF EXISTS %s(%s);", functionName, function.Arguments)

// Build argument list for DROP statement using normalized Parameters array
var argsList string
if len(function.Parameters) > 0 {
// Format parameters for DROP (omit names and defaults, include only types)
// Per PostgreSQL docs, DROP FUNCTION only needs input arguments (IN, INOUT, VARIADIC)
// Exclude OUT and TABLE mode parameters as they're part of the return signature
var argTypes []string
for _, param := range function.Parameters {
// Include only input parameter modes: IN (empty/implicit), INOUT, VARIADIC
if param.Mode == "" || param.Mode == "IN" || param.Mode == "INOUT" || param.Mode == "VARIADIC" {
argTypes = append(argTypes, param.DataType)
}
}
argsList = strings.Join(argTypes, ", ")
} else if function.Arguments != "" {
argsList = function.Arguments
}

if argsList != "" {
sql = fmt.Sprintf("DROP FUNCTION IF EXISTS %s(%s);", functionName, argsList)
} else {
sql = fmt.Sprintf("DROP FUNCTION IF EXISTS %s();", functionName)
}
Expand All @@ -90,8 +109,22 @@ func generateFunctionSQL(function *ir.Function, targetSchema string) string {
functionName := qualifyEntityName(function.Schema, function.Name, targetSchema)
stmt.WriteString(fmt.Sprintf("CREATE OR REPLACE FUNCTION %s", functionName))

// Add parameters using detailed signature if available
if function.Signature != "" {
// Add parameters - prefer structured Parameters array for normalized types
if len(function.Parameters) > 0 {
// Build parameter list from structured Parameters array
// Exclude TABLE mode parameters as they're part of RETURNS clause
var paramParts []string
for _, param := range function.Parameters {
if param.Mode != "TABLE" {
paramParts = append(paramParts, formatFunctionParameter(param, true))
}
}
if len(paramParts) > 0 {
stmt.WriteString(fmt.Sprintf("(\n %s\n)", strings.Join(paramParts, ",\n ")))
} else {
stmt.WriteString("()")
}
} else if function.Signature != "" {
Comment on lines +112 to +127
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prioritizing synthesized Parameters over the original Signature can produce invalid DDL when parameter names require quoting (e.g., reserved words) or when original quoting/formatting is significant. Prefer using function.Signature when present, falling back to structured Parameters only when Signature is empty, or ensure all parameter identifiers are properly quoted.

Copilot uses AI. Check for mistakes.
stmt.WriteString(fmt.Sprintf("(\n %s\n)", strings.ReplaceAll(function.Signature, ", ", ",\n ")))
} else if function.Arguments != "" {
stmt.WriteString(fmt.Sprintf("(%s)", function.Arguments))
Expand Down Expand Up @@ -184,6 +217,32 @@ func containsParameterReferences(body string) bool {
return false
}

// formatFunctionParameter formats a single function parameter with name, type, and optional default value
// For functions, mode is typically omitted (unlike procedures) unless it's OUT/INOUT
// includeDefault controls whether DEFAULT clauses are included in the output
func formatFunctionParameter(param *ir.Parameter, includeDefault bool) string {
var part string

// For functions, only include mode if it's OUT or INOUT (IN is implicit)
if param.Mode == "OUT" || param.Mode == "INOUT" || param.Mode == "VARIADIC" {
part = param.Mode + " "
}

// Add parameter name and type
if param.Name != "" {
part += param.Name + " " + param.DataType
} else {
part += param.DataType
}
Comment on lines +231 to +236
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameter names that are reserved words or contain uppercase/special characters require identifier quoting; emitting them unquoted can lead to invalid CREATE FUNCTION. Quote parameter names (e.g., part += quoteIdent(param.Name) + " " + param.DataType) and add a quoteIdent helper that double-quotes and escapes identifiers when they don't match a simple unquoted identifier pattern.

Copilot uses AI. Check for mistakes.

// Add DEFAULT value if present and requested
if includeDefault && param.DefaultValue != nil {
part += " DEFAULT " + *param.DefaultValue
}

return part
}

// functionsEqual compares two functions for equality
func functionsEqual(old, new *ir.Function) bool {
if old.Schema != new.Schema {
Expand All @@ -201,11 +260,76 @@ func functionsEqual(old, new *ir.Function) bool {
if old.Language != new.Language {
return false
}

// For RETURNS TABLE functions, the Parameters array includes TABLE output columns
// which can cause comparison issues. In this case, rely on ReturnType comparison instead.
isTableReturn := strings.HasPrefix(old.ReturnType, "TABLE(") || strings.HasPrefix(new.ReturnType, "TABLE(")

if !isTableReturn {
// For non-TABLE functions, compare using normalized Parameters array
// This ensures type aliases like "character varying" vs "varchar" are treated as equal
hasOldParams := len(old.Parameters) > 0
hasNewParams := len(new.Parameters) > 0

if hasOldParams && hasNewParams {
// Both have Parameters - compare them
return parametersEqual(old.Parameters, new.Parameters)
} else if hasOldParams || hasNewParams {
// One has Parameters, one doesn't - they're different
return false
}
}

// For TABLE functions or functions without Parameters, fall back to Arguments/Signature
if old.Arguments != new.Arguments {
return false
}
if old.Signature != new.Signature {
return false
}

return true
}

// parametersEqual compares two parameter arrays for equality
func parametersEqual(oldParams, newParams []*ir.Parameter) bool {
if len(oldParams) != len(newParams) {
return false
}

for i := range oldParams {
if !parameterEqual(oldParams[i], newParams[i]) {
return false
}
}

return true
}

// parameterEqual compares two parameters for equality
func parameterEqual(old, new *ir.Parameter) bool {
if old.Name != new.Name {
return false
}

// Compare data types (already normalized by ir.normalizeFunction)
if old.DataType != new.DataType {
return false
}

if old.Mode != new.Mode {
return false
}

// Compare default values
if (old.DefaultValue == nil) != (new.DefaultValue == nil) {
return false
}
if old.DefaultValue != nil && new.DefaultValue != nil {
if *old.DefaultValue != *new.DefaultValue {
return false
}
}

return true
}
21 changes: 21 additions & 0 deletions ir/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,27 @@ func normalizeFunction(function *Function) {
param.DataType = normalizePostgreSQLType(param.DataType)
}
}
// Normalize function body to handle whitespace differences
function.Definition = normalizeFunctionDefinition(function.Definition)
}

// normalizeFunctionDefinition normalizes function body whitespace
// PostgreSQL stores function bodies with specific whitespace that may differ from source
func normalizeFunctionDefinition(def string) string {
if def == "" {
return def
}

// Only trim trailing whitespace from each line, preserving the line structure
// This ensures leading/trailing blank lines are preserved (matching PostgreSQL storage)
lines := strings.Split(def, "\n")
var normalized []string
for _, line := range lines {
// Trim trailing whitespace but preserve leading whitespace for indentation
normalized = append(normalized, strings.TrimRight(line, " \t"))
}
Comment on lines +259 to +266
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trims spaces and tabs but leaves trailing carriage returns (\r) intact, which can appear from CRLF inputs and lead to unstable diffs. Use TrimRightFunc with unicode.IsSpace or include '\r' in the cutset to remove trailing CR as well.

Copilot uses AI. Check for mistakes.

return strings.Join(normalized, "\n")
}

// normalizeProcedure normalizes procedure representation
Expand Down
8 changes: 4 additions & 4 deletions ir/queries/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ ORDER BY s.schemaname, s.sequencename;
SELECT
r.routine_schema,
r.routine_name,
p.prosrc AS routine_definition,
CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END AS routine_definition,
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expression can yield NULL and has caused sqlc to infer routine_definition as sql.NullString, changing the downstream type and potentially breaking JSON shape. Wrap with COALESCE to keep a non-null string and preserve the previous string type: COALESCE(CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END, '') AS routine_definition, and re-generate.

Suggested change
CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END AS routine_definition,
COALESCE(CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END, '') AS routine_definition,

Copilot uses AI. Check for mistakes.
r.routine_type,
COALESCE(pg_get_function_result(p.oid), r.data_type) AS data_type,
r.external_language,
Expand All @@ -760,7 +760,7 @@ SELECT
p.proargnames,
p.proallargtypes::oid[]::text[] as proallargtypes
FROM information_schema.routines r
LEFT JOIN pg_proc p ON p.proname = r.routine_name
LEFT JOIN pg_proc p ON p.proname = r.routine_name
AND p.pronamespace = (SELECT oid FROM pg_namespace WHERE nspname = r.routine_schema)
LEFT JOIN pg_depend d ON d.objid = p.oid AND d.deptype = 'e'
LEFT JOIN pg_description desc_func ON desc_func.objoid = p.oid AND desc_func.classoid = 'pg_proc'::regclass
Expand All @@ -774,14 +774,14 @@ ORDER BY r.routine_schema, r.routine_name;
SELECT
r.routine_schema,
r.routine_name,
p.prosrc AS routine_definition,
CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END AS routine_definition,
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above for procedures: wrap with COALESCE to avoid NULL and retain a plain string type in generated code: COALESCE(CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END, '') AS routine_definition, then re-generate.

Suggested change
CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END AS routine_definition,
COALESCE(CASE WHEN p.prosrc ~ E'\n$' THEN p.prosrc ELSE p.prosrc || E'\n' END, '') AS routine_definition,

Copilot uses AI. Check for mistakes.
r.routine_type,
r.external_language,
COALESCE(desc_proc.description, '') AS procedure_comment,
oidvectortypes(p.proargtypes) AS procedure_arguments,
pg_get_function_arguments(p.oid) AS procedure_signature
FROM information_schema.routines r
LEFT JOIN pg_proc p ON p.proname = r.routine_name
LEFT JOIN pg_proc p ON p.proname = r.routine_name
AND p.pronamespace = (SELECT oid FROM pg_namespace WHERE nspname = r.routine_schema)
LEFT JOIN pg_depend d ON d.objid = p.oid AND d.deptype = 'e'
LEFT JOIN pg_description desc_proc ON desc_proc.objoid = p.oid AND desc_proc.classoid = 'pg_proc'::regclass
Expand Down
12 changes: 6 additions & 6 deletions ir/queries/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions testdata/diff/create_function/drop_function/diff.sql
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
DROP FUNCTION IF EXISTS get_user_stats(integer);
DROP FUNCTION IF EXISTS process_order(integer, numeric);
DROP FUNCTION IF EXISTS process_payment(integer, text);
41 changes: 41 additions & 0 deletions testdata/diff/create_function/drop_function/old.sql
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,45 @@ BEGIN
SELECT amount INTO total FROM orders WHERE id = order_id;
RETURN total - (total * discount_percent / 100);
END;
$$;

-- Function with OUT parameters - tests that DROP only includes IN parameters
CREATE FUNCTION get_user_stats(
user_id integer,
OUT total_orders integer,
OUT total_amount numeric,
OUT last_order_date timestamp
)
LANGUAGE plpgsql
SECURITY INVOKER
STABLE
AS $$
BEGIN
SELECT
COUNT(*),
COALESCE(SUM(amount), 0),
MAX(order_date)
INTO total_orders, total_amount, last_order_date
FROM orders
WHERE orders.user_id = get_user_stats.user_id;
END;
$$;

-- Function with mixed IN, OUT, and INOUT parameters
CREATE FUNCTION process_payment(
IN order_id integer,
INOUT transaction_id text,
OUT status text,
OUT processed_at timestamp
)
LANGUAGE plpgsql
SECURITY INVOKER
VOLATILE
AS $$
BEGIN
-- Process payment logic
transaction_id := 'TXN-' || transaction_id;
status := 'SUCCESS';
processed_at := NOW();
END;
$$;
14 changes: 13 additions & 1 deletion testdata/diff/create_function/drop_function/plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,28 @@
"pgschema_version": "1.4.0",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "912618f23b4d55b8abcf56d0155b6be724ffe7e43548af3391ec0b7cbcff5f6d"
"hash": "088aa747b7e56f2117edee2741301754f9890301077cbc83f29339c7aeccb41e"
},
"groups": [
{
"steps": [
{
"sql": "DROP FUNCTION IF EXISTS get_user_stats(integer);",
"type": "function",
"operation": "drop",
"path": "public.get_user_stats"
},
{
"sql": "DROP FUNCTION IF EXISTS process_order(integer, numeric);",
"type": "function",
"operation": "drop",
"path": "public.process_order"
},
{
"sql": "DROP FUNCTION IF EXISTS process_payment(integer, text);",
"type": "function",
"operation": "drop",
"path": "public.process_payment"
}
]
}
Expand Down
4 changes: 4 additions & 0 deletions testdata/diff/create_function/drop_function/plan.sql
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
DROP FUNCTION IF EXISTS get_user_stats(integer);

DROP FUNCTION IF EXISTS process_order(integer, numeric);

DROP FUNCTION IF EXISTS process_payment(integer, text);
10 changes: 8 additions & 2 deletions testdata/diff/create_function/drop_function/plan.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
Plan: 1 to drop.
Plan: 3 to drop.

Summary by type:
functions: 1 to drop
functions: 3 to drop

Functions:
- get_user_stats
- process_order
- process_payment

DDL to be executed:
--------------------------------------------------

DROP FUNCTION IF EXISTS get_user_stats(integer);

DROP FUNCTION IF EXISTS process_order(integer, numeric);

DROP FUNCTION IF EXISTS process_payment(integer, text);
2 changes: 1 addition & 1 deletion testdata/diff/migrate/v4/plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
"path": "public.salary.salary_log_trigger"
},
{
"sql": "CREATE OR REPLACE FUNCTION log_dml_operations()\nRETURNS trigger\nLANGUAGE plpgsql\nSECURITY INVOKER\nVOLATILE\nAS $$\nDECLARE\n table_category TEXT;\n log_level TEXT;\nBEGIN\n -- Get arguments passed from trigger (if any)\n -- TG_ARGV[0] is the first argument, TG_ARGV[1] is the second\n table_category := COALESCE(TG_ARGV[0], 'default');\n log_level := COALESCE(TG_ARGV[1], 'standard');\n \n IF (TG_OP = 'INSERT') THEN\n INSERT INTO audit (operation, query, user_name)\n VALUES (\n 'INSERT [' || table_category || ':' || log_level || ']', \n current_query(), \n current_user\n );\n RETURN NEW;\n ELSIF (TG_OP = 'UPDATE') THEN\n INSERT INTO audit (operation, query, user_name)\n VALUES (\n 'UPDATE [' || table_category || ':' || log_level || ']', \n current_query(), \n current_user\n );\n RETURN NEW;\n ELSIF (TG_OP = 'DELETE') THEN\n INSERT INTO audit (operation, query, user_name)\n VALUES (\n 'DELETE [' || table_category || ':' || log_level || ']', \n current_query(), \n current_user\n );\n RETURN OLD;\n END IF;\n RETURN NULL;\nEND;\n$$;",
"sql": "CREATE OR REPLACE FUNCTION log_dml_operations()\nRETURNS trigger\nLANGUAGE plpgsql\nSECURITY INVOKER\nVOLATILE\nAS $$\nDECLARE\n table_category TEXT;\n log_level TEXT;\nBEGIN\n -- Get arguments passed from trigger (if any)\n -- TG_ARGV[0] is the first argument, TG_ARGV[1] is the second\n table_category := COALESCE(TG_ARGV[0], 'default');\n log_level := COALESCE(TG_ARGV[1], 'standard');\n\n IF (TG_OP = 'INSERT') THEN\n INSERT INTO audit (operation, query, user_name)\n VALUES (\n 'INSERT [' || table_category || ':' || log_level || ']',\n current_query(),\n current_user\n );\n RETURN NEW;\n ELSIF (TG_OP = 'UPDATE') THEN\n INSERT INTO audit (operation, query, user_name)\n VALUES (\n 'UPDATE [' || table_category || ':' || log_level || ']',\n current_query(),\n current_user\n );\n RETURN NEW;\n ELSIF (TG_OP = 'DELETE') THEN\n INSERT INTO audit (operation, query, user_name)\n VALUES (\n 'DELETE [' || table_category || ':' || log_level || ']',\n current_query(),\n current_user\n );\n RETURN OLD;\n END IF;\n RETURN NULL;\nEND;\n$$;",
"type": "function",
"operation": "alter",
"path": "public.log_dml_operations"
Expand Down
Loading