Skip to content

refactor: convert multi-pass to single-pass in plan module #245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tianzhou merged 2 commits into from
Jan 15, 2026
Merged

refactor: convert multi-pass to single-pass in plan module #245

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cde4873
refactor: convert multi-pass to single-pass in plan module
tianzhou Jan 15, 2026
5371679
fix: correct comment about view sub-resource types
tianzhou Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 14 additions & 43 deletions internal/plan/plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,26 +150,21 @@ func groupDiffs(diffs []diff.Diff) []ExecutionGroup {
var groups []ExecutionGroup
var transactionalSteps []Step

// Track newly created tables to avoid concurrent rewrites for their indexes
// Track newly created tables/materialized views to avoid concurrent rewrites for their indexes.
// Single-pass: diffs are topologically sorted, so creates come before dependent index operations.
// We build these maps incrementally as we process each diff.
newlyCreatedTables := make(map[string]bool)
newlyCreatedMaterializedViews := make(map[string]bool)

// Convert diffs to steps
for _, d := range diffs {
// Track creates as we encounter them (before processing dependent operations)
if d.Type == diff.DiffTypeTable && d.Operation == diff.DiffOperationCreate {
// Extract table name from path (schema.table)
newlyCreatedTables[d.Path] = true
}
}

// Track newly created materialized views to avoid concurrent rewrites for their indexes
newlyCreatedMaterializedViews := make(map[string]bool)
for _, d := range diffs {
if d.Type == diff.DiffTypeMaterializedView && d.Operation == diff.DiffOperationCreate {
// Extract materialized view name from path (schema.materialized_view)
newlyCreatedMaterializedViews[d.Path] = true
}
}

// Convert diffs to steps
for _, d := range diffs {
// Try to generate rewrites if online operations are enabled
rewriteSteps := generateRewrite(d, newlyCreatedTables, newlyCreatedMaterializedViews)

Expand Down Expand Up @@ -467,33 +462,8 @@ func (p *Plan) calculateSummaryFromSteps() PlanSummary {
}
}

// First pass: identify all views and materialized views to distinguish them from tables
viewPaths := make(map[string]bool)
materializedViewPaths := make(map[string]bool)
for _, step := range dataToProcess {
stepObjTypeStr := step.Type
if !strings.HasSuffix(stepObjTypeStr, "s") {
stepObjTypeStr += "s"
}
if stepObjTypeStr == "views" {
viewPaths[step.Path] = true
} else if stepObjTypeStr == "materialized_views" {
materializedViewPaths[step.Path] = true
} else if strings.HasPrefix(step.Type, "view.") {
// For view sub-resources, extract the parent view path
parentPath := extractTablePathFromSubResource(step.Path, step.Type)
if parentPath != "" {
viewPaths[parentPath] = true
}
} else if strings.HasPrefix(step.Type, "materialized_view.") {
// For materialized view sub-resources, extract the parent path
parentPath := extractTablePathFromSubResource(step.Path, step.Type)
if parentPath != "" {
materializedViewPaths[parentPath] = true
}
}
}

// 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"
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copilot uses AI. Check for mistakes.
for _, step := range dataToProcess {
// Normalize object type to match the expected format (add 's' for plural)
stepObjTypeStr := step.Type
Expand All @@ -515,17 +485,18 @@ func (p *Plan) calculateSummaryFromSteps() PlanSummary {
}
materializedViewOperations[step.Path] = step.Operation
} else if isSubResource(step.Type) {
// For sub-resources, check if parent is a view, materialized view, or table
// For sub-resources, determine parent type from step.Type prefix
// Types are: "table.index", "table.column", "view.comment", "materialized_view.index", etc.
parentPath := extractTablePathFromSubResource(step.Path, step.Type)
if parentPath != "" {
if materializedViewPaths[parentPath] {
if strings.HasPrefix(step.Type, "materialized_view.") {
// Parent is a materialized view
materializedViewsWithSubResources[parentPath] = true
} else if viewPaths[parentPath] {
} else if strings.HasPrefix(step.Type, "view.") {
// Parent is a view
viewsWithSubResources[parentPath] = true
} else {
// Parent is a table
// Parent is a table (table.index, table.column, table.constraint, etc.)
tablesWithSubResources[parentPath] = true
}
}
Expand Down