diff --git a/cmd/dump/multifile_test.go b/cmd/dump/multifile_test.go index 22f2a5cf..347b8baa 100644 --- a/cmd/dump/multifile_test.go +++ b/cmd/dump/multifile_test.go @@ -8,6 +8,7 @@ import ( "github.com/pgschema/pgschema/internal/diff" "github.com/pgschema/pgschema/internal/dump" + "github.com/pgschema/pgschema/ir" ) func TestCreateMultiFileOutput(t *testing.T) { @@ -15,7 +16,7 @@ func TestCreateMultiFileOutput(t *testing.T) { tmpDir := t.TempDir() outputPath := filepath.Join(tmpDir, "schema.sql") - // Create test diffs directly + // Create test diffs with proper Source objects diffs := []diff.Diff{ { Statements: []diff.SQLStatement{ @@ -27,7 +28,9 @@ func TestCreateMultiFileOutput(t *testing.T) { Type: diff.DiffTypeType, Operation: diff.DiffOperationCreate, Path: "public.user_status", - Source: nil, + Source: &ir.Type{ + Name: "user_status", + }, }, { Statements: []diff.SQLStatement{ @@ -39,7 +42,9 @@ func TestCreateMultiFileOutput(t *testing.T) { Type: diff.DiffTypeTable, Operation: diff.DiffOperationCreate, Path: "public.users", - Source: nil, + Source: &ir.Table{ + Name: "users", + }, }, { Statements: []diff.SQLStatement{ @@ -51,7 +56,9 @@ func TestCreateMultiFileOutput(t *testing.T) { Type: diff.DiffTypeFunction, Operation: diff.DiffOperationCreate, Path: "public.get_user_count", - Source: nil, + Source: &ir.Function{ + Name: "get_user_count", + }, }, { Statements: []diff.SQLStatement{ @@ -63,7 +70,9 @@ func TestCreateMultiFileOutput(t *testing.T) { Type: diff.DiffTypeView, Operation: diff.DiffOperationCreate, Path: "public.active_users", - Source: nil, + Source: &ir.View{ + Name: "active_users", + }, }, } diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 128cf8c4..74f671bb 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -211,7 +211,7 @@ func (d *DiffOperation) UnmarshalJSON(data []byte) error { // DiffSource represents all possible source types for a diff type DiffSource interface { - IsDiffSource() // Marker method to constrain implementation + GetObjectName() string // Returns the object name (preserves names with dots like "public.idx_users") } // SQLStatement represents a single SQL statement with its transaction capability @@ -1397,17 +1397,17 @@ func referencesNewFunction(expr, defaultSchema string, newFunctions map[string]s return false } -// DiffSource interface implementations for diff types -func (d *schemaDiff) IsDiffSource() {} -func (d *functionDiff) IsDiffSource() {} -func (d *procedureDiff) IsDiffSource() {} -func (d *typeDiff) IsDiffSource() {} -func (d *sequenceDiff) IsDiffSource() {} -func (d *triggerDiff) IsDiffSource() {} -func (d *viewDiff) IsDiffSource() {} -func (d *tableDiff) IsDiffSource() {} -func (d *ColumnDiff) IsDiffSource() {} -func (d *ConstraintDiff) IsDiffSource() {} -func (d *IndexDiff) IsDiffSource() {} -func (d *policyDiff) IsDiffSource() {} -func (d *rlsChange) IsDiffSource() {} +// GetObjectName implementations for DiffSource interface +func (d *schemaDiff) GetObjectName() string { return d.New.Name } +func (d *functionDiff) GetObjectName() string { return d.New.Name } +func (d *procedureDiff) GetObjectName() string { return d.New.Name } +func (d *typeDiff) GetObjectName() string { return d.New.Name } +func (d *sequenceDiff) GetObjectName() string { return d.New.Name } +func (d *triggerDiff) GetObjectName() string { return d.New.Name } +func (d *viewDiff) GetObjectName() string { return d.New.Name } +func (d *tableDiff) GetObjectName() string { return d.Table.Name } +func (d *ColumnDiff) GetObjectName() string { return d.New.Name } +func (d *ConstraintDiff) GetObjectName() string { return d.New.Name } +func (d *IndexDiff) GetObjectName() string { return d.New.Name } +func (d *policyDiff) GetObjectName() string { return d.New.Name } +func (d *rlsChange) GetObjectName() string { return d.Table.Name } diff --git a/internal/dump/formatter.go b/internal/dump/formatter.go index 83de5fd7..2b31975b 100644 --- a/internal/dump/formatter.go +++ b/internal/dump/formatter.go @@ -391,8 +391,18 @@ func (f *DumpFormatter) formatObjectCommentHeader(step diff.Diff) string { // Determine schema name for comment commentSchemaName := f.getCommentSchemaName(step.Path) - // Get object name - objectName := f.getObjectName(step.Path) + // Get object name from source object to preserve names with dots + var objectName string + // Special handling for functions and procedures to include signature + switch obj := step.Source.(type) { + case *ir.Function: + objectName = obj.Name + "(" + obj.GetArguments() + ")" + case *ir.Procedure: + objectName = obj.Name + "(" + obj.GetArguments() + ")" + default: + // Use the GetObjectName interface method for all other types + objectName = step.Source.GetObjectName() + } parts := strings.Split(step.Type.String(), ".") objectType := parts[len(parts)-1] @@ -411,16 +421,6 @@ func (f *DumpFormatter) formatObjectCommentHeader(step diff.Diff) string { } } - // For functions and procedures, include the signature in the name to distinguish overloads - if step.Source != nil { - switch obj := step.Source.(type) { - case *ir.Function: - objectName = obj.Name + "(" + obj.GetArguments() + ")" - case *ir.Procedure: - objectName = obj.Name + "(" + obj.GetArguments() + ")" - } - } - output.WriteString(fmt.Sprintf("-- Name: %s; Type: %s; Schema: %s; Owner: -\n", objectName, displayType, commentSchemaName)) output.WriteString("--\n") output.WriteString("\n") diff --git a/internal/plan/plan.go b/internal/plan/plan.go index ae0812ca..208d45aa 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -687,6 +687,7 @@ func (p *Plan) writeTableChanges(summary *strings.Builder, c *color.Color) { operation string path string subType string + source diff.DiffSource }) // Track all seen operations globally to avoid duplicates across groups @@ -715,10 +716,12 @@ func (p *Plan) writeTableChanges(summary *strings.Builder, c *color.Color) { operation string path string subType string + source diff.DiffSource }{ operation: step.Operation.String(), path: step.Path, subType: step.Type.String(), + source: step.Source, }) } } @@ -775,11 +778,14 @@ func (p *Plan) writeTableChanges(summary *strings.Builder, c *color.Color) { }) for _, subRes := range subResourceList { + // Extract object name from source + objectName := getObjectNameFromSource(subRes.source) + // Handle online index replacement display if subRes.subType == diff.DiffTypeTableIndex.String() && subRes.operation == diff.DiffOperationAlter.String() { subSymbol := c.PlanSymbol("change") displaySubType := strings.TrimPrefix(subRes.subType, "table.") - fmt.Fprintf(summary, " %s %s (%s - concurrent rebuild)\n", subSymbol, getLastPathComponent(subRes.path), displaySubType) + fmt.Fprintf(summary, " %s %s (%s - concurrent rebuild)\n", subSymbol, objectName, displaySubType) continue } @@ -796,7 +802,7 @@ func (p *Plan) writeTableChanges(summary *strings.Builder, c *color.Color) { } // Clean up sub-resource type for display (remove "table." prefix) displaySubType := strings.TrimPrefix(subRes.subType, "table.") - fmt.Fprintf(summary, " %s %s (%s)\n", subSymbol, getLastPathComponent(subRes.path), displaySubType) + fmt.Fprintf(summary, " %s %s (%s)\n", subSymbol, objectName, displaySubType) } } } @@ -1118,6 +1124,15 @@ func getLastPathComponent(path string) string { return path } +// getObjectNameFromSource extracts the object name from the source object. +// This preserves object names that contain dots (e.g., "public.idx_users") +func getObjectNameFromSource(source diff.DiffSource) string { + if source == nil { + return "" + } + return source.GetObjectName() +} + // extractTablePathFromSubResource extracts the parent table, view, or materialized view path from a sub-resource path func extractTablePathFromSubResource(subResourcePath, subResourceType string) string { if strings.HasPrefix(subResourceType, "table.") { diff --git a/internal/postgres/desired_state.go b/internal/postgres/desired_state.go index 414915b6..5f6854cc 100644 --- a/internal/postgres/desired_state.go +++ b/internal/postgres/desired_state.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "fmt" "regexp" + "strings" "time" ) @@ -98,20 +99,77 @@ func stripSchemaQualifications(sql string, schemaName string) string { // Escape the schema name for use in regex escapedSchema := regexp.QuoteMeta(schemaName) - // Pattern matches: optional quote + schemaName + optional quote + dot + captured object name - // This handles all four combinations of quoted/unquoted schema and object names + // Pattern matches schema qualification and captures the object name + // We need to handle 4 cases: + // 1. unquoted_schema.unquoted_object -> unquoted_object + // 2. unquoted_schema."quoted_object" -> "quoted_object" + // 3. "quoted_schema".unquoted_object -> unquoted_object + // 4. "quoted_schema"."quoted_object" -> "quoted_object" + // + // Key: The dot must be outside quotes (a schema.object separator, not part of an identifier) + // Important: For unquoted schema patterns, we must ensure the schema name isn't inside a quoted identifier + // Example: Don't match 'public' in CREATE INDEX "public.idx" where the whole thing is a quoted identifier - // Pattern for unquoted identifier after dot - pattern1 := fmt.Sprintf(`"?%s"?\.([a-zA-Z_][a-zA-Z0-9_$]*)`, escapedSchema) + // Pattern 1: quoted schema + dot + quoted object: "schema"."object" + // Example: "public"."table" -> "table" + pattern1 := fmt.Sprintf(`"%s"\.(\"[^"]+\")`, escapedSchema) re1 := regexp.MustCompile(pattern1) - // Pattern for quoted identifier after dot - pattern2 := fmt.Sprintf(`"?%s"?\.(\"[^"]+\")`, escapedSchema) + // Pattern 2: quoted schema + dot + unquoted object: "schema".object + // Example: "public".table -> table + pattern2 := fmt.Sprintf(`"%s"\.([a-zA-Z_][a-zA-Z0-9_$]*)`, escapedSchema) re2 := regexp.MustCompile(pattern2) + // Pattern 3: unquoted schema + dot + quoted object: schema."object" + // Example: public."table" -> "table" + // Use negative lookbehind to ensure schema isn't preceded by a quote + // and negative lookahead to ensure the dot after schema isn't inside quotes + pattern3 := fmt.Sprintf(`(?:^|[^"])%s\.(\"[^"]+\")`, escapedSchema) + re3 := regexp.MustCompile(pattern3) + + // Pattern 4: unquoted schema + dot + unquoted object: schema.object + // Example: public.table -> table + // Use negative lookbehind to ensure schema isn't preceded by a quote + pattern4 := fmt.Sprintf(`(?:^|[^"])%s\.([a-zA-Z_][a-zA-Z0-9_$]*)`, escapedSchema) + re4 := regexp.MustCompile(pattern4) + result := sql + // Apply in order: quoted schema first to avoid double-matching result = re1.ReplaceAllString(result, "$1") result = re2.ReplaceAllString(result, "$1") + // For patterns 3 and 4, we need to preserve the character before the schema + result = re3.ReplaceAllStringFunc(result, func(match string) string { + // If match starts with a non-quote character, preserve it + if len(match) > 0 && match[0] != '"' { + // Extract the quote identifier (everything after schema.) + parts := strings.SplitN(match, ".", 2) + if len(parts) == 2 { + return string(match[0]) + parts[1] + } + } + // Otherwise just return the captured quoted identifier + parts := strings.SplitN(match, ".", 2) + if len(parts) == 2 { + return parts[1] + } + return match + }) + result = re4.ReplaceAllStringFunc(result, func(match string) string { + // If match starts with a non-quote character, preserve it + if len(match) > 0 && match[0] != '"' { + // Extract the unquoted identifier (everything after schema.) + parts := strings.SplitN(match, ".", 2) + if len(parts) == 2 { + return string(match[0]) + parts[1] + } + } + // Otherwise just return the captured unquoted identifier + parts := strings.SplitN(match, ".", 2) + if len(parts) == 2 { + return parts[1] + } + return match + }) return result } diff --git a/ir/ir.go b/ir/ir.go index e3f83147..9b668f10 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -557,16 +557,16 @@ func (s *Schema) SetType(name string, typ *Type) { s.Types[name] = typ } -// DiffSource interface implementations for IR types -func (t *Table) IsDiffSource() {} -func (c *Column) IsDiffSource() {} -func (c *Constraint) IsDiffSource() {} -func (i *Index) IsDiffSource() {} -func (t *Trigger) IsDiffSource() {} -func (p *RLSPolicy) IsDiffSource() {} -func (f *Function) IsDiffSource() {} -func (p *Procedure) IsDiffSource() {} -func (v *View) IsDiffSource() {} -func (s *Sequence) IsDiffSource() {} -func (t *Type) IsDiffSource() {} +// GetObjectName implementations for DiffSource interface +func (t *Table) GetObjectName() string { return t.Name } +func (c *Column) GetObjectName() string { return c.Name } +func (c *Constraint) GetObjectName() string { return c.Name } +func (i *Index) GetObjectName() string { return i.Name } +func (t *Trigger) GetObjectName() string { return t.Name } +func (p *RLSPolicy) GetObjectName() string { return p.Name } +func (f *Function) GetObjectName() string { return f.Name } +func (p *Procedure) GetObjectName() string { return p.Name } +func (v *View) GetObjectName() string { return v.Name } +func (s *Sequence) GetObjectName() string { return s.Name } +func (t *Type) GetObjectName() string { return t.Name } diff --git a/testdata/diff/create_index/add_index/diff.sql b/testdata/diff/create_index/add_index/diff.sql index 347c6ce3..6582e861 100644 --- a/testdata/diff/create_index/add_index/diff.sql +++ b/testdata/diff/create_index/add_index/diff.sql @@ -10,3 +10,5 @@ CREATE INDEX IF NOT EXISTS idx_users_email ON users (email varchar_pattern_ops); CREATE INDEX IF NOT EXISTS idx_users_id ON users (id); CREATE INDEX IF NOT EXISTS idx_users_name ON users (name); + +CREATE INDEX IF NOT EXISTS "public.idx_users" ON users (email, name); diff --git a/testdata/diff/create_index/add_index/new.sql b/testdata/diff/create_index/add_index/new.sql index 0bdd0af6..17d8e8f6 100644 --- a/testdata/diff/create_index/add_index/new.sql +++ b/testdata/diff/create_index/add_index/new.sql @@ -8,3 +8,5 @@ CREATE TABLE public.users ( CREATE INDEX idx_users_name ON public.users (name); CREATE INDEX idx_users_email ON public.users (email varchar_pattern_ops); CREATE INDEX idx_users_id ON public.users (id); +-- Test index name with dots (issue #196) +CREATE INDEX "public.idx_users" ON public.users (email, name); diff --git a/testdata/diff/create_index/add_index/plan.json b/testdata/diff/create_index/add_index/plan.json index c7aa8c4f..9e96fbb1 100644 --- a/testdata/diff/create_index/add_index/plan.json +++ b/testdata/diff/create_index/add_index/plan.json @@ -31,6 +31,12 @@ "type": "table.index", "operation": "create", "path": "public.users.idx_users_name" + }, + { + "sql": "CREATE INDEX IF NOT EXISTS \"public.idx_users\" ON users (email, name);", + "type": "table.index", + "operation": "create", + "path": "public.users.public.idx_users" } ] } diff --git a/testdata/diff/create_index/add_index/plan.sql b/testdata/diff/create_index/add_index/plan.sql index 347c6ce3..6582e861 100644 --- a/testdata/diff/create_index/add_index/plan.sql +++ b/testdata/diff/create_index/add_index/plan.sql @@ -10,3 +10,5 @@ CREATE INDEX IF NOT EXISTS idx_users_email ON users (email varchar_pattern_ops); CREATE INDEX IF NOT EXISTS idx_users_id ON users (id); CREATE INDEX IF NOT EXISTS idx_users_name ON users (name); + +CREATE INDEX IF NOT EXISTS "public.idx_users" ON users (email, name); diff --git a/testdata/diff/create_index/add_index/plan.txt b/testdata/diff/create_index/add_index/plan.txt index bd7fc091..50425870 100644 --- a/testdata/diff/create_index/add_index/plan.txt +++ b/testdata/diff/create_index/add_index/plan.txt @@ -8,6 +8,7 @@ Tables: + idx_users_email (index) + idx_users_id (index) + idx_users_name (index) + + public.idx_users (index) DDL to be executed: -------------------------------------------------- @@ -24,3 +25,5 @@ CREATE INDEX IF NOT EXISTS idx_users_email ON users (email varchar_pattern_ops); CREATE INDEX IF NOT EXISTS idx_users_id ON users (id); CREATE INDEX IF NOT EXISTS idx_users_name ON users (name); + +CREATE INDEX IF NOT EXISTS "public.idx_users" ON users (email, name); diff --git a/testdata/diff/create_table/add_table_like/plan.txt b/testdata/diff/create_table/add_table_like/plan.txt index 74973cfb..86830933 100644 --- a/testdata/diff/create_table/add_table_like/plan.txt +++ b/testdata/diff/create_table/add_table_like/plan.txt @@ -6,7 +6,7 @@ Summary by type: Tables: + products + users - + created_at (column.comment) + + users (column.comment) + users_created_at_idx (index) DDL to be executed: diff --git a/testdata/dump/issue_80_index_name_quote/pgschema.sql b/testdata/dump/issue_80_index_name_quote/pgschema.sql index 45106104..676d03bd 100644 --- a/testdata/dump/issue_80_index_name_quote/pgschema.sql +++ b/testdata/dump/issue_80_index_name_quote/pgschema.sql @@ -82,7 +82,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS "user email index" ON users (email); CREATE INDEX IF NOT EXISTS "user-status-index" ON users (status); -- --- Name: idx; Type: INDEX; Schema: -; Owner: - +-- Name: users.position.idx; Type: INDEX; Schema: -; Owner: - -- CREATE INDEX IF NOT EXISTS "users.position.idx" ON users ("position");