Conversation
) Functions that reference views in their return type (e.g., RETURNS SETOF view_name) or parameter types were created before the referenced views, causing "type does not exist" errors during apply. Split function creation into two phases: functions without view dependencies are created before views (existing behavior), and functions with view dependencies are created after views. Also fix return type normalization to strip same-schema qualifiers from SETOF types for consistent comparison across different search_path contexts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR fixes a dependency ordering issue where functions referencing views in their return types or parameters were being created before the referenced views, causing Key changes:
Test coverage:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[Generate DDL Migration] --> BuildLookup[Build view lookup map]
BuildLookup --> SeparateFns[Separate functions by view dependencies]
SeparateFns --> CheckDep{Function references<br/>new view?}
CheckDep -->|Yes| FnWithDeps[functionsWithViewDeps]
CheckDep -->|No| FnWithoutDeps[functionsWithoutViewDeps]
FnWithoutDeps --> CreateTables[Create tables]
CreateTables --> CreateFnsPhase1[Create functions WITHOUT view deps]
CreateFnsPhase1 --> CreateDomains[Create domains]
CreateDomains --> CreateProcs[Create procedures]
CreateProcs --> CreateViews[Create views]
FnWithDeps --> WaitForViews[Wait for views...]
CreateViews --> CreateFnsPhase2[Create functions WITH view deps]
WaitForViews -.-> CreateFnsPhase2
CreateFnsPhase2 --> CreatePrivs[Create privileges]
CreatePrivs --> Done[Migration complete]
style CheckDep fill:#e1f5ff
style CreateViews fill:#d4edda
style CreateFnsPhase2 fill:#d4edda
Last reviewed commit: ade173c |
There was a problem hiding this comment.
Pull request overview
This pull request addresses issue #300 by fixing dependency ordering problems when creating functions that reference views in their return types or parameters. The fix implements a two-phase function creation strategy: functions without view dependencies are created before views (preserving existing behavior for table composite types), while functions with view dependencies are deferred until after views are created.
Changes:
- Split function creation into two phases based on view dependencies to ensure proper DDL ordering
- Added return type normalization to strip same-schema qualifiers from SETOF types for consistent comparison
- Introduced helper functions to detect view dependencies in function signatures
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ir/normalize.go |
Added stripSchemaFromReturnType and stripSchemaPrefix functions to normalize return types by removing same-schema qualifiers |
internal/diff/diff.go |
Implemented two-phase function creation logic, refactored lookup building into shared buildSchemaNameLookup, added view dependency detection |
testdata/diff/dependency/issue_300_view_depends_on_view/* |
Test case verifying view-to-view dependency ordering (already handled by topological sort) |
testdata/diff/dependency/issue_300_function_table_composite_type/* |
Test case verifying functions with table composite types and view return types are created in correct order |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Check parameter types | ||
| for _, param := range fn.Parameters { |
There was a problem hiding this comment.
Missing nil check for parameter. If a parameter in fn.Parameters is nil, accessing param.DataType will cause a panic. Consider adding a nil check before accessing param.DataType.
| for _, param := range fn.Parameters { | |
| for _, param := range fn.Parameters { | |
| if param == nil { | |
| continue | |
| } |
| names := make([]struct{ schema, name string }, len(functions)) | ||
| for i, fn := range functions { | ||
| names[i] = struct{ schema, name string }{fn.Schema, fn.Name} |
There was a problem hiding this comment.
The refactored buildFunctionLookup has lost the nil check that existed in the previous implementation. The old code checked if fn == nil || fn.Name == "" before accessing fn.Name. While the current implementation may work if functions are never nil in practice, this removes a defensive programming safeguard that was previously in place. Consider adding a nil check in buildSchemaNameLookup or in the callers before constructing the names slice.
| names := make([]struct{ schema, name string }, len(functions)) | |
| for i, fn := range functions { | |
| names[i] = struct{ schema, name string }{fn.Schema, fn.Name} | |
| names := make([]struct{ schema, name string }, 0, len(functions)) | |
| for _, fn := range functions { | |
| if fn == nil || fn.Name == "" { | |
| continue | |
| } | |
| names = append(names, struct{ schema, name string }{fn.Schema, fn.Name}) |
| // buildViewLookup returns case-insensitive lookup keys for newly added views. | ||
| func buildViewLookup(views []*ir.View) map[string]struct{} { | ||
| names := make([]struct{ schema, name string }, len(views)) | ||
| for i, v := range views { | ||
| names[i] = struct{ schema, name string }{v.Schema, v.Name} | ||
| } | ||
| return buildSchemaNameLookup(names) | ||
| } |
There was a problem hiding this comment.
Similar to buildFunctionLookup, this function should handle potential nil views defensively. Consider adding a nil check before accessing v.Schema and v.Name, or ensure that views in the slice are never nil.
1 participant
Summary
RETURNS SETOF view_name) or parameter types were being created before the referenced views, causingtype does not existerrors duringpgschema applySETOFtypes (e.g.,SETOF public.actor→SETOF actor) for consistent comparison across different search_path contextsFixes #300
Test plan
testdata/diff/dependency/issue_300_function_table_composite_type/— tests a function using a table composite type parameter and a view return typetestdata/diff/dependency/issue_300_view_depends_on_view/— confirms view-to-view dependency ordering works correctly (already handled by existing topological sort)PGSCHEMA_TEST_FILTER="dependency/" go test -v ./cmd -run TestPlanAndApply— all 12 dependency tests passPGSCHEMA_TEST_FILTER="create_function/" go test -v ./cmd -run TestPlanAndApply— all function tests passPGSCHEMA_TEST_FILTER="create_view/" go test -v ./cmd -run TestPlanAndApply— all view tests pass🤖 Generated with Claude Code