diff --git a/internal/diff/diff.go b/internal/diff/diff.go index e2827070..66331d49 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -1528,10 +1528,19 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto key := fmt.Sprintf("%s.%s", tableDiff.Table.Schema, tableDiff.Table.Name) existingTables[key] = true } + // Build lookup of all new table names (qualified) for policy deferral (#373). + // Policies that reference other new tables must be deferred until all tables exist. + newTableLookup := make(map[string]struct{}, len(d.addedTables)) + for _, table := range d.addedTables { + newTableLookup[fmt.Sprintf("%s.%s", strings.ToLower(table.Schema), strings.ToLower(table.Name))] = struct{}{} + } var shouldDeferPolicy func(*ir.RLSPolicy) bool - if len(newFunctionLookup) > 0 { + if len(newFunctionLookup) > 0 || len(newTableLookup) > 0 { shouldDeferPolicy = func(policy *ir.RLSPolicy) bool { - return policyReferencesNewFunction(policy, newFunctionLookup) + if policyReferencesNewFunction(policy, newFunctionLookup) { + return true + } + return policyReferencesOtherNewTable(policy, newTableLookup) } } @@ -2028,6 +2037,38 @@ func policyReferencesNewFunction(policy *ir.RLSPolicy, newFunctions map[string]s return false } +// policyReferencesOtherNewTable determines if a policy's USING or WITH CHECK expressions +// reference any newly added table other than the policy's own table (#373). +// newTables keys are fully qualified (schema.table) to avoid cross-schema collisions. +func policyReferencesOtherNewTable(policy *ir.RLSPolicy, newTables map[string]struct{}) bool { + if len(newTables) == 0 || policy == nil { + return false + } + + ownQualified := fmt.Sprintf("%s.%s", strings.ToLower(policy.Schema), strings.ToLower(policy.Table)) + + for _, expr := range []string{policy.Using, policy.WithCheck} { + if expr == "" { + continue + } + exprLower := strings.ToLower(expr) + for qualifiedName := range newTables { + // Skip the policy's own table + if qualifiedName == ownQualified { + continue + } + // Extract the unqualified table name for substring matching. + // Policy expressions may use unqualified or qualified references. + parts := strings.SplitN(qualifiedName, ".", 2) + tableName := parts[len(parts)-1] + if strings.Contains(exprLower, tableName) { + return true + } + } + } + return false +} + // tableUsesDeferredDomain determines if a table uses any deferred domain types in its columns. func tableUsesDeferredDomain(table *ir.Table, deferredDomains map[string]struct{}) bool { if len(deferredDomains) == 0 || table == nil { diff --git a/internal/diff/policy.go b/internal/diff/policy.go index fa46e023..5e4006bc 100644 --- a/internal/diff/policy.go +++ b/internal/diff/policy.go @@ -10,10 +10,18 @@ import ( // generateCreatePoliciesSQL generates CREATE POLICY statements func generateCreatePoliciesSQL(policies []*ir.RLSPolicy, targetSchema string, collector *diffCollector) { - // Sort policies by name for consistent ordering + // Sort policies by (schema, table, name) for deterministic ordering across tables. + // Policy names are only unique per table, so sorting by name alone is insufficient + // when policies from multiple tables are collected together (#373). sortedPolicies := make([]*ir.RLSPolicy, len(policies)) copy(sortedPolicies, policies) sort.Slice(sortedPolicies, func(i, j int) bool { + if sortedPolicies[i].Schema != sortedPolicies[j].Schema { + return sortedPolicies[i].Schema < sortedPolicies[j].Schema + } + if sortedPolicies[i].Table != sortedPolicies[j].Table { + return sortedPolicies[i].Table < sortedPolicies[j].Table + } return sortedPolicies[i].Name < sortedPolicies[j].Name }) diff --git a/internal/diff/table.go b/internal/diff/table.go index bf5acd02..20d6cdf2 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -384,8 +384,9 @@ type deferredConstraint struct { } // generateCreateTablesSQL generates CREATE TABLE statements with co-located indexes, policies, and RLS. -// Policies that reference newly added helper functions are collected for deferred creation after -// dependent functions/procedures have been created, while all other policies are emitted inline. +// Policies that reference other new tables in the same migration or newly added functions +// (via USING/WITH CHECK expressions) are deferred for creation after all tables and functions +// exist (#373). All other policies are emitted inline. // It returns deferred policies and foreign key constraints that should be applied after dependent objects exist. // Tables are assumed to be pre-sorted in topological order for dependency-aware creation. func generateCreateTablesSQL( @@ -474,7 +475,8 @@ func generateCreateTablesSQL( generateRLSChangesSQL(rlsChanges, targetSchema, collector) } - // Collect policies that can run immediately; defer only those that depend on new helper functions + // Collect policies: defer those that reference other new tables or new functions (#373), + // emit the rest inline with their parent table. if len(table.Policies) > 0 { var inlinePolicies []*ir.RLSPolicy policyNames := sortedKeys(table.Policies) diff --git a/testdata/diff/dependency/issue_373_policy_references_other_table/diff.sql b/testdata/diff/dependency/issue_373_policy_references_other_table/diff.sql new file mode 100644 index 00000000..8603680e --- /dev/null +++ b/testdata/diff/dependency/issue_373_policy_references_other_table/diff.sql @@ -0,0 +1,15 @@ +CREATE TABLE IF NOT EXISTS manager ( + id SERIAL, + user_id uuid NOT NULL +); + +ALTER TABLE manager ENABLE ROW LEVEL SECURITY; + +CREATE TABLE IF NOT EXISTS project_manager ( + id SERIAL, + project_id integer NOT NULL, + manager_id integer NOT NULL, + is_deleted boolean DEFAULT false NOT NULL +); + +CREATE POLICY employee_manager_select ON manager FOR SELECT TO PUBLIC USING (id IN ( SELECT pam.manager_id FROM project_manager pam WHERE ((pam.project_id IN ( SELECT unnest(ARRAY[1, 2, 3]) AS unnest)) AND (pam.is_deleted = false)))); diff --git a/testdata/diff/dependency/issue_373_policy_references_other_table/new.sql b/testdata/diff/dependency/issue_373_policy_references_other_table/new.sql new file mode 100644 index 00000000..3bd421e8 --- /dev/null +++ b/testdata/diff/dependency/issue_373_policy_references_other_table/new.sql @@ -0,0 +1,27 @@ +CREATE TABLE project_manager ( + id SERIAL, + project_id int NOT NULL, + manager_id int NOT NULL, + is_deleted boolean NOT NULL DEFAULT false +); + +CREATE TABLE manager ( + id SERIAL, + user_id uuid NOT NULL +); + +ALTER TABLE manager ENABLE ROW LEVEL SECURITY; + +CREATE POLICY employee_manager_select ON manager + FOR SELECT + TO PUBLIC + USING ( + id IN ( + SELECT pam.manager_id + FROM project_manager pam + WHERE pam.project_id IN ( + SELECT unnest(ARRAY[1, 2, 3]) + ) + AND pam.is_deleted = false + ) + ); \ No newline at end of file diff --git a/testdata/diff/dependency/issue_373_policy_references_other_table/old.sql b/testdata/diff/dependency/issue_373_policy_references_other_table/old.sql new file mode 100644 index 00000000..68cc3483 --- /dev/null +++ b/testdata/diff/dependency/issue_373_policy_references_other_table/old.sql @@ -0,0 +1 @@ +-- Empty schema (no tables) \ No newline at end of file diff --git a/testdata/diff/dependency/issue_373_policy_references_other_table/plan.json b/testdata/diff/dependency/issue_373_policy_references_other_table/plan.json new file mode 100644 index 00000000..10c9160b --- /dev/null +++ b/testdata/diff/dependency/issue_373_policy_references_other_table/plan.json @@ -0,0 +1,38 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.4", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "965b1131737c955e24c7f827c55bd78e4cb49a75adfd04229e0ba297376f5085" + }, + "groups": [ + { + "steps": [ + { + "sql": "CREATE TABLE IF NOT EXISTS manager (\n id SERIAL,\n user_id uuid NOT NULL\n);", + "type": "table", + "operation": "create", + "path": "public.manager" + }, + { + "sql": "ALTER TABLE manager ENABLE ROW LEVEL SECURITY;", + "type": "table.rls", + "operation": "alter", + "path": "public.manager" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS project_manager (\n id SERIAL,\n project_id integer NOT NULL,\n manager_id integer NOT NULL,\n is_deleted boolean DEFAULT false NOT NULL\n);", + "type": "table", + "operation": "create", + "path": "public.project_manager" + }, + { + "sql": "CREATE POLICY employee_manager_select ON manager FOR SELECT TO PUBLIC USING (id IN ( SELECT pam.manager_id FROM project_manager pam WHERE ((pam.project_id IN ( SELECT unnest(ARRAY[1, 2, 3]) AS unnest)) AND (pam.is_deleted = false))));", + "type": "table.policy", + "operation": "create", + "path": "public.manager.employee_manager_select" + } + ] + } + ] +} diff --git a/testdata/diff/dependency/issue_373_policy_references_other_table/plan.sql b/testdata/diff/dependency/issue_373_policy_references_other_table/plan.sql new file mode 100644 index 00000000..8603680e --- /dev/null +++ b/testdata/diff/dependency/issue_373_policy_references_other_table/plan.sql @@ -0,0 +1,15 @@ +CREATE TABLE IF NOT EXISTS manager ( + id SERIAL, + user_id uuid NOT NULL +); + +ALTER TABLE manager ENABLE ROW LEVEL SECURITY; + +CREATE TABLE IF NOT EXISTS project_manager ( + id SERIAL, + project_id integer NOT NULL, + manager_id integer NOT NULL, + is_deleted boolean DEFAULT false NOT NULL +); + +CREATE POLICY employee_manager_select ON manager FOR SELECT TO PUBLIC USING (id IN ( SELECT pam.manager_id FROM project_manager pam WHERE ((pam.project_id IN ( SELECT unnest(ARRAY[1, 2, 3]) AS unnest)) AND (pam.is_deleted = false)))); diff --git a/testdata/diff/dependency/issue_373_policy_references_other_table/plan.txt b/testdata/diff/dependency/issue_373_policy_references_other_table/plan.txt new file mode 100644 index 00000000..fa03ed88 --- /dev/null +++ b/testdata/diff/dependency/issue_373_policy_references_other_table/plan.txt @@ -0,0 +1,29 @@ +Plan: 2 to add. + +Summary by type: + tables: 2 to add + +Tables: + + manager + + employee_manager_select (policy) + ~ manager (rls) + + project_manager + +DDL to be executed: +-------------------------------------------------- + +CREATE TABLE IF NOT EXISTS manager ( + id SERIAL, + user_id uuid NOT NULL +); + +ALTER TABLE manager ENABLE ROW LEVEL SECURITY; + +CREATE TABLE IF NOT EXISTS project_manager ( + id SERIAL, + project_id integer NOT NULL, + manager_id integer NOT NULL, + is_deleted boolean DEFAULT false NOT NULL +); + +CREATE POLICY employee_manager_select ON manager FOR SELECT TO PUBLIC USING (id IN ( SELECT pam.manager_id FROM project_manager pam WHERE ((pam.project_id IN ( SELECT unnest(ARRAY[1, 2, 3]) AS unnest)) AND (pam.is_deleted = false))));