From c5212ca5651d208bb91ff015fc45526adec2476d Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Mon, 20 Oct 2025 01:13:41 +0800 Subject: [PATCH 1/2] feat: enhance function default parameter support and fix parsing bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit improves function default parameter handling by: 1. Fixing parameter parsing bug in inspector.go: - Added splitParameterString() to correctly handle commas within complex defaults (arrays, JSON, expressions) - Properly handles escaped quotes in strings like 'O''Brien' - Respects nesting depth of parentheses, brackets, and braces 2. Enhancing default value normalization in normalize.go: - Handles NULL::type casts → NULL - Handles numeric literal casts like '-1'::integer → -1 - Handles string literal casts including escaped quotes - Handles parenthesized expression casts like (100)::bigint → 100::bigint - Ensures database-extracted and file-parsed defaults normalize identically 3. Expanding test coverage in testdata/diff/create_function/add_function/: - Added tests for multiple numeric defaults (0, 1) - Added tests for string defaults ('', 'pending') - Added tests for boolean defaults (true, false) - Verified idempotency: function can be applied twice with no changes These changes ensure function default parameters work correctly across parsing from SQL files, extraction from PostgreSQL databases, and comparison for migration generation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- ir/inspector.go | 71 ++++++++++++++++++- ir/normalize.go | 49 +++++++++---- .../create_function/add_function/diff.sql | 6 +- .../diff/create_function/add_function/new.sql | 9 ++- .../create_function/add_function/plan.json | 2 +- .../create_function/add_function/plan.sql | 6 +- .../create_function/add_function/plan.txt | 6 +- 7 files changed, 130 insertions(+), 19 deletions(-) diff --git a/ir/inspector.go b/ir/inspector.go index 8688f4ca..012e7ade 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -1169,6 +1169,73 @@ func (i *Inspector) buildFunctions(ctx context.Context, schema *IR, targetSchema return nil } +// splitParameterString splits a parameter string by commas, but respects quotes, +// parentheses, and brackets. This handles complex defaults like '{1,2,3}' or '{"key": "value"}' +func splitParameterString(signature string) []string { + var params []string + var current strings.Builder + depth := 0 // Track nesting depth of (), [], {} + inQuote := false // Track if we're inside a string literal + + i := 0 + for i < len(signature) { + ch := rune(signature[i]) + + switch ch { + case '\'': + // Toggle quote state, but handle escaped quotes + if !inQuote { + inQuote = true + current.WriteRune(ch) + i++ + } else { + // Check if this is an escaped quote (two single quotes) + if i+1 < len(signature) && signature[i+1] == '\'' { + current.WriteRune(ch) + current.WriteRune('\'') + i += 2 // Skip both quotes + } else { + inQuote = false + current.WriteRune(ch) + i++ + } + } + case '(', '[', '{': + if !inQuote { + depth++ + } + current.WriteRune(ch) + i++ + case ')', ']', '}': + if !inQuote { + depth-- + } + current.WriteRune(ch) + i++ + case ',': + if !inQuote && depth == 0 { + // This comma is a parameter separator + params = append(params, strings.TrimSpace(current.String())) + current.Reset() + } else { + // This comma is inside quotes or nested structure + current.WriteRune(ch) + } + i++ + default: + current.WriteRune(ch) + i++ + } + } + + // Add the last parameter + if current.Len() > 0 { + params = append(params, strings.TrimSpace(current.String())) + } + + return params +} + // parseParametersFromSignature parses function signature string into Parameter structs // Example signature: "order_id integer, discount_percent numeric DEFAULT 0" // Or with modes: "IN order_id integer, OUT result integer" @@ -1180,8 +1247,8 @@ func (i *Inspector) parseParametersFromSignature(signature string) []*Parameter var parameters []*Parameter position := 1 - // Split by comma to get individual parameters - paramStrings := strings.Split(signature, ",") + // Split by comma to get individual parameters (smart split that respects quotes/brackets) + paramStrings := splitParameterString(signature) for _, paramStr := range paramStrings { paramStr = strings.TrimSpace(paramStr) if paramStr == "" { diff --git a/ir/normalize.go b/ir/normalize.go index 10e06700..b0b3a9bc 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -113,21 +113,46 @@ func normalizeDefaultValue(value string) string { } // Handle type casting - remove explicit type casts that are semantically equivalent - // Use regex to properly handle type casts within complex expressions - // Pattern: 'literal'::type -> 'literal' (removes redundant casts from string literals) if strings.Contains(value, "::") { - // Use regex to match and remove type casts from string literals - // This handles: 'text'::text, 'utc'::text, '{}'::jsonb, '{}'::text[], etc. - // Also handles multi-word types like 'value'::character varying + // Handle NULL::type -> NULL + // Example: NULL::text -> NULL + re := regexp.MustCompile(`\bNULL::(?:[a-zA-Z_][\w\s.]*)(?:\[\])?`) + value = re.ReplaceAllString(value, "NULL") + + // Handle numeric literals with type casts + // Example: '-1'::integer -> -1 + // Example: '100'::bigint -> 100 + // Note: PostgreSQL sometimes casts numeric literals to different types, e.g., -1::integer stored as numeric + re = regexp.MustCompile(`'(-?\d+(?:\.\d+)?)'::(?:integer|bigint|smallint|numeric|decimal|real|double precision|int2|int4|int8|float4|float8)`) + value = re.ReplaceAllString(value, "$1") + + // Handle string literals with type casts (including escaped quotes) + // Example: 'text'::text -> 'text' + // Example: 'O''Brien'::text -> 'O''Brien' + // Example: '{}'::jsonb -> '{}' + // Example: '{1,2,3}'::integer[] -> '{1,2,3}' // Pattern explanation: - // '([^']*)' - matches a quoted string literal (capturing the content) + // '(?:[^']|'')*' - matches a quoted string literal, handling escaped quotes '' // ::[a-zA-Z_][\w\s.]* - matches ::typename - // [a-zA-Z_] - type name must start with letter or underscore - // [\w\s.]* - followed by word chars, spaces, or dots (for "character varying" or "pg_catalog.text") - // (?:\[\])? - optionally followed by [] for array types (non-capturing group) - // (?:\b|(?=\[)|$) - followed by word boundary, opening bracket, or end of string - re := regexp.MustCompile(`'([^']*)'::(?:[a-zA-Z_][\w\s.]*)(?:\[\])?`) - value = re.ReplaceAllString(value, "'$1'") + // (?:\[\])? - optionally followed by [] for array types + re = regexp.MustCompile(`('(?:[^']|'')*')::(?:[a-zA-Z_][\w\s.]*)(?:\[\])?`) + value = re.ReplaceAllString(value, "$1") + + // Handle date/timestamp literals with type casts + // Example: '2024-01-01'::date -> '2024-01-01' + // Already handled by the string literal pattern above + + // Handle parenthesized expressions with type casts - remove outer parentheses + // Example: (100)::bigint -> 100::bigint + // Example: ('prefix_'::text || 'suffix'::text) -> ('prefix_' || 'suffix') + // First remove type casts from string literals inside parentheses (already done above) + // Then remove outer parentheses if they're followed by a type cast + re = regexp.MustCompile(`\((\d+)\)::(?:bigint|integer|smallint|numeric|decimal)`) + value = re.ReplaceAllString(value, "$1::$2") + // But wait, we want to keep the cast for explicit casts like 100::bigint + // So let's just remove the parentheses around simple numbers + re = regexp.MustCompile(`\((\d+)\)(::(?:bigint|integer|smallint|numeric|decimal))`) + value = re.ReplaceAllString(value, "$1$2") } return value diff --git a/testdata/diff/create_function/add_function/diff.sql b/testdata/diff/create_function/add_function/diff.sql index b0129734..fd556b37 100644 --- a/testdata/diff/create_function/add_function/diff.sql +++ b/testdata/diff/create_function/add_function/diff.sql @@ -1,7 +1,11 @@ CREATE OR REPLACE FUNCTION process_order( order_id integer, discount_percent numeric DEFAULT 0, - note varchar DEFAULT '' + priority_level integer DEFAULT 1, + note varchar DEFAULT '', + status text DEFAULT 'pending', + apply_tax boolean DEFAULT true, + is_priority boolean DEFAULT false ) RETURNS numeric LANGUAGE plpgsql diff --git a/testdata/diff/create_function/add_function/new.sql b/testdata/diff/create_function/add_function/new.sql index a430daee..460749fc 100644 --- a/testdata/diff/create_function/add_function/new.sql +++ b/testdata/diff/create_function/add_function/new.sql @@ -1,7 +1,14 @@ CREATE FUNCTION process_order( order_id integer, + -- Simple numeric defaults discount_percent numeric DEFAULT 0, - note varchar DEFAULT '' + priority_level integer DEFAULT 1, + -- String defaults + note varchar DEFAULT '', + status text DEFAULT 'pending', + -- Boolean defaults + apply_tax boolean DEFAULT true, + is_priority boolean DEFAULT false ) RETURNS numeric LANGUAGE plpgsql diff --git a/testdata/diff/create_function/add_function/plan.json b/testdata/diff/create_function/add_function/plan.json index 1fc13b93..831f7fd5 100644 --- a/testdata/diff/create_function/add_function/plan.json +++ b/testdata/diff/create_function/add_function/plan.json @@ -9,7 +9,7 @@ { "steps": [ { - "sql": "CREATE OR REPLACE FUNCTION process_order(\n order_id integer,\n discount_percent numeric DEFAULT 0,\n note varchar DEFAULT ''\n)\nRETURNS numeric\nLANGUAGE plpgsql\nSECURITY DEFINER\nVOLATILE\nSTRICT\nAS $$\nDECLARE\n total numeric;\nBEGIN\n SELECT amount INTO total FROM orders WHERE id = order_id;\n RETURN total - (total * discount_percent / 100);\nEND;\n$$;", + "sql": "CREATE OR REPLACE FUNCTION process_order(\n order_id integer,\n discount_percent numeric DEFAULT 0,\n priority_level integer DEFAULT 1,\n note varchar DEFAULT '',\n status text DEFAULT 'pending',\n apply_tax boolean DEFAULT true,\n is_priority boolean DEFAULT false\n)\nRETURNS numeric\nLANGUAGE plpgsql\nSECURITY DEFINER\nVOLATILE\nSTRICT\nAS $$\nDECLARE\n total numeric;\nBEGIN\n SELECT amount INTO total FROM orders WHERE id = order_id;\n RETURN total - (total * discount_percent / 100);\nEND;\n$$;", "type": "function", "operation": "create", "path": "public.process_order" diff --git a/testdata/diff/create_function/add_function/plan.sql b/testdata/diff/create_function/add_function/plan.sql index b0129734..fd556b37 100644 --- a/testdata/diff/create_function/add_function/plan.sql +++ b/testdata/diff/create_function/add_function/plan.sql @@ -1,7 +1,11 @@ CREATE OR REPLACE FUNCTION process_order( order_id integer, discount_percent numeric DEFAULT 0, - note varchar DEFAULT '' + priority_level integer DEFAULT 1, + note varchar DEFAULT '', + status text DEFAULT 'pending', + apply_tax boolean DEFAULT true, + is_priority boolean DEFAULT false ) RETURNS numeric LANGUAGE plpgsql diff --git a/testdata/diff/create_function/add_function/plan.txt b/testdata/diff/create_function/add_function/plan.txt index eca4bd93..3b64a999 100644 --- a/testdata/diff/create_function/add_function/plan.txt +++ b/testdata/diff/create_function/add_function/plan.txt @@ -12,7 +12,11 @@ DDL to be executed: CREATE OR REPLACE FUNCTION process_order( order_id integer, discount_percent numeric DEFAULT 0, - note varchar DEFAULT '' + priority_level integer DEFAULT 1, + note varchar DEFAULT '', + status text DEFAULT 'pending', + apply_tax boolean DEFAULT true, + is_priority boolean DEFAULT false ) RETURNS numeric LANGUAGE plpgsql From 1facb19466fd295322f01a277f5e10d359c7b537 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Mon, 20 Oct 2025 01:16:59 +0800 Subject: [PATCH 2/2] fix: remove dead code with incorrect backreference in normalize.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Line 151 had a bug where it referenced $2 in the replacement string but the regex pattern only had one capture group. This line was dead code that would never execute correctly - the actual working version was on line 154-155 which correctly captured both the number and the type cast. Removed the buggy line and cleaned up the comments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- ir/normalize.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ir/normalize.go b/ir/normalize.go index b0b3a9bc..5cdbd61f 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -144,13 +144,7 @@ func normalizeDefaultValue(value string) string { // Handle parenthesized expressions with type casts - remove outer parentheses // Example: (100)::bigint -> 100::bigint - // Example: ('prefix_'::text || 'suffix'::text) -> ('prefix_' || 'suffix') - // First remove type casts from string literals inside parentheses (already done above) - // Then remove outer parentheses if they're followed by a type cast - re = regexp.MustCompile(`\((\d+)\)::(?:bigint|integer|smallint|numeric|decimal)`) - value = re.ReplaceAllString(value, "$1::$2") - // But wait, we want to keep the cast for explicit casts like 100::bigint - // So let's just remove the parentheses around simple numbers + // Pattern captures the number and the type cast separately re = regexp.MustCompile(`\((\d+)\)(::(?:bigint|integer|smallint|numeric|decimal))`) value = re.ReplaceAllString(value, "$1$2") }