fix(pg-delta): recreate policies for column rewrite dependencies#275
fix(pg-delta): recreate policies for column rewrite dependencies#275xmh1011 wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 6dbc536 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5626f1f to
0476a23
Compare
|
Follow-up after comparing this PR with the review discussion on #273. This PR still addresses a separate issue #263 case: an RLS policy whose expression depends on a column being rewritten needs to be promoted from One design note before this is finalized: #273 introduced the Planned cleanup after #273 merges:
I am intentionally not stacking #273's infrastructure onto this PR before #273 merges, to keep this diff reviewable and avoid duplicating the same base changes across two open PRs. |
0476a23 to
e5b75ee
Compare
|
@avallete Follow-up after #273 merged: I rebased this branch onto the current Pushed
RED after the rebase, before the refactor: the generic invalidates regression failed with Validation:
|
e5b75ee to
98c0d8c
Compare
Signed-off-by: xiaominghao <xiaominghao@baidu.com>
RED: PG17 focused integration generated ALTER TABLE ... ALTER COLUMN role TYPE before two ALTER POLICY statements, with no DROP POLICY / CREATE POLICY. The focused unit regression also showed expandReplaceDependencies did not add DropRlsPolicy for a policy whose pg_depend row referenced the rewritten column. Signed-off-by: xiaominghao <xiaominghao@baidu.com>
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>
98c0d8c to
6dbc536
Compare
Refs #263
This is the third incremental fix for issue #263, scoped to Case 1:
ALTER TABLE ... ALTER COLUMN ... TYPEfails when an existing RLS policy still depends on the rewritten column.What changed
For this case, pg-delta was producing the right table rewrite but the wrong policy handling:
diffRlsPoliciessaw the policy expression text change after the column type changed fromtextto an enum, so it emittedALTER POLICY ... USINGandALTER POLICY ... WITH CHECK.ALTER COLUMN TYPEwhile the old policy definition still depends on that column, so the policy must be fully dropped before the rewrite and recreated after it.BaseChange.invalidatesrather than adding concrete subtype checks to generic expansion/sorting code.This PR now builds on #273's
invalidatesprimitive:change.invalidates;mainCatalog.dependsto find survivingrlsPolicy:*objects whose policy expressions depend on those ids;DropRlsPolicy/CreateRlsPolicyfor those policies;ALTER POLICYchanges that are superseded by the replacement.The fix stays within existing pg_catalog/pg_depend metadata and does not parse policy SQL or couple
expandReplaceDependenciestoAlterTableAlterColumnType.RED evidence
expandReplaceDependenciesdid not addDropRlsPolicyfor a policy whosepg_dependrow referenced the rewritten column.ALTER TABLE ... ALTER COLUMN role TYPE ...followed byALTER POLICY ... USING/ALTER POLICY ... WITH CHECK, with noDROP POLICY/CREATE POLICYaround the column rewrite.0A000: cannot alter type of a column used in a policy definition.expand-replace-dependencies.test.tsfailed withExpected: true / Received: falseforDropRlsPolicybecause the helper still checkedAlterTableAlterColumnTypedirectly.Local validation
1 pass / 0 fail.expand-replace-dependencies.test.ts:9 pass / 0 fail.policy depending on a column type rewrite is dropped and recreated:1 pass / 0 fail.policy-dependencies.test.ts:9 pass / 0 fail(one connection timeout retried by the test runner; final result was green)../node_modules/.bin/bun run format-and-lint:fix: passed../node_modules/.bin/bun run check-types: passed../node_modules/.bin/bun run knip --fix: passed; knip only reported existing configuration hints.Local integration validation used
PGDELTA_SKIP_DUMMY_SECLABEL_BUILD=1to avoid rebuilding the local dummy security-label image; CI should keep security-label coverage enabled.