Skip to content

Commit 0e9d685

Browse files
tianzhouclaude
andauthored
fix: defer cross-table policy creation until all tables exist (#373) (#374)
* fix: defer all policies to after all tables are created (#373) Policies may reference other tables in USING/WITH CHECK expressions (e.g., subqueries), and those tables may not exist yet when the policy's parent table is created. Always defer policy creation to after all tables exist, rather than only deferring policies that reference new functions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: sort deferred policies by (schema, table, name) and update doc comment Address review feedback: - Sort policies by (schema, table, name) for deterministic ordering when policies from multiple tables are collected together - Update generateCreateTablesSQL doc comment to reflect that all policies are now deferred Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: only defer policies that reference other new tables (#373) Instead of deferring all policies (which changed dump output ordering), selectively defer only policies whose USING/WITH CHECK expressions reference another new table being created in the same batch. Policies that only reference their own table or existing tables remain inline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update comment to say "same migration" instead of "same batch" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use qualified table names in lookup and update doc comment - Store only fully-qualified (schema.table) keys in newTableLookup to avoid cross-schema name collisions - Update generateCreateTablesSQL doc comment to mention both table and function deferral conditions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 113704a commit 0e9d685

9 files changed

Lines changed: 182 additions & 6 deletions

File tree

internal/diff/diff.go

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,10 +1528,19 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto
15281528
key := fmt.Sprintf("%s.%s", tableDiff.Table.Schema, tableDiff.Table.Name)
15291529
existingTables[key] = true
15301530
}
1531+
// Build lookup of all new table names (qualified) for policy deferral (#373).
1532+
// Policies that reference other new tables must be deferred until all tables exist.
1533+
newTableLookup := make(map[string]struct{}, len(d.addedTables))
1534+
for _, table := range d.addedTables {
1535+
newTableLookup[fmt.Sprintf("%s.%s", strings.ToLower(table.Schema), strings.ToLower(table.Name))] = struct{}{}
1536+
}
15311537
var shouldDeferPolicy func(*ir.RLSPolicy) bool
1532-
if len(newFunctionLookup) > 0 {
1538+
if len(newFunctionLookup) > 0 || len(newTableLookup) > 0 {
15331539
shouldDeferPolicy = func(policy *ir.RLSPolicy) bool {
1534-
return policyReferencesNewFunction(policy, newFunctionLookup)
1540+
if policyReferencesNewFunction(policy, newFunctionLookup) {
1541+
return true
1542+
}
1543+
return policyReferencesOtherNewTable(policy, newTableLookup)
15351544
}
15361545
}
15371546

@@ -2028,6 +2037,38 @@ func policyReferencesNewFunction(policy *ir.RLSPolicy, newFunctions map[string]s
20282037
return false
20292038
}
20302039

2040+
// policyReferencesOtherNewTable determines if a policy's USING or WITH CHECK expressions
2041+
// reference any newly added table other than the policy's own table (#373).
2042+
// newTables keys are fully qualified (schema.table) to avoid cross-schema collisions.
2043+
func policyReferencesOtherNewTable(policy *ir.RLSPolicy, newTables map[string]struct{}) bool {
2044+
if len(newTables) == 0 || policy == nil {
2045+
return false
2046+
}
2047+
2048+
ownQualified := fmt.Sprintf("%s.%s", strings.ToLower(policy.Schema), strings.ToLower(policy.Table))
2049+
2050+
for _, expr := range []string{policy.Using, policy.WithCheck} {
2051+
if expr == "" {
2052+
continue
2053+
}
2054+
exprLower := strings.ToLower(expr)
2055+
for qualifiedName := range newTables {
2056+
// Skip the policy's own table
2057+
if qualifiedName == ownQualified {
2058+
continue
2059+
}
2060+
// Extract the unqualified table name for substring matching.
2061+
// Policy expressions may use unqualified or qualified references.
2062+
parts := strings.SplitN(qualifiedName, ".", 2)
2063+
tableName := parts[len(parts)-1]
2064+
if strings.Contains(exprLower, tableName) {
2065+
return true
2066+
}
2067+
}
2068+
}
2069+
return false
2070+
}
2071+
20312072
// tableUsesDeferredDomain determines if a table uses any deferred domain types in its columns.
20322073
func tableUsesDeferredDomain(table *ir.Table, deferredDomains map[string]struct{}) bool {
20332074
if len(deferredDomains) == 0 || table == nil {

internal/diff/policy.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,18 @@ import (
1010

1111
// generateCreatePoliciesSQL generates CREATE POLICY statements
1212
func generateCreatePoliciesSQL(policies []*ir.RLSPolicy, targetSchema string, collector *diffCollector) {
13-
// Sort policies by name for consistent ordering
13+
// Sort policies by (schema, table, name) for deterministic ordering across tables.
14+
// Policy names are only unique per table, so sorting by name alone is insufficient
15+
// when policies from multiple tables are collected together (#373).
1416
sortedPolicies := make([]*ir.RLSPolicy, len(policies))
1517
copy(sortedPolicies, policies)
1618
sort.Slice(sortedPolicies, func(i, j int) bool {
19+
if sortedPolicies[i].Schema != sortedPolicies[j].Schema {
20+
return sortedPolicies[i].Schema < sortedPolicies[j].Schema
21+
}
22+
if sortedPolicies[i].Table != sortedPolicies[j].Table {
23+
return sortedPolicies[i].Table < sortedPolicies[j].Table
24+
}
1725
return sortedPolicies[i].Name < sortedPolicies[j].Name
1826
})
1927

internal/diff/table.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,9 @@ type deferredConstraint struct {
384384
}
385385

386386
// generateCreateTablesSQL generates CREATE TABLE statements with co-located indexes, policies, and RLS.
387-
// Policies that reference newly added helper functions are collected for deferred creation after
388-
// dependent functions/procedures have been created, while all other policies are emitted inline.
387+
// Policies that reference other new tables in the same migration or newly added functions
388+
// (via USING/WITH CHECK expressions) are deferred for creation after all tables and functions
389+
// exist (#373). All other policies are emitted inline.
389390
// It returns deferred policies and foreign key constraints that should be applied after dependent objects exist.
390391
// Tables are assumed to be pre-sorted in topological order for dependency-aware creation.
391392
func generateCreateTablesSQL(
@@ -474,7 +475,8 @@ func generateCreateTablesSQL(
474475
generateRLSChangesSQL(rlsChanges, targetSchema, collector)
475476
}
476477

477-
// Collect policies that can run immediately; defer only those that depend on new helper functions
478+
// Collect policies: defer those that reference other new tables or new functions (#373),
479+
// emit the rest inline with their parent table.
478480
if len(table.Policies) > 0 {
479481
var inlinePolicies []*ir.RLSPolicy
480482
policyNames := sortedKeys(table.Policies)
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
CREATE TABLE IF NOT EXISTS manager (
2+
id SERIAL,
3+
user_id uuid NOT NULL
4+
);
5+
6+
ALTER TABLE manager ENABLE ROW LEVEL SECURITY;
7+
8+
CREATE TABLE IF NOT EXISTS project_manager (
9+
id SERIAL,
10+
project_id integer NOT NULL,
11+
manager_id integer NOT NULL,
12+
is_deleted boolean DEFAULT false NOT NULL
13+
);
14+
15+
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))));
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
CREATE TABLE project_manager (
2+
id SERIAL,
3+
project_id int NOT NULL,
4+
manager_id int NOT NULL,
5+
is_deleted boolean NOT NULL DEFAULT false
6+
);
7+
8+
CREATE TABLE manager (
9+
id SERIAL,
10+
user_id uuid NOT NULL
11+
);
12+
13+
ALTER TABLE manager ENABLE ROW LEVEL SECURITY;
14+
15+
CREATE POLICY employee_manager_select ON manager
16+
FOR SELECT
17+
TO PUBLIC
18+
USING (
19+
id IN (
20+
SELECT pam.manager_id
21+
FROM project_manager pam
22+
WHERE pam.project_id IN (
23+
SELECT unnest(ARRAY[1, 2, 3])
24+
)
25+
AND pam.is_deleted = false
26+
)
27+
);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-- Empty schema (no tables)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"version": "1.0.0",
3+
"pgschema_version": "1.7.4",
4+
"created_at": "1970-01-01T00:00:00Z",
5+
"source_fingerprint": {
6+
"hash": "965b1131737c955e24c7f827c55bd78e4cb49a75adfd04229e0ba297376f5085"
7+
},
8+
"groups": [
9+
{
10+
"steps": [
11+
{
12+
"sql": "CREATE TABLE IF NOT EXISTS manager (\n id SERIAL,\n user_id uuid NOT NULL\n);",
13+
"type": "table",
14+
"operation": "create",
15+
"path": "public.manager"
16+
},
17+
{
18+
"sql": "ALTER TABLE manager ENABLE ROW LEVEL SECURITY;",
19+
"type": "table.rls",
20+
"operation": "alter",
21+
"path": "public.manager"
22+
},
23+
{
24+
"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);",
25+
"type": "table",
26+
"operation": "create",
27+
"path": "public.project_manager"
28+
},
29+
{
30+
"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))));",
31+
"type": "table.policy",
32+
"operation": "create",
33+
"path": "public.manager.employee_manager_select"
34+
}
35+
]
36+
}
37+
]
38+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
CREATE TABLE IF NOT EXISTS manager (
2+
id SERIAL,
3+
user_id uuid NOT NULL
4+
);
5+
6+
ALTER TABLE manager ENABLE ROW LEVEL SECURITY;
7+
8+
CREATE TABLE IF NOT EXISTS project_manager (
9+
id SERIAL,
10+
project_id integer NOT NULL,
11+
manager_id integer NOT NULL,
12+
is_deleted boolean DEFAULT false NOT NULL
13+
);
14+
15+
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))));
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
Plan: 2 to add.
2+
3+
Summary by type:
4+
tables: 2 to add
5+
6+
Tables:
7+
+ manager
8+
+ employee_manager_select (policy)
9+
~ manager (rls)
10+
+ project_manager
11+
12+
DDL to be executed:
13+
--------------------------------------------------
14+
15+
CREATE TABLE IF NOT EXISTS manager (
16+
id SERIAL,
17+
user_id uuid NOT NULL
18+
);
19+
20+
ALTER TABLE manager ENABLE ROW LEVEL SECURITY;
21+
22+
CREATE TABLE IF NOT EXISTS project_manager (
23+
id SERIAL,
24+
project_id integer NOT NULL,
25+
manager_id integer NOT NULL,
26+
is_deleted boolean DEFAULT false NOT NULL
27+
);
28+
29+
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))));

0 commit comments

Comments
 (0)