refactor: convert multi-pass to single-pass in plan module #245
Conversation
- groupDiffs: merge three loops into one by tracking creates incrementally (diffs are topologically sorted, so creates come before dependent operations) - calculateSummaryFromSteps: remove pre-scan loop by determining parent type directly from step.Type prefix (table.index, view.index, materialized_view.index) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors two functions in the plan module to convert multi-pass loops into single-pass operations for improved performance and code simplicity.
Changes:
- Merged three separate loops in
groupDiffsinto one by tracking creates incrementally - Eliminated a pre-scan loop in
calculateSummaryFromStepsby determining parent type directly from thestep.Typeprefix - Updated comments to reflect the single-pass approach
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Single-pass: process all steps, determining parent type from step.Type prefix | ||
| // Sub-resource types encode their parent: "table.index", "view.index", "materialized_view.index" |
There was a problem hiding this comment.
The comment mentions "view.index" as a sub-resource type, but based on the DiffType constants in internal/diff/diff.go, views only have "view.comment" as a sub-resource. Regular views don't support indexes in PostgreSQL (only materialized views do). The comment should be updated to reflect the actual sub-resource types: "table.index", "view.comment", "materialized_view.index".
Views don't support indexes in PostgreSQL - only materialized views do. Updated comment to show accurate examples: table.index, table.column, view.comment, materialized_view.index. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* refactor: convert multi-pass to single-pass in plan module - groupDiffs: merge three loops into one by tracking creates incrementally (diffs are topologically sorted, so creates come before dependent operations) - calculateSummaryFromSteps: remove pre-scan loop by determining parent type directly from step.Type prefix (table.index, view.index, materialized_view.index) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: correct comment about view sub-resource types Views don't support indexes in PostgreSQL - only materialized views do. Updated comment to show accurate examples: table.index, table.column, view.comment, materialized_view.index. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Related #240