Skip to content

Conversation

@tianzhou
Copy link
Contributor

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:

  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

Copilot AI review requested due to automatic review settings December 18, 2025 16:11
Copy link
Contributor

Copilot AI left a 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 with GetObjectName() 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>
@tianzhou tianzhou force-pushed the fix-index-name-mutation branch from 13e503d to d8f9ba1 Compare December 18, 2025 16:27
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>
@tianzhou tianzhou requested a review from Copilot December 18, 2025 16:39
Copy link
Contributor

Copilot AI left a 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.

@tianzhou tianzhou merged commit 0ad344f into main Dec 18, 2025
7 of 8 checks passed
@tianzhou tianzhou deleted the fix-index-name-mutation branch January 5, 2026 14:12
alecthomas pushed a commit to alecthomas/pgschema that referenced this pull request Jan 26, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

index names are improperly mutated when they contain the name of the schema in them

1 participant