Skip to content

Comments

Add detailed cycle-breaking comments to topologicallySortTypes#206

Merged
tianzhou merged 3 commits intodependencyfrom
copilot/sub-pr-205
Dec 18, 2025
Merged

Add detailed cycle-breaking comments to topologicallySortTypes#206
tianzhou merged 3 commits intodependencyfrom
copilot/sub-pr-205

Conversation

Copy link
Contributor

Copilot AI commented Dec 18, 2025

The cycle-breaking logic in topologicallySortTypes lacked the detailed inline comments present in topologicallySortTables and topologicallySortViews, making it harder for maintainers to understand this critical algorithm.

Changes

  • Added comprehensive cycle-breaking strategy comments explaining:

    • Why setting inDegree[next] = 0 is safe (processed map prevents duplicates)
    • How type dependencies differ from table constraints (cannot be added post-creation)
    • PostgreSQL's restrictions on circular type dependencies (only via arrays/indirection)
    • Deterministic ordering via insertion order
  • Added explanation for the !processed[neighbor] check that prevents re-adding already-processed types to the queue

Example

The comments now match the structure and detail level of the table sorting function:

// CYCLE BREAKING STRATEGY FOR TYPES:
// Setting inDegree[next] = 0 effectively declares "this type has no remaining dependencies"
// for the purpose of breaking the cycle. This is safe because:
//
// 1. The 'processed' map prevents any type from being added to the result twice, even if
//    its inDegree becomes zero or negative multiple times (see line 344 check).
//
// 2. For circular type dependencies in PostgreSQL, the dependency cycle can only occur
//    through composite types referencing each other. Unlike table foreign keys, type
//    dependencies cannot be added after creation - the entire type definition must be
//    complete at CREATE TYPE time.
//
// 3. PostgreSQL itself prohibits creating types with true circular dependencies
//    (composite type A containing type B, which contains type A) because it would
//    result in infinite size. The only cycles that can occur in practice involve
//    array types or indirection (e.g., A contains B[], B contains A[]), which
//    PostgreSQL allows because arrays don't expand the size infinitely.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits December 18, 2025 06:46
Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com>
Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com>
Copilot AI changed the title [WIP] Update type topological sort implementation based on PR feedback Add detailed cycle-breaking comments to topologicallySortTypes Dec 18, 2025
Copilot AI requested a review from tianzhou December 18, 2025 06:50
@tianzhou tianzhou marked this pull request as ready for review December 18, 2025 06:54
@tianzhou tianzhou merged commit 8f8ba97 into dependency Dec 18, 2025
1 check passed
tianzhou added a commit that referenced this pull request Dec 18, 2025
* fix: type topological sort

* Add detailed cycle-breaking comments to topologicallySortTypes (#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 (#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>
@tianzhou tianzhou deleted the copilot/sub-pr-205 branch December 18, 2025 07:24
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants