fix: topological sort for modified tables' constraint dependencies #249

tianzhou · 2026-01-16T17:08:08Z

Modified tables are now sorted topologically based on constraint dependencies, ensuring that UNIQUE/PK constraints are added before foreign keys that reference them.

Previously, modified tables were sorted only alphabetically, which caused failures when a table with a newly-added FK came before the table with the newly-added UNIQUE constraint it references (e.g., a_employees before z_companies).

This fix implements topologicallySortModifiedTables() which:

Detects when table A adds FK → table B's newly-added UNIQUE/PK
Creates dependency edge: B → A (B must be processed first)
Uses Kahn's algorithm for topological sort
Falls back to alphabetical for cycle breaking

Enhanced testdata/diff/online/add_fk/ to expose and verify the fix:

Renamed tables to z_companies and a_employees (reverse alphabetical)
Added scenario testing FK to newly-added UNIQUE constraint
Added comments explaining the test purpose

) Modified tables are now sorted topologically based on constraint dependencies, ensuring that UNIQUE/PK constraints are added before foreign keys that reference them. Previously, modified tables were sorted only alphabetically, which caused failures when a table with a newly-added FK came before the table with the newly-added UNIQUE constraint it references (e.g., a_employees before z_companies). This fix implements topologicallySortModifiedTables() which: - Detects when table A adds FK → table B's newly-added UNIQUE/PK - Creates dependency edge: B → A (B must be processed first) - Uses Kahn's algorithm for topological sort - Falls back to alphabetical for cycle breaking Enhanced testdata/diff/online/add_fk/ to expose and verify the fix: - Renamed tables to z_companies and a_employees (reverse alphabetical) - Added scenario testing FK to newly-added UNIQUE constraint - Added comments explaining the test purpose Fixes issue #248: "ERROR: there is no unique constraint matching given keys for referenced table" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR fixes a critical bug (#248) where modified tables with newly-added foreign keys and unique constraints were processed in alphabetical order, causing failures when a foreign key referencing a newly-added unique/primary key constraint was added before the constraint itself. The fix implements topological sorting for modified tables based on constraint dependencies using Kahn's algorithm.

Changes:

Implements topologicallySortModifiedTables() function to sort modified tables based on constraint dependencies
Adds constraintMatchesFKReference() helper to match foreign keys with their referenced unique/primary key constraints
Updates test data to verify the fix with intentionally reversed alphabetical table names

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/diff/topological.go	Adds topological sorting logic for modified tables with constraint dependency detection
internal/diff/diff.go	Integrates topological sorting into the migration generation flow
testdata/diff/online/add_fk/old.sql	Updates test case with reversed table names and documentation
testdata/diff/online/add_fk/new.sql	Adds scenario testing FK to newly-added UNIQUE constraint
testdata/diff/online/add_fk/plan.txt	Updates expected plan output
testdata/diff/online/add_fk/plan.sql	Updates expected SQL output
testdata/diff/online/add_fk/plan.json	Updates expected JSON plan
testdata/diff/online/add_fk/diff.sql	Updates expected diff output

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/diff/topological.go

internal/diff/diff.go

Address PR review feedback: 1. Fix constraintMatchesFKReference to use order-preserving comparison - PostgreSQL requires composite FK columns to match UNIQUE/PK columns in exact order - Previous set-based approach incorrectly matched FK (col1, col2) with UNIQUE (col2, col1) - Now uses position-by-position comparison after sorting by column position 2. Add comment explaining alphabetical pre-sort for modified tables - Clarifies that pre-sort ensures deterministic insertion order for cycle breaking - Consistent with comment pattern used for added tables (line 1262) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-16T17:30:26Z

internal/diff/topological.go

+// topologicallySortModifiedTables sorts modified tables based on constraint dependencies
+// Tables with added UNIQUE/PK constraints that are referenced by other tables' added FKs
+// will come before those tables
+func topologicallySortModifiedTables(tableDiffs []*tableDiff) []*tableDiff {


The topologicallySortModifiedTables() function lacks unit test coverage. Other topological sorting functions in this file have comprehensive unit tests (e.g., TestTopologicallySortTablesHandlesCycles, TestTopologicallySortTypesHandlesCycles). Consider adding similar tests to verify: (1) correct dependency ordering when table A's FK references table B's newly-added UNIQUE constraint, (2) cycle detection and breaking, (3) deterministic ordering when there are no dependencies, and (4) self-reference handling.

Copilot · 2026-01-16T17:30:26Z

internal/diff/topological.go

+// constraintMatchesFKReference checks if a UNIQUE/PK constraint matches the columns
+// referenced by a foreign key constraint.
+// In PostgreSQL, composite foreign keys must reference columns in the same order as they
+// appear in the referenced unique/primary key constraint.
+// For example, FK (col1, col2) can only reference UNIQUE (col1, col2), not UNIQUE (col2, col1).
+func constraintMatchesFKReference(uniqueConstraint, fkConstraint *ir.Constraint) bool {


The constraintMatchesFKReference() helper function lacks unit test coverage. Consider adding tests to verify: (1) matching single-column constraints, (2) matching multi-column constraints with correct order, (3) non-matching constraints with different column orders, (4) non-matching constraints with different column counts, and (5) handling of position-based column ordering.

Copilot · 2026-01-16T17:30:27Z

internal/diff/topological.go

+	for len(result) < len(tableDiffMap) {
+		if len(queue) == 0 {
+			// Cycle detected: pick the next unprocessed table using original insertion order
+			next := nextInOrder(insertionOrder, processed)
+			if next == "" {
+				break
+			}
+			queue = append(queue, next)
+			inDegree[next] = 0
+		}


The cycle-breaking strategy lacks the detailed explanation found in similar functions (see topologicallySortTables lines 69-87 and topologicallySortFunctions lines 458-476). For consistency and maintainability, consider adding a similar detailed comment block explaining: (1) why setting inDegree[next] = 0 is safe, (2) how the processed map prevents duplicate processing, (3) why cycle breaking works for modified tables with FK/UNIQUE constraint dependencies, and (4) that using insertion order ensures deterministic output.

…gplex#249) * fix: topological sort for modified tables' constraint dependencies (pgplex#248) Modified tables are now sorted topologically based on constraint dependencies, ensuring that UNIQUE/PK constraints are added before foreign keys that reference them. Previously, modified tables were sorted only alphabetically, which caused failures when a table with a newly-added FK came before the table with the newly-added UNIQUE constraint it references (e.g., a_employees before z_companies). This fix implements topologicallySortModifiedTables() which: - Detects when table A adds FK → table B's newly-added UNIQUE/PK - Creates dependency edge: B → A (B must be processed first) - Uses Kahn's algorithm for topological sort - Falls back to alphabetical for cycle breaking Enhanced testdata/diff/online/add_fk/ to expose and verify the fix: - Renamed tables to z_companies and a_employees (reverse alphabetical) - Added scenario testing FK to newly-added UNIQUE constraint - Added comments explaining the test purpose Fixes issue pgplex#248: "ERROR: there is no unique constraint matching given keys for referenced table" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: use order-preserving column comparison for FK constraint matching Address PR review feedback: 1. Fix constraintMatchesFKReference to use order-preserving comparison - PostgreSQL requires composite FK columns to match UNIQUE/PK columns in exact order - Previous set-based approach incorrectly matched FK (col1, col2) with UNIQUE (col2, col1) - Now uses position-by-position comparison after sorting by column position 2. Add comment explaining alphabetical pre-sort for modified tables - Clarifies that pre-sort ensures deterministic insertion order for cycle breaking - Consistent with comment pattern used for added tables (line 1262) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings January 16, 2026 17:08

Copilot started reviewing on behalf of tianzhou January 16, 2026 17:08 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

internal/diff/topological.go Outdated Show resolved Hide resolved

internal/diff/diff.go Show resolved Hide resolved

tianzhou requested a review from Copilot January 16, 2026 17:27

Copilot started reviewing on behalf of tianzhou January 16, 2026 17:27 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

tianzhou merged commit f056ad5 into main Jan 16, 2026
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: topological sort for modified tables' constraint dependencies #249

fix: topological sort for modified tables' constraint dependencies #249

Uh oh!

tianzhou commented Jan 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix: topological sort for modified tables' constraint dependencies #249

fix: topological sort for modified tables' constraint dependencies #249

Uh oh!

Conversation

tianzhou commented Jan 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant