From da80421a57494688a0f187a7ce2b64f88605d6fe Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 10 Nov 2025 22:44:43 -0800 Subject: [PATCH 1/2] chore: fix regex --- internal/diff/diff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index f87e08b4..906bf5ac 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -1171,7 +1171,7 @@ func buildFunctionLookup(functions []*ir.Function) map[string]struct{} { return lookup } -var functionCallRegex = regexp.MustCompile(`(?i)([a-z_][a-z0-9_$.]*)\s*\(`) +var functionCallRegex = regexp.MustCompile(`(?i)([a-z_][a-z0-9_$]*(?:\.[a-z_][a-z0-9_$]*)*)\s*\(`) // policyReferencesNewFunction determines if a policy references any newly added functions. func policyReferencesNewFunction(policy *ir.RLSPolicy, newFunctions map[string]struct{}) bool { From b7b61fea73e9d892e712d39772cffa7c1352a683 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 10 Nov 2025 23:04:57 -0800 Subject: [PATCH 2/2] chore: address review comments --- internal/diff/table.go | 6 +++--- internal/diff/topological.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/internal/diff/table.go b/internal/diff/table.go index 0aaf8c46..6a278472 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -540,7 +540,7 @@ func generateTableSQL(table *ir.Table, targetSchema string, createdTables map[st var deferred []*deferredConstraint currentKey := fmt.Sprintf("%s.%s", table.Schema, table.Name) for _, constraint := range inlineConstraints { - if shouldDeferConstraint(constraint, currentKey, createdTables, existingTables) { + if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) { deferred = append(deferred, &deferredConstraint{ table: table, constraint: constraint, @@ -565,14 +565,14 @@ func generateTableSQL(table *ir.Table, targetSchema string, createdTables map[st return strings.Join(parts, "\n"), deferred } -func shouldDeferConstraint(constraint *ir.Constraint, currentKey string, createdTables map[string]bool, existingTables map[string]bool) bool { +func shouldDeferConstraint(table *ir.Table, constraint *ir.Constraint, currentKey string, createdTables map[string]bool, existingTables map[string]bool) bool { if constraint == nil || constraint.Type != ir.ConstraintTypeForeignKey { return false } refSchema := constraint.ReferencedSchema if refSchema == "" { - refSchema = constraint.Schema + refSchema = table.Schema } if constraint.ReferencedTable == "" { return false diff --git a/internal/diff/topological.go b/internal/diff/topological.go index 41861f6d..34adadd0 100644 --- a/internal/diff/topological.go +++ b/internal/diff/topological.go @@ -68,6 +68,23 @@ func topologicallySortTables(tables []*ir.Table) []*ir.Table { for len(result) < len(tableMap) { if len(queue) == 0 { // Cycle detected: pick the next unprocessed table using original insertion order + // + // CYCLE BREAKING STRATEGY: + // Setting inDegree[next] = 0 effectively declares "this table has no remaining dependencies" + // for the purpose of breaking the cycle. This is safe because: + // + // 1. The 'processed' map prevents any table from being added to the result twice, even if + // its inDegree becomes zero or negative multiple times (see line 92 check). + // + // 2. For circular foreign key dependencies (e.g., A↔B), the table creation order doesn't + // matter because pgschema follows PostgreSQL's pattern of creating tables first and + // adding foreign key constraints afterwards via ALTER TABLE statements. + // + // 3. Using insertion order (alphabetical by schema.name) ensures deterministic output + // when multiple valid orderings exist. + // + // This approach aligns with PostgreSQL's pg_dump, which breaks dependency cycles by + // separating table creation from constraint creation. next := nextInOrder(insertionOrder, processed) if next == "" { break @@ -89,6 +106,10 @@ func topologicallySortTables(tables []*ir.Table) []*ir.Table { for _, neighbor := range neighbors { inDegree[neighbor]-- + // Add neighbor to queue if all its dependencies are satisfied. + // The '!processed[neighbor]' check is critical: it prevents re-adding tables + // that have already been processed, even if their inDegree becomes <= 0 again + // due to cycle breaking (where we artificially set inDegree to 0). if inDegree[neighbor] <= 0 && !processed[neighbor] { queue = append(queue, neighbor) sort.Strings(queue) @@ -155,6 +176,10 @@ func topologicallySortViews(views []*ir.View) []*ir.View { for len(result) < len(viewMap) { if len(queue) == 0 { + // Cycle detected: See detailed explanation in topologicallySortTables. + // Views with circular dependencies are uncommon but possible via recursive CTEs + // or mutual references in view definitions. We apply the same cycle-breaking + // strategy: pick next in insertion order and set inDegree to 0. next := nextInOrder(insertionOrder, processed) if next == "" { break @@ -176,6 +201,10 @@ func topologicallySortViews(views []*ir.View) []*ir.View { for _, neighbor := range neighbors { inDegree[neighbor]-- + // Add neighbor to queue if all its dependencies are satisfied. + // The '!processed[neighbor]' check is critical: it prevents re-adding views + // that have already been processed, even if their inDegree becomes <= 0 again + // due to cycle breaking (where we artificially set inDegree to 0). if inDegree[neighbor] <= 0 && !processed[neighbor] { queue = append(queue, neighbor) sort.Strings(queue)