Skip to content

Commit 6dbc536

Browse files
committed
refactor(pg-delta): use invalidates for policy replacement
RED: after rebasing onto supabase#273, the design regression using a generic invalidating change failed in expand-replace-dependencies.test.ts with Expected: true / Received: false for DropRlsPolicy because the helper still checked AlterTableAlterColumnType directly. GREEN: expand-replace-dependencies.test.ts now passes, PG17 policy-dependencies.test.ts focused and full-file runs pass, and format-and-lint/check-types/knip pass. Signed-off-by: xiaominghao <xiaominghao@baidu.com>
1 parent 9163484 commit 6dbc536

2 files changed

Lines changed: 33 additions & 24 deletions

File tree

packages/pg-delta/src/core/expand-replace-dependencies.test.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import { diffSequences } from "./objects/sequence/sequence.diff.ts";
2020
import { Sequence } from "./objects/sequence/sequence.model.ts";
2121
import {
2222
AlterTableAlterColumnSetDefault,
23-
AlterTableAlterColumnType,
2423
AlterTableChangeOwner,
2524
AlterTableDropColumn,
2625
AlterTableDropConstraint,
@@ -49,6 +48,7 @@ function mockChange(overrides: {
4948
scope: "object",
5049
creates,
5150
drops,
51+
invalidates: [],
5252
requires: [],
5353
table: { schema: "public", name: "t" },
5454
serialize: () => [],
@@ -58,6 +58,20 @@ function mockChange(overrides: {
5858
} as unknown as Change;
5959
}
6060

61+
function mockInvalidatingChange(invalidates: string[]): Change {
62+
return {
63+
objectType: "table",
64+
operation: "alter",
65+
scope: "object",
66+
creates: [],
67+
drops: [],
68+
invalidates,
69+
requires: [],
70+
table: { schema: "public", name: "t" },
71+
serialize: () => "",
72+
} as unknown as Change;
73+
}
74+
6175
describe("expandReplaceDependencies", () => {
6276
test("returns changes unchanged when there are no replace roots", async () => {
6377
const catalog = await createEmptyCatalog(160004, "u");
@@ -826,7 +840,7 @@ describe("expandReplaceDependencies", () => {
826840
).toBe(true);
827841
});
828842

829-
test("promotes dependent RLS policy when a referenced column is rewritten", async () => {
843+
test("promotes dependent RLS policy when a referenced column is invalidated", async () => {
830844
const baseline = await createEmptyCatalog(170000, "postgres");
831845
const columnTemplate = {
832846
position: 1,
@@ -922,11 +936,9 @@ describe("expandReplaceDependencies", () => {
922936
withCheckExpression: branchPolicy.with_check_expression,
923937
});
924938
const changes: Change[] = [
925-
new AlterTableAlterColumnType({
926-
table: branchTable,
927-
column: branchRoleColumn,
928-
previousColumn: mainRoleColumn,
929-
}),
939+
mockInvalidatingChange([
940+
"column:public.solution_categories_with_policy.role",
941+
]),
930942
alterUsing,
931943
alterWithCheck,
932944
];

packages/pg-delta/src/core/expand-replace-dependencies.ts

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ import { DropProcedure } from "./objects/procedure/changes/procedure.drop.ts";
1313
import { CreateCommentOnRlsPolicy } from "./objects/rls-policy/changes/rls-policy.comment.ts";
1414
import { CreateRlsPolicy } from "./objects/rls-policy/changes/rls-policy.create.ts";
1515
import { DropRlsPolicy } from "./objects/rls-policy/changes/rls-policy.drop.ts";
16-
import {
17-
AlterTableAddConstraint,
18-
AlterTableAlterColumnType,
19-
} from "./objects/table/changes/table.alter.ts";
16+
import { AlterTableAddConstraint } from "./objects/table/changes/table.alter.ts";
2017
import { CreateCommentOnConstraint } from "./objects/table/changes/table.comment.ts";
2118
import { CreateTable } from "./objects/table/changes/table.create.ts";
2219
import { DropTable } from "./objects/table/changes/table.drop.ts";
@@ -127,7 +124,7 @@ export function expandReplaceDependencies({
127124
}
128125

129126
const promotedRlsPolicyIds = new Set<string>();
130-
const additions: Change[] = collectColumnRewritePolicyReplacements({
127+
const additions: Change[] = collectInvalidatedRlsPolicyReplacements({
131128
changes,
132129
mainCatalog,
133130
branchCatalog,
@@ -304,7 +301,7 @@ export function expandReplaceDependencies({
304301
};
305302
}
306303

307-
function collectColumnRewritePolicyReplacements({
304+
function collectInvalidatedRlsPolicyReplacements({
308305
changes,
309306
mainCatalog,
310307
branchCatalog,
@@ -319,22 +316,22 @@ function collectColumnRewritePolicyReplacements({
319316
droppedIds: Set<string>;
320317
promotedRlsPolicyIds: Set<string>;
321318
}): Change[] {
322-
const rewrittenColumnIds = new Set<string>();
319+
// In-place rewrites report stable ids through `invalidates`: the referenced
320+
// object keeps its identity, but dependents bound to the old definition must
321+
// be torn down first. RLS policy expressions are tracked in pg_depend, so use
322+
// those catalog edges to promote only policies that depend on an invalidated
323+
// id, without coupling this expansion pass to a concrete table-change class.
324+
const invalidatedIds = new Set<string>();
323325
for (const change of changes) {
324-
if (!(change instanceof AlterTableAlterColumnType)) continue;
325-
rewrittenColumnIds.add(
326-
stableId.column(
327-
change.table.schema,
328-
change.table.name,
329-
change.column.name,
330-
),
331-
);
326+
for (const invalidatedId of change.invalidates) {
327+
invalidatedIds.add(invalidatedId);
328+
}
332329
}
333-
if (rewrittenColumnIds.size === 0) return [];
330+
if (invalidatedIds.size === 0) return [];
334331

335332
const replacements: Change[] = [];
336333
for (const dep of mainCatalog.depends) {
337-
if (!rewrittenColumnIds.has(dep.referenced_stable_id)) continue;
334+
if (!invalidatedIds.has(dep.referenced_stable_id)) continue;
338335

339336
const targetId = normalizeDependentId(dep.dependent_stable_id);
340337
if (!targetId?.startsWith("rlsPolicy:")) continue;

0 commit comments

Comments
 (0)