hi. main difference with v2: ALTER COLUMN SET DATA TYPE will fail for policy containing sublink.
CREATE TABLE t(a int); CREATE TABLE t1(a int); CREATE POLICY p2 ON t1 AS PERMISSIVE USING ((SELECT t.a IS NOT NULL FROM t)); ALTER TABLE t ALTER COLUMN a SET DATA TYPE INT8; ERROR: cannot alter type of a column used by a policy definition when the policy contains subquery DETAIL: policy p2 on table t depends on column "a" If the above example does not fail, we would also need to acquire an AccessExclusiveLock on table "t1" which does not seem ideal. Even if this is ok, We use the recreated policy definition, CreatePolicyStmt does not carry all referenced OID information. As a result, re-parsing and analyzing the policy qual or WITH CHECK qual would inevitably lead to repeated RangeVar name lookup issue The attached patch will let ALTER COLUMN SET DATA TYPE work fine for policy dependent objects, if the policy qual or with check qual does not have Sublink, otherwise it will error out. v3-0001, v3-0002 is used in [1] too. [1]: https://postgr.es/m/CACJufxFuEOB-i2z2qhyCG=dGwDf7g6Fs_o8cz=bui76uuuf...@mail.gmail.com -- jian https://www.enterprisedb.com/
From 56992a0fc4df5c4e3f55601afc4df9853235e9ee Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Wed, 24 Dec 2025 10:47:18 +0800 Subject: [PATCH v3 3/3] ALTER TABLE command to change policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change prevents ALTER TABLE … ALTER COLUMN SET DATA TYPE from failing when the column has dependent policy objects, whereas previously it would raise an error. The approach closely mirrors how statistics are rebuilt: identify all policy objects that need to be rebuilt, construct their CREATE definitions, and then recreate those objects. However, we must fail when the qual or WITH CHECK clause contains a subquery. Otherwise, we would need to acquire AccessExclusiveLock on the referenced tables too, which seems not good, for example, changing dtaa type in table t would also lock table t1 in AccessExclusiveLock CREATE TABLE t(a int); CREATE TABLE t1(a int); CREATE POLICY p2 ON t1 AS PERMISSIVE USING ((SELECT t.a IS NOT NULL FROM t)); ALTER TABLE t ALTER COLUMN a SET DATA TYPE INT8; ERROR: cannot alter type of a column used by a policy definition when the policy contains subquery DETAIL: policy p2 on table t depends on column "a" Another reason to disallow subqueries in qual or WITH CHECK clauses is that, internally, we reconstruct policy definitions. Recreating such policies can trigger repeated RangeVar name lookups, leading to correctness and robustness issues. discussion: https://postgr.es/m/cacjufxe42vysvedemaobgmgylztcguawh_h-c9dcsldtd5j...@mail.gmail.com --- src/backend/commands/policy.c | 68 +++++ src/backend/commands/tablecmds.c | 246 ++++++++++++++++-- src/backend/parser/gram.y | 1 + src/backend/utils/adt/ruleutils.c | 163 ++++++++++++ src/include/commands/policy.h | 2 + src/include/nodes/parsenodes.h | 2 + src/include/utils/ruleutils.h | 1 + .../test_ddl_deparse/test_ddl_deparse.c | 3 + src/test/regress/expected/rowsecurity.out | 69 +++++ src/test/regress/sql/rowsecurity.sql | 54 ++++ 10 files changed, 592 insertions(+), 17 deletions(-) diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index a595cd937d5..b4a16d0ff2d 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -26,6 +26,7 @@ #include "catalog/pg_authid.h" #include "catalog/pg_policy.h" #include "catalog/pg_type.h" +#include "commands/comment.h" #include "commands/policy.h" #include "miscadmin.h" #include "nodes/pg_list.h" @@ -742,6 +743,11 @@ CreatePolicy(CreatePolicyStmt *stmt, const char *queryString) relation_close(target_table, NoLock); table_close(pg_policy_rel, RowExclusiveLock); + /* Add any requested comment */ + if (stmt->polcomment != NULL) + CreateComments(policy_id, PolicyRelationId, 0, + stmt->polcomment); + return myself; } @@ -1264,3 +1270,65 @@ relation_has_policies(Relation rel) return ret; } + +/* + * PolicyGetRelation: given a policy object's OID, get the OID of + * the relation it is defined on. + */ +Oid +PolicyGetRelation(Oid policyId) +{ + Relation pg_policy_rel; + ScanKeyData skey[1]; + SysScanDesc sscan; + HeapTuple tuple; + Oid relid; + + /* Fetch the policy's table OID the hard way */ + pg_policy_rel = table_open(PolicyRelationId, AccessShareLock); + ScanKeyInit(&skey[0], + Anum_pg_policy_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(policyId)); + sscan = systable_beginscan(pg_policy_rel, PolicyOidIndexId, true, + NULL, 1, skey); + tuple = systable_getnext(sscan); + + if (HeapTupleIsValid(tuple)) + relid = ((Form_pg_policy) GETSTRUCT(tuple))->polrelid; + else + { + relid = InvalidOid; /* shouldn't happen */ + elog(ERROR, "could not find tuple for policy %u", policyId); + } + systable_endscan(sscan); + + table_close(pg_policy_rel, AccessShareLock); + + return relid; +} + +/* + * get_policy_applied_command + * + * Convert pg_policy.polcmd char representation to their full command strings. + */ +char * +get_policy_applied_command(char polcmd) +{ + if (polcmd == '*') + return pstrdup("all"); + else if (polcmd == ACL_SELECT_CHR) + return pstrdup("select"); + else if (polcmd == ACL_INSERT_CHR) + return pstrdup("insert"); + else if (polcmd == ACL_UPDATE_CHR) + return pstrdup("update"); + else if (polcmd == ACL_DELETE_CHR) + return pstrdup("delete"); + else + { + elog(ERROR, "unrecognized policy command"); + return NULL; + } +} diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6b1a00ed477..cc1cfe9bf1d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -60,6 +60,7 @@ #include "commands/comment.h" #include "commands/defrem.h" #include "commands/event_trigger.h" +#include "commands/policy.h" #include "commands/sequence.h" #include "commands/tablecmds.h" #include "commands/tablespace.h" @@ -208,6 +209,8 @@ typedef struct AlteredTableInfo char *clusterOnIndex; /* index to use for CLUSTER */ List *changedStatisticsOids; /* OIDs of statistics to rebuild */ List *changedStatisticsDefs; /* string definitions of same */ + List *changedPolicyOids; /* OIDs of policy to rebuild */ + List *changedPolicyDefs; /* string definitions of same */ } AlteredTableInfo; /* Struct describing one new constraint to check in Phase 3 scan */ @@ -546,6 +549,8 @@ static ObjectAddress ATExecAddIndex(AlteredTableInfo *tab, Relation rel, IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode); static ObjectAddress ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, CreateStatsStmt *stmt, bool is_rebuild, LOCKMODE lockmode); +static ObjectAddress ATExecAddPolicies(AlteredTableInfo *tab, Relation rel, + CreatePolicyStmt *stmt, bool is_rebuild, LOCKMODE lockmode); static ObjectAddress ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Constraint *newConstraint, bool recurse, bool is_readd, @@ -648,9 +653,12 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, Relation rel, AttrNumber attnum, const char *colName); +static void CheckDepentPolicies(AlteredTableInfo *tab, AlterTableType subtype, + Relation rel, AttrNumber attnum, const char *colName); static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); +static void RememberPolicyForRebuilding(Oid policyId, AlteredTableInfo *tab); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode); static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, @@ -5464,6 +5472,10 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, address = ATExecAddStatistics(tab, rel, (CreateStatsStmt *) cmd->def, true, lockmode); break; + case AT_ReAddPolicies: /* ADD POLICIES */ + address = ATExecAddPolicies(tab, rel, castNode(CreatePolicyStmt, cmd->def), + true, lockmode); + break; case AT_AddConstraint: /* ADD CONSTRAINT */ /* Transform the command only during initial examination */ if (cur_pass == AT_PASS_ADD_CONSTR) @@ -6751,6 +6763,8 @@ alter_table_type_to_string(AlterTableType cmdtype) return "ALTER COLUMN ... DROP IDENTITY"; case AT_ReAddStatistics: return NULL; /* not real grammar */ + case AT_ReAddPolicies: + return NULL; /* not real grammar */ } return NULL; @@ -9723,6 +9737,29 @@ ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, return address; } +/* + * ALTER TABLE ADD POLICIES + * + * This is no such command in the grammar, but we use this internally to add + * AT_ReAddPolicies subcommands to rebuild policy after a table + * column type change. + */ +static ObjectAddress +ATExecAddPolicies(AlteredTableInfo *tab, Relation rel, + CreatePolicyStmt *stmt, bool is_rebuild, LOCKMODE lockmode) +{ + ObjectAddress address; + + Assert(IsA(stmt, CreatePolicyStmt)); + + /* The CreatePolicyStmt has already been through transformPolicyStmt */ + Assert(stmt->transformed); + + address = CreatePolicy(stmt, NULL); + + return address; +} + /* * ALTER TABLE ADD CONSTRAINT USING INDEX * @@ -14868,6 +14905,13 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ RememberAllDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName); + /* + * Check that all policies does not contain subquery before adding its + * definition to tab->changedPolicyDefs, as + * RememberAllDependentForRebuilding only collects the policy OID. + */ + CheckDepentPolicies(tab, AT_AlterColumnType, rel, attnum, colName); + /* * Now scan for dependencies of this column on other things. The only * things we should find are the dependency on the column datatype and @@ -15062,6 +15106,109 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, return address; } +/* + * Subroutine for ATExecAlterColumnType. + * + * ALTER COLUMN SET DATA TYPE cannot cope with policies that contain subqueries + * in the USING or WITH CHECK qual; otherwise, repeated name lookup issues may + * occur. + */ +static void +CheckDepentPolicies(AlteredTableInfo *tab, AlterTableType subtype, + Relation rel, AttrNumber attnum, const char *colName) +{ + Relation pg_policy; + ScanKeyData skey[1]; + SysScanDesc sscan; + HeapTuple tuple; + Form_pg_policy form_policy; + Node *qual; + Node *with_check_qual; + bool hassublinks = false; + + pg_policy = table_open(PolicyRelationId, AccessShareLock); + + foreach_oid(poloid, tab->changedPolicyOids) + { + Datum datum; + bool isnull; + char *str_value; + + /* Fetch the policy's table OID the hard way */ + ScanKeyInit(&skey[0], + Anum_pg_policy_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(poloid)); + sscan = systable_beginscan(pg_policy, PolicyOidIndexId, true, + NULL, 1, skey); + tuple = systable_getnext(sscan); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "could not find tuple for pg_policy %u", poloid); + + form_policy = (Form_pg_policy) GETSTRUCT(tuple); + + /* Get policy qual */ + datum = heap_getattr(tuple, Anum_pg_policy_polqual, + RelationGetDescr(pg_policy), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + qual = stringToNode(str_value); + + if (checkExprHasSubLink(qual)) + hassublinks = true; + + pfree(str_value); + } + + if (hassublinks) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type of a column used by a policy definition when the policy contains subquery"), + errdetail("policy %s on table %s depends on column \"%s\"", + NameStr(form_policy->polname), + RelationGetRelationName(rel), + colName)); + + /* Get WITH CHECK qual */ + datum = heap_getattr(tuple, Anum_pg_policy_polwithcheck, + RelationGetDescr(pg_policy), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + with_check_qual = stringToNode(str_value); + + if (checkExprHasSubLink(with_check_qual)) + hassublinks = true; + + pfree(str_value); + } + + if (hassublinks) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type of a column used by a policy definition when the policy contains subquery"), + errdetail("policy %s on table %s depends on column \"%s\"", + NameStr(form_policy->polname), + RelationGetRelationName(rel), + colName)); + + Assert(form_policy->polrelid == RelationGetRelid(rel)); + + systable_endscan(sscan); + } + + table_close(pg_policy, AccessShareLock); + + foreach_oid(poloid, tab->changedPolicyOids) + { + /* OK, capture the policies's existing definition string */ + char *defstring = pg_get_policy_def_command(poloid); + + tab->changedPolicyDefs = lappend(tab->changedPolicyDefs, defstring); + } +} + /* * Subroutine for ATExecAlterColumnType and ATExecSetExpression: Find everything * that depends on the column (constraints, indexes, etc), and record enough @@ -15196,22 +15343,8 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, break; case PolicyRelationId: - - /* - * A policy can depend on a column because the column is - * specified in the policy's USING or WITH CHECK qual - * expressions. It might be possible to rewrite and recheck - * the policy expression, but punt for now. It's certainly - * easy enough to remove and recreate the policy; still, FIXME - * someday. - */ if (subtype == AT_AlterColumnType) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter type of a column used in a policy definition"), - errdetail("%s depends on column \"%s\"", - getObjectDescription(&foundObject, false), - colName))); + RememberPolicyForRebuilding(foundObject.objectId, tab); break; case AttrDefaultRelationId: @@ -15453,6 +15586,27 @@ RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab) } } +/* + * Subroutine for ATExecAlterColumnType: remember that a policy object needs to + * be rebuilt (which we might already know). + */ +static void +RememberPolicyForRebuilding(Oid policyId, AlteredTableInfo *tab) +{ + /* + * This de-duplication check is critical for two independent reasons: we + * mustn't try to recreate the same policy twice, and if a policy depends + * on more than one column whose type is to be altered, we must capture + * its definition string before applying any of the column type changes. + * ruleutils.c will get confused if we ask again later. + * + * changedPolicyDefs will be collected later on CheckDepentPolicies. + */ + if (!list_member_oid(tab->changedPolicyOids, policyId)) + tab->changedPolicyOids = lappend_oid(tab->changedPolicyOids, + policyId); +} + /* * Cleanup after we've finished all the ALTER TYPE or SET EXPRESSION * operations for a particular relation. We have to drop and recreate all the @@ -15597,6 +15751,35 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) add_exact_object_address(&obj, objects); } + /* add dependencies for new policies */ + forboth(oid_item, tab->changedPolicyOids, + def_item, tab->changedPolicyDefs) + { + Oid oldId = lfirst_oid(oid_item); + Oid relid; + + relid = PolicyGetRelation(oldId); + + /* + * As above, make sure we have lock on the relations if it's not the + * same table. However, we take AccessExclusiveLock here, aligning + * with the lock level used in CreatePolicy and RemovePolicyById. + * + * CAUTION: this should be done after all cases that grab + * AccessExclusiveLock, else we risk causing deadlock due to needing + * to promote our table lock. + */ + if (relid != tab->relid) + LockRelationOid(relid, ShareUpdateExclusiveLock); + + ATPostAlterTypeParse(oldId, tab->relid, InvalidOid, + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); + + ObjectAddressSet(obj, PolicyRelationId, oldId); + add_exact_object_address(&obj, objects); + } + /* * Queue up command to restore replica identity index marking */ @@ -15645,8 +15828,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) } /* - * Parse the previously-saved definition string for a constraint, index or - * statistics object against the newly-established column data type(s), and + * Parse the previously-saved definition string for a constraint, index, + * statistics or policy object against the newly-established column data type(s), and * queue up the resulting command parsetrees for execution. * * This might fail if, for example, you have a WHERE clause that uses an @@ -15698,6 +15881,21 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, transformStatsStmt(oldRelId, (CreateStatsStmt *) stmt, cmd)); + else if (IsA(stmt, CreatePolicyStmt)) + { + RangeTblEntry *rte; + CreatePolicyStmt *polstmt = castNode(CreatePolicyStmt, stmt); + + rte = makeNode(RangeTblEntry); + rte->rtekind = RTE_RELATION; + rte->relid = oldRelId; + rte->rellockmode = AccessExclusiveLock; + polstmt->rte = rte; + + querytree_list = lappend(querytree_list, + transformPolicyStmt(polstmt, + cmd)); + } else querytree_list = lappend(querytree_list, stmt); } @@ -15848,6 +16046,20 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, tab->subcmds[AT_PASS_MISC] = lappend(tab->subcmds[AT_PASS_MISC], newcmd); } + else if (IsA(stm, CreatePolicyStmt)) + { + CreatePolicyStmt *stmt = (CreatePolicyStmt *) stm; + AlterTableCmd *newcmd; + + /* keep the policy's object's comment */ + stmt->polcomment = GetComment(oldId, PolicyRelationId, 0); + + newcmd = makeNode(AlterTableCmd); + newcmd->subtype = AT_ReAddPolicies; + newcmd->def = (Node *) stmt; + tab->subcmds[AT_PASS_MISC] = + lappend(tab->subcmds[AT_PASS_MISC], newcmd); + } else elog(ERROR, "unexpected statement type: %d", (int) nodeTag(stm)); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 97900568464..0ec86872f16 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -6030,6 +6030,7 @@ CreatePolicyStmt: n->qual = $9; n->with_check = $10; n->transformed = false; + n->polcomment = NULL; $$ = (Node *) n; } ; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 9f85eb86da1..a30ccf40407 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -33,11 +33,13 @@ #include "catalog/pg_opclass.h" #include "catalog/pg_operator.h" #include "catalog/pg_partitioned_table.h" +#include "catalog/pg_policy.h" #include "catalog/pg_proc.h" #include "catalog/pg_statistic_ext.h" #include "catalog/pg_trigger.h" #include "catalog/pg_type.h" #include "commands/defrem.h" +#include "commands/policy.h" #include "commands/tablespace.h" #include "common/keywords.h" #include "executor/spi.h" @@ -367,6 +369,7 @@ static char *pg_get_partkeydef_worker(Oid relid, int prettyFlags, bool attrsOnly, bool missing_ok); static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, int prettyFlags, bool missing_ok); +static char *pg_get_policydef_worker(Oid policyId, int prettyFlags, bool missing_ok); static text *pg_get_expr_worker(text *expr, Oid relid, int prettyFlags); static int print_function_arguments(StringInfo buf, HeapTuple proctup, bool print_table_args, bool print_defaults); @@ -2610,6 +2613,166 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, return buf.data; } +/* + * Internal version that returns a full CREATE POLICY command + */ +char * +pg_get_policy_def_command(Oid policyId) +{ + int prettyFlags; + + prettyFlags = PRETTYFLAG_INDENT; + + return pg_get_policydef_worker(policyId, prettyFlags, false); +} + +static char * +pg_get_policydef_worker(Oid policyId, int prettyFlags, bool missing_ok) +{ + HeapTuple tup; + Form_pg_policy policy_form; + StringInfoData buf; + SysScanDesc scandesc; + ScanKeyData scankey[1]; + Snapshot snapshot = RegisterSnapshot(GetTransactionSnapshot()); + Relation relation = table_open(PolicyRelationId, AccessShareLock); + ArrayType *policy_roles; + Datum roles_datum; + Datum datum; + bool isnull; + Oid *roles; + int num_roles; + List *context = NIL; + char *str_value; + char *exprsrc; + char *rolString; + char *policy_command; + Node *expr; + + ScanKeyInit(&scankey[0], + Anum_pg_policy_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(policyId)); + + scandesc = systable_beginscan(relation, + PolicyOidIndexId, + true, + snapshot, + 1, + scankey); + tup = systable_getnext(scandesc); + + UnregisterSnapshot(snapshot); + + if (!HeapTupleIsValid(tup)) + { + if (missing_ok) + { + systable_endscan(scandesc); + table_close(relation, AccessShareLock); + return NULL; + } + elog(ERROR, "could not find tuple for policy %u", policyId); + } + + policy_form = (Form_pg_policy) GETSTRUCT(tup); + context = deparse_context_for(get_relation_name(policy_form->polrelid), + policy_form->polrelid); + + initStringInfo(&buf); + if (OidIsValid(policy_form->oid)) + appendStringInfo(&buf, "CREATE POLICY %s ON %s ", + quote_identifier(NameStr(policy_form->polname)), + generate_qualified_relation_name(policy_form->polrelid)); + else + elog(ERROR, "invalid policy: %u", policyId); + + /* Get policy type, permissive or restrictive */ + if (policy_form->polpermissive) + appendStringInfoString(&buf, "AS PERMISSIVE "); + else + appendStringInfoString(&buf, "AS RESTRICTIVE "); + + appendStringInfoString(&buf, "FOR "); + + /* Get policy applied command type */ + policy_command = get_policy_applied_command(policy_form->polcmd); + if (strcmp(policy_command, "all") == 0) + appendStringInfoString(&buf, "ALL "); + else if (strcmp(policy_command, "select") == 0) + appendStringInfoString(&buf, "SELECT "); + else if (strcmp(policy_command, "insert") == 0) + appendStringInfoString(&buf, "INSERT "); + else if (strcmp(policy_command, "update") == 0) + appendStringInfoString(&buf, "UPDATE "); + else if (strcmp(policy_command, "delete") == 0) + appendStringInfoString(&buf, "DELETE "); + else + elog(ERROR, "invalid command type %c", policy_form->polcmd); + + appendStringInfoString(&buf, "TO "); + + /* Get the current set of roles */ + datum = heap_getattr(tup, + Anum_pg_policy_polroles, + RelationGetDescr(relation), + &isnull); + Assert(!isnull); + + policy_roles = DatumGetArrayTypePCopy(datum); + roles = (Oid *) ARR_DATA_PTR(policy_roles); + num_roles = ARR_DIMS(policy_roles)[0]; + + for (int i = 0; i < num_roles; i++) + { + if (i > 0) + appendStringInfoString(&buf, ", "); + + if (OidIsValid(roles[i])) + { + datum = ObjectIdGetDatum(roles[i]); + roles_datum = DirectFunctionCall1(pg_get_userbyid, datum); + rolString = DatumGetCString(DirectFunctionCall1(nameout, roles_datum)); + appendStringInfo(&buf, "%s", rolString); + } + else + appendStringInfoString(&buf, "PUBLIC"); + } + + /* Get policy qual */ + datum = heap_getattr(tup, Anum_pg_policy_polqual, + RelationGetDescr(relation), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + expr = stringToNode(str_value); + exprsrc = deparse_expression_pretty(expr, context, false, false, + prettyFlags, 0); + appendStringInfo(&buf, " USING (%s) ", exprsrc); + pfree(str_value); + } + + /* Get WITH CHECK qual */ + datum = heap_getattr(tup, Anum_pg_policy_polwithcheck, + RelationGetDescr(relation), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + expr = stringToNode(str_value); + pfree(str_value); + + exprsrc = deparse_expression_pretty(expr, context, false, false, + prettyFlags, 0); + appendStringInfo(&buf, "WITH CHECK (%s)", exprsrc); + } + + /* Cleanup */ + systable_endscan(scandesc); + table_close(relation, AccessShareLock); + + return buf.data; +} + /* * Convert an int16[] Datum into a comma-separated list of column names diff --git a/src/include/commands/policy.h b/src/include/commands/policy.h index dab4030c38d..a9f6d38a0d3 100644 --- a/src/include/commands/policy.h +++ b/src/include/commands/policy.h @@ -34,5 +34,7 @@ extern Oid get_relation_policy_oid(Oid relid, const char *policy_name, extern ObjectAddress rename_policy(RenameStmt *stmt); extern bool relation_has_policies(Relation rel); +extern Oid PolicyGetRelation(Oid policyId); +extern char *get_policy_applied_command(char polcmd); #endif /* POLICY_H */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index c217ec73be9..06d9d639edf 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2508,6 +2508,7 @@ typedef enum AlterTableType AT_SetIdentity, /* SET identity column options */ AT_DropIdentity, /* DROP IDENTITY */ AT_ReAddStatistics, /* internal to commands/tablecmds.c */ + AT_ReAddPolicies, /* internal to commands/tablecmds.c */ } AlterTableType; typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ @@ -3105,6 +3106,7 @@ typedef struct CreatePolicyStmt RangeTblEntry *rte; char *cmd_name; /* the command name the policy applies to */ + char *polcomment; /* comment to apply to policies, or NULL */ bool permissive; /* restrictive or permissive policy */ List *roles; /* the roles associated with the policy */ Node *qual; /* the policy's condition */ diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h index 7ba7d887914..b21ec468bde 100644 --- a/src/include/utils/ruleutils.h +++ b/src/include/utils/ruleutils.h @@ -34,6 +34,7 @@ extern char *pg_get_partkeydef_columns(Oid relid, bool pretty); extern char *pg_get_partconstrdef_string(Oid partitionId, char *aliasname); extern char *pg_get_constraintdef_command(Oid constraintId); +extern char *pg_get_policy_def_command(Oid policyId); extern char *deparse_expression(Node *expr, List *dpcontext, bool forceprefix, bool showimplicit); extern List *deparse_context_for(const char *aliasname, Oid relid); diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c index 17d72e412ff..9726fc764ee 100644 --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c @@ -314,6 +314,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS) case AT_ReAddStatistics: strtype = "(re) ADD STATS"; break; + case AT_ReAddPolicies: + strtype = "(re) ADD POLICIES"; + break; } if (subcmd->recurse) diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index c958ef4d70a..c748f118bda 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -26,6 +26,27 @@ GRANT regress_rls_group2 TO regress_rls_carol; CREATE SCHEMA regress_rls_schema; GRANT ALL ON SCHEMA regress_rls_schema to public; SET search_path = regress_rls_schema; +--check alter column set data type will recreate security policy +CREATE TABLE rls_tbl (a int, b int, c int); +CREATE POLICY p1 ON rls_tbl + FOR UPDATE + TO regress_rls_alice, regress_rls_bob, regress_rls_carol + USING (a < 20) WITH CHECK (c <> 0 and a is not null); +CREATE POLICY p2 ON rls_tbl AS RESTRICTIVE + FOR ALL + TO PUBLIC + USING (a < 40) WITH CHECK (c > 0 and a is not null); +ALTER TABLE rls_tbl ALTER COLUMN a SET DATA TYPE INT8, ALTER COLUMN c SET DATA TYPE INT8; +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'rls_tbl' +ORDER BY policyname; + schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check +--------------------+-----------+------------+-------------+-------------------------------------------------------+--------+----------+-------------------------------- + regress_rls_schema | rls_tbl | p1 | PERMISSIVE | {regress_rls_alice,regress_rls_bob,regress_rls_carol} | UPDATE | (a < 20) | ((c <> 0) AND (a IS NOT NULL)) + regress_rls_schema | rls_tbl | p2 | RESTRICTIVE | {public} | ALL | (a < 40) | ((c > 0) AND (a IS NOT NULL)) +(2 rows) + +DROP TABLE rls_tbl; -- setup of malicious function CREATE OR REPLACE FUNCTION f_leak(text) RETURNS bool COST 0.0000001 LANGUAGE plpgsql @@ -2637,6 +2658,54 @@ SELECT * FROM document; 14 | 11 | 1 | regress_rls_bob | new novel | (16 rows) +CREATE TABLE document_c(LIKE document INCLUDING ALL); +CREATE POLICY p5 ON document_c AS RESTRICTIVE FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol +USING (CID IS NOT NULL AND + (WITH CTE AS (SELECT uaccount IS NOT NULL FROM uaccount) SELECT true FROM CTE)); +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE INT8; --error +ERROR: cannot alter type of a column used by a policy definition when the policy contains subquery +DETAIL: policy p5 on table document_c depends on column "cid" +ALTER TABLE uaccount ALTER COLUMN seclv SET DATA TYPE INT8; --error +ERROR: cannot alter type of a column used by a policy definition when the policy contains subquery +DETAIL: policy pp3 on table uaccount depends on column "seclv" +DROP POLICY p5 ON document_c CASCADE; +CREATE POLICY p5 ON document_c AS RESTRICTIVE + FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol + USING (cid IS NOT NULL AND dlevel > 0); +COMMENT ON POLICY p5 ON document_c IS 'policy p5'; +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check +--------------------+------------+------------+-------------+-------------------------------------------------------+-----+--------------------------------------+------------ + regress_rls_schema | document_c | p5 | RESTRICTIVE | {regress_rls_alice,regress_rls_bob,regress_rls_carol} | ALL | ((cid IS NOT NULL) AND (dlevel > 0)) | +(1 row) + +ALTER TABLE document_c ALTER COLUMN dlevel SET DATA TYPE text; --error +ERROR: operator does not exist: text > integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE bool USING (cid::bool); +ALTER TABLE document_c + ALTER COLUMN cid SET DATA TYPE INT8 USING cid::INT4::INT8, + ALTER COLUMN dlevel SET DATA TYPE INT8; +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check +--------------------+------------+------------+-------------+-------------------------------------------------------+-----+--------------------------------------+------------ + regress_rls_schema | document_c | p5 | RESTRICTIVE | {regress_rls_alice,regress_rls_bob,regress_rls_carol} | ALL | ((cid IS NOT NULL) AND (dlevel > 0)) | +(1 row) + +--comments should not change +SELECT polname, description +FROM pg_description, pg_policy c +WHERE classoid = 'pg_policy'::regclass AND objoid = c.oid AND c.polrelid = 'document_c'::regclass +ORDER BY polname; + polname | description +---------+------------- + p5 | policy p5 +(1 row) + +DROP TABLE document_c; -- -- ROLE/GROUP -- diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 5d923c5ca3b..a3a0f9e56f0 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -35,6 +35,25 @@ CREATE SCHEMA regress_rls_schema; GRANT ALL ON SCHEMA regress_rls_schema to public; SET search_path = regress_rls_schema; +--check alter column set data type will recreate security policy +CREATE TABLE rls_tbl (a int, b int, c int); +CREATE POLICY p1 ON rls_tbl + FOR UPDATE + TO regress_rls_alice, regress_rls_bob, regress_rls_carol + USING (a < 20) WITH CHECK (c <> 0 and a is not null); +CREATE POLICY p2 ON rls_tbl AS RESTRICTIVE + FOR ALL + TO PUBLIC + USING (a < 40) WITH CHECK (c > 0 and a is not null); + +ALTER TABLE rls_tbl ALTER COLUMN a SET DATA TYPE INT8, ALTER COLUMN c SET DATA TYPE INT8; + +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'rls_tbl' +ORDER BY policyname; + +DROP TABLE rls_tbl; + -- setup of malicious function CREATE OR REPLACE FUNCTION f_leak(text) RETURNS bool COST 0.0000001 LANGUAGE plpgsql @@ -1163,6 +1182,41 @@ DROP POLICY p1 ON document; -- Just check everything went per plan SELECT * FROM document; +CREATE TABLE document_c(LIKE document INCLUDING ALL); +CREATE POLICY p5 ON document_c AS RESTRICTIVE FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol +USING (CID IS NOT NULL AND + (WITH CTE AS (SELECT uaccount IS NOT NULL FROM uaccount) SELECT true FROM CTE)); + +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE INT8; --error +ALTER TABLE uaccount ALTER COLUMN seclv SET DATA TYPE INT8; --error + +DROP POLICY p5 ON document_c CASCADE; +CREATE POLICY p5 ON document_c AS RESTRICTIVE + FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol + USING (cid IS NOT NULL AND dlevel > 0); + +COMMENT ON POLICY p5 ON document_c IS 'policy p5'; + +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + +ALTER TABLE document_c ALTER COLUMN dlevel SET DATA TYPE text; --error +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE bool USING (cid::bool); +ALTER TABLE document_c + ALTER COLUMN cid SET DATA TYPE INT8 USING cid::INT4::INT8, + ALTER COLUMN dlevel SET DATA TYPE INT8; + +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + +--comments should not change +SELECT polname, description +FROM pg_description, pg_policy c +WHERE classoid = 'pg_policy'::regclass AND objoid = c.oid AND c.polrelid = 'document_c'::regclass +ORDER BY polname; + +DROP TABLE document_c; + -- -- ROLE/GROUP -- -- 2.34.1
From 75d8d77852af358e76710d27b4019da6ec376973 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 23 Dec 2025 09:58:44 +0800 Subject: [PATCH v3 2/3] refactoring CreatePolicy: split policy qual, check qual Currently CreatePolicy is only directly called through parser. But imagine copying policies from one table to another table (CREATE TABLE LIKE INCLUDING POLICIES) or changing a column's data type (ALTER COLUMN SET DATA TYPE). In these case, CreatePolicy will be called indirectly, either via the parser (indirectly), or by constructing a CreatePolicyStmt node from pg_policy catalog. As mentioned above, if we directly use pg_policy catalog info for constructing a new CreatePolicyStmt node, then the policy qual, with_check qual in the new node doesn't need to run parse analysis again, in fact, if we do parse analysis for transformed node again, it will fail. Another reason for refactoring is we may need to do parse analysis earlier because we can only use CreatePolicyStmt.rte, not CreatePolicyStmt.table. Overall splitting CreatePolicy qual, check qual parse analysis into a separate function will make CreatePolicy more neat. If you follow the call chain transformWhereClause -> transformExpr -> transformExprRecurse -> transformSubLink -> parse_sub_analyze, you can see that ParseState.p_rtable is not modified during transformWhereClause. Therefore, it is safe to pass a single RTE list to recordDependencyOnExpr during CreatePolicy. discussion: https://postgr.es/m/cacjufxe42vysvedemaobgmgylztcguawh_h-c9dcsldtd5j...@mail.gmail.com discussion: https://postgr.es/m/CACJufxFuEOB-i2z2qhyCG=dGwDf7g6Fs_o8cz=bui76uuuf...@mail.gmail.com --- src/backend/commands/policy.c | 54 +++++++----------------- src/backend/parser/gram.y | 1 + src/backend/parser/parse_utilcmd.c | 68 ++++++++++++++++++++++++++++++ src/backend/tcop/utility.c | 2 +- src/include/commands/policy.h | 2 +- src/include/nodes/parsenodes.h | 1 + src/include/parser/parse_utilcmd.h | 1 + 7 files changed, 88 insertions(+), 41 deletions(-) diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index a47744962e9..a595cd937d5 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -33,6 +33,7 @@ #include "parser/parse_collate.h" #include "parser/parse_node.h" #include "parser/parse_relation.h" +#include "parser/parse_utilcmd.h" #include "rewrite/rewriteManip.h" #include "rewrite/rowsecurity.h" #include "utils/acl.h" @@ -564,9 +565,10 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id) * handles the execution of the CREATE POLICY command. * * stmt - the CreatePolicyStmt that describes the policy to create. + * queryString - the source text of the CREATE POLICY command. */ ObjectAddress -CreatePolicy(CreatePolicyStmt *stmt) +CreatePolicy(CreatePolicyStmt *stmt, const char *queryString) { Relation pg_policy_rel; Oid policy_id; @@ -576,9 +578,7 @@ CreatePolicy(CreatePolicyStmt *stmt) Datum *role_oids; int nitems = 0; ArrayType *role_ids; - ParseState *qual_pstate; - ParseState *with_check_pstate; - ParseNamespaceItem *nsitem; + List *rtable; Node *qual; Node *with_check_qual; ScanKeyData skey[2]; @@ -615,10 +615,6 @@ CreatePolicy(CreatePolicyStmt *stmt) role_oids = policy_role_list_to_array(stmt->roles, &nitems); role_ids = construct_array_builtin(role_oids, nitems, OIDOID); - /* Parse the supplied clause */ - qual_pstate = make_parsestate(NULL); - with_check_pstate = make_parsestate(NULL); - /* zero-clear */ memset(values, 0, sizeof(values)); memset(isnull, 0, sizeof(isnull)); @@ -640,35 +636,17 @@ CreatePolicy(CreatePolicyStmt *stmt) stmt->rte->rellockmode = AccessExclusiveLock; } - /* Open target_table to build quals. No additional lock is necessary. */ + rtable = list_make1(stmt->rte); + + /* do parse analysis for qual and check qual */ + transformPolicyStmt(stmt, NULL); + + qual = stmt->qual; + with_check_qual = stmt->with_check; + + /* Open target_table, No additional lock is necessary. */ target_table = relation_open(table_id, NoLock); - /* Add for the regular security quals */ - nsitem = addRangeTableEntryForRelation(qual_pstate, target_table, - AccessShareLock, - NULL, false, false); - addNSItemToQuery(qual_pstate, nsitem, false, true, true); - - /* Add for the with-check quals */ - nsitem = addRangeTableEntryForRelation(with_check_pstate, target_table, - AccessShareLock, - NULL, false, false); - addNSItemToQuery(with_check_pstate, nsitem, false, true, true); - - qual = transformWhereClause(qual_pstate, - stmt->qual, - EXPR_KIND_POLICY, - "POLICY"); - - with_check_qual = transformWhereClause(with_check_pstate, - stmt->with_check, - EXPR_KIND_POLICY, - "POLICY"); - - /* Fix up collation information */ - assign_expr_collations(qual_pstate, qual); - assign_expr_collations(with_check_pstate, with_check_qual); - /* Open pg_policy catalog */ pg_policy_rel = table_open(PolicyRelationId, RowExclusiveLock); @@ -735,11 +713,11 @@ CreatePolicy(CreatePolicyStmt *stmt) recordDependencyOn(&myself, &target, DEPENDENCY_AUTO); - recordDependencyOnExpr(&myself, qual, qual_pstate->p_rtable, + recordDependencyOnExpr(&myself, qual, rtable, DEPENDENCY_NORMAL); recordDependencyOnExpr(&myself, with_check_qual, - with_check_pstate->p_rtable, DEPENDENCY_NORMAL); + rtable, DEPENDENCY_NORMAL); /* Register role dependencies */ target.classId = AuthIdRelationId; @@ -760,8 +738,6 @@ CreatePolicy(CreatePolicyStmt *stmt) /* Clean up. */ heap_freetuple(policy_tuple); - free_parsestate(qual_pstate); - free_parsestate(with_check_pstate); systable_endscan(sscan); relation_close(target_table, NoLock); table_close(pg_policy_rel, RowExclusiveLock); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index f6d46f08c22..97900568464 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -6029,6 +6029,7 @@ CreatePolicyStmt: n->roles = $8; n->qual = $9; n->with_check = $10; + n->transformed = false; $$ = (Node *) n; } ; diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 2b7b084f216..37f140d691d 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3208,6 +3208,74 @@ transformStatsStmt(Oid relid, CreateStatsStmt *stmt, const char *queryString) return stmt; } +/* + * transformPolicyStmt - parse analysis for CREATE POLICY + * + * To avoid race conditions, it's important that this function relies only on + * the relid on stmt->rte (and not on stmt->table) to determine the target + * relation. + */ +CreatePolicyStmt * +transformPolicyStmt(CreatePolicyStmt *stmt, const char *queryString) +{ + Relation target_table; + ParseNamespaceItem *nsitem; + ParseState *qual_pstate; + ParseState *with_check_pstate; + + /* Nothing to do if statement already transformed. */ + if (stmt->transformed) + { + Assert(stmt->rte != NULL); + return stmt; + } + + target_table = relation_open(stmt->rte->relid, NoLock); + + /* Parse the supplied clause */ + qual_pstate = make_parsestate(NULL); + with_check_pstate = make_parsestate(NULL); + + qual_pstate->p_sourcetext = queryString; + with_check_pstate->p_sourcetext = queryString; + + /* Add for the regular security quals */ + nsitem = addRangeTableEntryForRelation(qual_pstate, target_table, + AccessShareLock, + NULL, false, false); + addNSItemToQuery(qual_pstate, nsitem, false, true, true); + + /* Add for the with-check quals */ + nsitem = addRangeTableEntryForRelation(with_check_pstate, target_table, + AccessShareLock, + NULL, false, false); + addNSItemToQuery(with_check_pstate, nsitem, false, true, true); + + stmt->qual = transformWhereClause(qual_pstate, + stmt->qual, + EXPR_KIND_POLICY, + "POLICY"); + + stmt->with_check = transformWhereClause(with_check_pstate, + stmt->with_check, + EXPR_KIND_POLICY, + "POLICY"); + + /* Fix up collation information */ + assign_expr_collations(qual_pstate, stmt->qual); + assign_expr_collations(with_check_pstate, stmt->with_check); + + free_parsestate(qual_pstate); + free_parsestate(with_check_pstate); + + relation_close(target_table, NoLock); + + /* Mark statement as successfully transformed */ + stmt->transformed = true; + + return stmt; +} + /* * transformRuleStmt - diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index d18a3a60a46..de9dc267ef2 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1826,7 +1826,7 @@ ProcessUtilitySlow(ParseState *pstate, break; case T_CreatePolicyStmt: /* CREATE POLICY */ - address = CreatePolicy((CreatePolicyStmt *) parsetree); + address = CreatePolicy(castNode(CreatePolicyStmt, parsetree), queryString); break; case T_AlterPolicyStmt: /* ALTER POLICY */ diff --git a/src/include/commands/policy.h b/src/include/commands/policy.h index f06aa1df439..dab4030c38d 100644 --- a/src/include/commands/policy.h +++ b/src/include/commands/policy.h @@ -25,7 +25,7 @@ extern void RemovePolicyById(Oid policy_id); extern bool RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id); -extern ObjectAddress CreatePolicy(CreatePolicyStmt *stmt); +extern ObjectAddress CreatePolicy(CreatePolicyStmt *stmt, const char *queryString); extern ObjectAddress AlterPolicy(AlterPolicyStmt *stmt); extern Oid get_relation_policy_oid(Oid relid, const char *policy_name, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 7acbd2bf72c..c217ec73be9 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3109,6 +3109,7 @@ typedef struct CreatePolicyStmt List *roles; /* the roles associated with the policy */ Node *qual; /* the policy's condition */ Node *with_check; /* the policy's WITH CHECK condition. */ + bool transformed; /* true when transformPolicyStmt is finished */ } CreatePolicyStmt; /*---------------------- diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h index 4965fac4495..916df6eabbe 100644 --- a/src/include/parser/parse_utilcmd.h +++ b/src/include/parser/parse_utilcmd.h @@ -28,6 +28,7 @@ extern IndexStmt *transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString); extern CreateStatsStmt *transformStatsStmt(Oid relid, CreateStatsStmt *stmt, const char *queryString); +extern CreatePolicyStmt *transformPolicyStmt(CreatePolicyStmt *stmt, const char *queryString); extern void transformRuleStmt(RuleStmt *stmt, const char *queryString, List **actions, Node **whereClause); extern List *transformCreateSchemaStmtElements(List *schemaElts, -- 2.34.1
From cfe1a262bc5d9ea5b8cd8eba6005c52b56e2ebbf Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 23 Dec 2025 09:56:30 +0800 Subject: [PATCH v3 1/3] add RangeTblEntry to CreatePolicyStmt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently CreatePolicy is only directly called through parser. But imagine copying policies from one table to another table (CREATE TABLE LIKE INCLUDING POLICIES) or changing a column's data type (ALTER COLUMN SET DATA TYPE). In these case, CreatePolicy will be called indirectly, either via the parser, or by constructing a CreatePolicyStmt node from catalog metadata. These indirectly called CreatePolicy may cause repeated lookup CreatePolicyStmt->table RangeVar. (The relation already exists, but lookup up again via RangeVar). Generally we should avoid look up the same less-than-fully-qualified name multiple times, we might get different answers due to concurrent activity, and that might create a security vulnerability, along the lines of CVE-2014-0062. To avoid that, Add add a RangeTblEntry Node to CreatePolicyStmt, So we can record the Relation Oid advance. discussion: https://postgr.es/m/cacjufxe42vysvedemaobgmgylztcguawh_h-c9dcsldtd5j...@mail.gmail.com discussion: https://postgr.es/m/CACJufxFuEOB-i2z2qhyCG=dGwDf7g6Fs_o8cz=bui76uuuf...@mail.gmail.com --- src/backend/commands/policy.c | 21 ++++++++++++++++----- src/backend/parser/gram.y | 1 + src/include/nodes/parsenodes.h | 10 ++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index 5bd5f8c9968..a47744962e9 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -623,11 +623,22 @@ CreatePolicy(CreatePolicyStmt *stmt) memset(values, 0, sizeof(values)); memset(isnull, 0, sizeof(isnull)); - /* Get id of table. Also handles permissions checks. */ - table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock, - 0, - RangeVarCallbackForPolicy, - stmt); + if (stmt->rte != NULL) + table_id = stmt->rte->relid; + else + { + /* Get id of table. Also handles permissions checks. */ + table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock, + 0, + RangeVarCallbackForPolicy, + stmt); + + /* Create a range table entry. */ + stmt->rte = makeNode(RangeTblEntry); + stmt->rte->rtekind = RTE_RELATION; + stmt->rte->relid = table_id; + stmt->rte->rellockmode = AccessExclusiveLock; + } /* Open target_table to build quals. No additional lock is necessary. */ target_table = relation_open(table_id, NoLock); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 28f4e11e30f..f6d46f08c22 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -6023,6 +6023,7 @@ CreatePolicyStmt: n->policy_name = $3; n->table = $5; + n->rte = NULL; n->permissive = $6; n->cmd_name = $7; n->roles = $8; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index bc7adba4a0f..7acbd2bf72c 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3094,6 +3094,16 @@ typedef struct CreatePolicyStmt NodeTag type; char *policy_name; /* Policy's name */ RangeVar *table; /* the table name the policy applies to */ + + /* + * RangeTblEntry for the table. This is useful for avoid repeated name + * lookups issue. If CreatePolicyStmt.table has been looked up, we should + * not rely on it to resolve the relation again, use this rte field + * instead. This is useful when calling CreatePolicy not directly from + * parser. + */ + RangeTblEntry *rte; + char *cmd_name; /* the command name the policy applies to */ bool permissive; /* restrictive or permissive policy */ List *roles; /* the roles associated with the policy */ -- 2.34.1
