-
Notifications
You must be signed in to change notification settings - Fork 29
Fix index name mutation for identifiers containing dots #213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #196 where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The fix ensures PostgreSQL quoted identifiers with dots are properly preserved throughout the migration pipeline.
Key changes:
- Replaced the
IsDiffSource()marker method withGetObjectName()to provide consistent name extraction across all diff and IR types - Enhanced
stripSchemaQualifications()regex patterns to avoid matching inside quoted identifiers - Updated plan summary and dump formatter to use source objects directly instead of path parsing
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 |
|---|---|
| testdata/diff/create_index/add_index/new.sql | Adds test case for index with dots in name: "public.idx_users" |
| testdata/diff/create_index/add_index/plan.txt | Expected plan output showing the new index correctly displayed |
| testdata/diff/create_index/add_index/plan.sql | Expected SQL output with properly quoted index name |
| testdata/diff/create_index/add_index/plan.json | Expected JSON output showing the new index in migration plan |
| testdata/diff/create_index/add_index/diff.sql | Expected diff SQL with properly quoted index creation |
| ir/ir.go | Implements GetObjectName() for all IR types to replace the removed IsDiffSource() marker method |
| internal/postgres/desired_state.go | Rewrites stripSchemaQualifications() regex to handle quoted identifiers containing dots |
| internal/plan/plan.go | Adds source tracking for sub-resources and uses GetObjectName() for display |
| internal/dump/formatter.go | Updates formatObjectCommentHeader() to use source object's GetObjectName() instead of path parsing |
| internal/diff/diff.go | Updates DiffSource interface with GetObjectName() method and implements it for all diff types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This fixes an issue where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The bug manifested in three places: 1. Comment header generation in dump output 2. Schema qualification stripping when applying SQL to temporary schemas 3. Plan summary display Changes: - Added GetObjectName() method to DiffSource interface for consistent name extraction - Removed unused IsDiffSource() marker method from interface - Fixed formatObjectCommentHeader() to use Source object instead of path parsing - Rewrote stripSchemaQualifications() regex to avoid matching inside quoted identifiers - Enhanced plan.go to track and use source objects for sub-resources - Added test case for index with dots in name: "public.idx_users" The fix ensures that PostgreSQL quoted identifiers containing dots are properly preserved throughout the dump/plan/apply pipeline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
13e503d to
d8f9ba1
Compare
Updated TestCreateMultiFileOutput to create proper IR objects (Type, Table, Function, View) as Source instead of nil. This allows us to remove the fallback logic in formatObjectCommentHeader() that was extracting names from paths. Changes: - cmd/dump/multifile_test.go: Added ir.Type, ir.Table, ir.Function, ir.View as Source objects for test diffs - internal/dump/formatter.go: Removed fallback to getObjectName(path) since all code paths now provide proper Source objects This makes the code cleaner and ensures all diffs have proper type information available for formatting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Fix index name mutation for identifiers containing dots (pgplex#196) This fixes an issue where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The bug manifested in three places: 1. Comment header generation in dump output 2. Schema qualification stripping when applying SQL to temporary schemas 3. Plan summary display Changes: - Added GetObjectName() method to DiffSource interface for consistent name extraction - Removed unused IsDiffSource() marker method from interface - Fixed formatObjectCommentHeader() to use Source object instead of path parsing - Rewrote stripSchemaQualifications() regex to avoid matching inside quoted identifiers - Enhanced plan.go to track and use source objects for sub-resources - Added test case for index with dots in name: "public.idx_users" The fix ensures that PostgreSQL quoted identifiers containing dots are properly preserved throughout the dump/plan/apply pipeline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Refactor test to use proper Source objects instead of nil fallback Updated TestCreateMultiFileOutput to create proper IR objects (Type, Table, Function, View) as Source instead of nil. This allows us to remove the fallback logic in formatObjectCommentHeader() that was extracting names from paths. Changes: - cmd/dump/multifile_test.go: Added ir.Type, ir.Table, ir.Function, ir.View as Source objects for test diffs - internal/dump/formatter.go: Removed fallback to getObjectName(path) since all code paths now provide proper Source objects This makes the code cleaner and ensures all diffs have proper type information available for formatting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Fix #196
This fixes an issue where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The bug manifested in three places:
Changes:
The fix ensures that PostgreSQL quoted identifiers containing dots are properly preserved throughout the dump/plan/apply pipeline.
🤖 Generated with Claude Code