feat: enhance function default parameter support and fix parsing bugs #99
Conversation
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 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances function default parameter handling by fixing parameter parsing bugs and improving default value normalization to ensure consistency between SQL file parsing and database extraction.
Key Changes:
- Fixed comma-in-default parsing bug by introducing
splitParameterString()that respects quotes, brackets, and parentheses - Enhanced default value normalization to handle NULL casts, numeric literal casts, string literal casts with escaped quotes, and parenthesized expressions
- Expanded test coverage with multiple parameter types (numeric, string, boolean defaults)
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ir/inspector.go | Added splitParameterString() function to correctly parse parameters with complex defaults containing commas |
| ir/normalize.go | Enhanced normalizeDefaultValue() with comprehensive regex patterns for various cast types and literals |
| testdata/diff/create_function/add_function/*.{sql,txt,json} | Added test cases for numeric (0, 1), string ('', 'pending'), and boolean (true, false) defaults |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ir/normalize.go
Outdated
| 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") |
There was a problem hiding this comment.
Lines 150-151 appear to be dead code that's immediately overwritten by line 154. The comment on lines 152-153 suggests the first pattern was incorrect, but the code should be removed rather than left in place with a correcting comment. This reduces confusion and improves maintainability.
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 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // 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.]*)(?:\[\])?`) |
There was a problem hiding this comment.
This regex is compiled on every call to normalizeDefaultValue(). Consider moving regex compilation to package-level variables to avoid repeated compilation overhead.
| // 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)`) |
There was a problem hiding this comment.
This regex is compiled on every call to normalizeDefaultValue(). Consider moving regex compilation to package-level variables to avoid repeated compilation overhead.
| 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.]*)(?:\[\])?`) |
There was a problem hiding this comment.
This regex is compiled on every call to normalizeDefaultValue(). Consider moving regex compilation to package-level variables to avoid repeated compilation overhead.
| // Handle parenthesized expressions with type casts - remove outer parentheses | ||
| // Example: (100)::bigint -> 100::bigint | ||
| // Pattern captures the number and the type cast separately | ||
| re = regexp.MustCompile(`\((\d+)\)(::(?:bigint|integer|smallint|numeric|decimal))`) |
There was a problem hiding this comment.
This regex is compiled on every call to normalizeDefaultValue(). Consider moving regex compilation to package-level variables to avoid repeated compilation overhead.
1 participant
This commit improves function default parameter handling by:
Fixing parameter parsing bug in inspector.go:
Enhancing default value normalization in normalize.go:
Expanding test coverage in testdata/diff/create_function/add_function/:
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