fix: normalize schema qualifier in function arguments#160
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds normalization of schema-qualified type names in function and procedure parameters. When PostgreSQL returns type names with schema qualifications (e.g., public.order_status), the code now strips the schema prefix if it matches the routine's schema, while preserving cross-schema references (e.g., utils.priority_level remains qualified when the function is in public schema).
Key changes:
- Added
stripSameSchemaPrefixmethod to normalize type names by removing same-schema qualifications - Updated
parseParametersFromSignatureto accept aroutineSchemaparameter and normalize parameter types - Test data files updated to demonstrate the functionality with functions having parameters from both same-schema and cross-schema types
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ir/inspector.go | Added stripSameSchemaPrefix method and updated parseParametersFromSignature to normalize parameter type names by schema context |
| testdata/diff/create_function/alter_function_same_signature/plan.txt | Updated test data with additional function parameters using custom types |
| testdata/diff/create_function/alter_function_same_signature/plan.sql | Updated test SQL with additional function parameters using custom types |
| testdata/diff/create_function/alter_function_same_signature/plan.json | Updated JSON plan with new pgschema version, fingerprint, and SQL containing additional parameters |
| testdata/diff/create_function/alter_function_same_signature/old.sql | Added enum type definition and additional function parameters to old schema state |
| testdata/diff/create_function/alter_function_same_signature/new.sql | Added enum type definition and additional function parameters with comment to new schema state |
| testdata/diff/create_function/alter_function_same_signature/diff.sql | Updated diff output showing additional function parameters |
Comments suppressed due to low confidence (1)
ir/inspector.go:1057
- When parsing multi-word type names (e.g.,
character varying(255)), this code joins all parts after the parameter name with spaces. However, thestripSameSchemaPrefixfunction on line 1065 expects a single type identifier and won't correctly handle schema prefixes in complex types likepublic.character varying(255). This could cause the schema prefix to remain when it should be stripped. Consider parsing the first word of the type separately for schema stripping, then reconstructing the full type.
param.DataType = strings.Join(remainingParts[1:], " ")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a17cbd1 to
7160ec4
Compare
7160ec4 to
6e5676e
Compare
Contributor
Author
|
cc @p-c-h-b |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix issue 2 of #155
Fix developed based on #158