fix: type topological sort #205
Merged
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #195 by implementing topological sorting for PostgreSQL type creation to ensure types are created in the correct dependency order. Previously, types were sorted alphabetically (with domains last), which could fail when composite types reference other custom types like enums.
Key changes:
- Replaced simple alphabetical sorting with dependency-aware topological sorting using Kahn's algorithm
- Added support for detecting type-to-type dependencies in composite types and domain types
- Added test case demonstrating correct ordering of enum and composite types with dependencies
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/diff/type.go | Replaced alphabetical sorting logic with call to new topologicallySortTypes function |
| internal/diff/topological.go | Added topologicallySortTypes, extractTypeName, and findLastDot functions to implement dependency-aware type sorting |
| testdata/diff/dependency/type_to_type/new.sql | Test input with composite type referencing enum type |
| testdata/diff/dependency/type_to_type/old.sql | Empty schema baseline for test |
| testdata/diff/dependency/type_to_type/plan.sql | Expected SQL output with correct type ordering (enum before composite) |
| testdata/diff/dependency/type_to_type/plan.txt | Human-readable plan showing the expected migration steps |
| testdata/diff/dependency/type_to_type/plan.json | JSON format of the migration plan |
| testdata/diff/dependency/type_to_type/diff.sql | Generated diff SQL matching expected plan |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Contributor
* Initial plan * Add detailed cycle-breaking comments to topologicallySortTypes Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Fix line reference in topologicallySortTypes comment Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com>
* Initial plan * Add comprehensive unit tests for topologicallySortTypes function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Remove duplicate newTestCompositeTypeMultiple helper function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 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.
alecthomas
pushed a commit
to alecthomas/pgschema
that referenced
this pull request
Jan 26, 2026
* fix: type topological sort * Add detailed cycle-breaking comments to topologicallySortTypes (pgplex#206) * Initial plan * Add detailed cycle-breaking comments to topologicallySortTypes Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Fix line reference in topologicallySortTypes comment Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Add unit tests for topologicallySortTypes function (pgplex#207) * Initial plan * Add comprehensive unit tests for topologicallySortTypes function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Remove duplicate newTestCompositeTypeMultiple helper function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
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 #195