Conversation
Column default expressions with function calls were generating schema-qualified names (e.g., "public.get_status()") even when PostgreSQL's pg_get_expr() returns unqualified names for functions in the same schema. This caused drift detection where the second plan/apply would attempt to re-apply the same default. Changes: - Add normalizeDefaultExpr() helper in internal/diff/table.go to strip same-schema qualifiers from function calls in default expressions - Apply normalization in buildColumnClauses() for CREATE TABLE and ADD COLUMN - Apply normalization in column.go generateColumnSQL() for ALTER COLUMN SET DEFAULT - Update columnsEqual() to compare normalized default values - Update test expectations in testdata/diff/dependency/function_to_table/ This matches PostgreSQL's behavior where pg_get_expr() returns unqualified function names for functions in the same schema as the table, ensuring idempotent plan generation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Moved schema qualifier normalization from diff generation to IR normalization for better separation of concerns and architectural consistency. Changes: - Extended ir.normalizeDefaultValue() to strip same-schema function qualifiers - Updated normalizeColumn() to pass table schema context - Updated normalizeFunction/Procedure() to pass schema for parameter defaults - Removed normalizeDefaultExpr() from internal/diff/table.go - Simplified buildColumnClauses() to use already-normalized values - Simplified generateColumnSQL() in column.go to use normalized values - Simplified columnsEqual() - no longer needs schema parameter Benefits: - Single normalization point in IR layer (follows existing pattern) - Diff code is simpler - just compares normalized values - More maintainable - normalization logic lives with other IR normalizations - Consistent with how other IR attributes are normalized All 77 diff tests and full integration test suite pass. 🤖 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 fixes dependency ordering issues when creating tables with column defaults that reference functions (issue #174). The fix ensures functions are created before tables that use them in column defaults, CHECK constraints, or generated columns.
Key Changes:
- Extended normalization to strip same-schema qualifiers from function calls in column defaults, matching PostgreSQL's behavior
- Split table creation into two batches: tables without function dependencies first, then tables with function dependencies after functions are created
- Added logic to detect when tables reference newly added functions in defaults, generated expressions, and CHECK constraints
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
ir/normalize.go |
Added schema parameter to normalizeColumn and normalizeDefaultValue to strip same-schema function qualifiers; updated function and procedure parameter normalization to pass schema context |
internal/diff/diff.go |
Implemented two-phase table creation to handle function dependencies; added tableReferencesNewFunction to detect tables that reference new functions |
internal/diff/table.go |
Removed duplicate schema-agnostic logic for default values, now handled by normalization; added comment noting normalization is done in IR layer |
internal/diff/column.go |
Added comments noting default values are already normalized |
testdata/diff/dependency/function_to_table/* |
Added test data demonstrating correct dependency ordering for function-to-table references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| generateCreatePoliciesSQL(allDeferredPolicies, targetSchema, collector) | ||
|
|
||
| // Create triggers (triggers may depend on functions/procedures) | ||
| // Note: We need to create triggers for ALL tables, not just the original d.addedTables |
There was a problem hiding this comment.
The comment states "We need to create triggers for ALL tables, not just the original d.addedTables", but the code on line 985 still only passes d.addedTables. Either the comment is misleading or the code needs to be updated. If triggers should be created for all newly added tables (including both tablesWithoutFunctionDeps and tablesWithFunctionDeps), the code is already correct since d.addedTables contains all of them. Consider clarifying the comment or removing it if it's not accurate.
| // Note: We need to create triggers for ALL tables, not just the original d.addedTables |
| // Pattern: schema.function_name( | ||
| // Replace "tableSchema." with "" when followed by identifier and ( | ||
| prefix := tableSchema + "." | ||
| pattern := regexp.MustCompile(regexp.QuoteMeta(prefix) + `([a-zA-Z_][a-zA-Z0-9_]*)\(`) |
There was a problem hiding this comment.
The regex pattern for matching function names should include $ in the character class to match PostgreSQL's identifier rules. PostgreSQL allows $ in identifiers, and the functionCallRegex in internal/diff/diff.go (line 1197) correctly includes it: [a-z0-9_$].
Update the pattern to:
pattern := regexp.MustCompile(regexp.QuoteMeta(prefix) + `([a-zA-Z_][a-zA-Z0-9_$]*)\(`)This ensures consistent handling of function names that contain $ characters (e.g., public.my$function()).
| pattern := regexp.MustCompile(regexp.QuoteMeta(prefix) + `([a-zA-Z_][a-zA-Z0-9_]*)\(`) | |
| pattern := regexp.MustCompile(regexp.QuoteMeta(prefix) + `([a-zA-Z_][a-zA-Z0-9_$]*)\(`) |
1 participant
Fix #174