let ALTER COLUMN SET DATA TYPE cope with POLICY dependency
hi.
in [1]https://git.postgresql.org/cgit/postgresql.git/commit/?id=143b39c1855f8a22f474f20354ee5ee5d2f4d266,
RememberAllDependentForRebuilding
/*
* 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.
*/
After 11 year, I am trying to allow column type changes to cope with
security policy dependencies.
CREATE TABLE s (a int, b int);
CREATE POLICY p2 ON s USING (s.b = 1);
--master branch will result error
ALTER TABLE s ALTER COLUMN b SET DATA TYPE INT8;
ERROR: cannot alter type of a column used in a policy definition
DETAIL: policy p2 on table s depends on column "b"
with the attached patch, ALTER TABLE SET DATA TYPE can cope with columns that
have associated security policy.
The above ALTER TABLE SET DATA TYPE will just work fine.
The code roughly follows how statistics are recreated after a column
data type change.
Currently table rewrite does not recheck the policy expression, for example:
RESET SESSION AUTHORIZATION;
CREATE USER regress_rls_alice NOLOGIN;
GRANT ALL ON SCHEMA public to public;
DROP TABLE IF EXISTS R1;
SET row_security = on;
begin;
set role regress_rls_alice;
CREATE TABLE r1 (a int, b int GENERATED ALWAYS AS (a * 10) STORED);
INSERT INTO r1 VALUES (1), (2), (4);
CREATE POLICY p0 ON r1 USING (true);
CREATE POLICY p1 ON r1 AS RESTRICTIVE USING (b > 10);
ALTER TABLE r1 ENABLE ROW LEVEL SECURITY;
ALTER TABLE r1 FORCE ROW LEVEL SECURITY;
commit;
set role regress_rls_alice;
INSERT INTO r1 VALUES (0); -- Should fail p1
ALTER TABLE r1 ALTER COLUMN b SET EXPRESSION AS (-1); --OK
so i guess ALTER TABLE SET DATA TYPE, table rewrite no checking policy
should be fine?
[1]: https://git.postgresql.org/cgit/postgresql.git/commit/?id=143b39c1855f8a22f474f20354ee5ee5d2f4d266
Attachments:
v1-0001-let-ALTER-COLUMN-SET-DATA-TYPE-cope-with-POLICY-dependency.patchtext/x-patch; charset=US-ASCII; name=v1-0001-let-ALTER-COLUMN-SET-DATA-TYPE-cope-with-POLICY-dependency.patchDownload
From be887b714a3bc788b6859954fad63137d64d1f61 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 12 Sep 2025 16:06:53 +0800
Subject: [PATCH v1 1/1] let ALTER COLUMN SET DATA TYPE cope with POLICY
dependency
CREATE TABLE s (a int, b int);
CREATE POLICY p2 ON s USING (s.b = 1);
--no-error, while master branch will result error
ALTER TABLE s ALTER COLUMN b SET DATA TYPE INT8;
discussion: https://postgr.es/m/
---
src/backend/commands/policy.c | 59 ++++++
src/backend/commands/tablecmds.c | 122 ++++++++++--
src/backend/parser/gram.y | 1 +
src/backend/utils/adt/ruleutils.c | 185 ++++++++++++++++++
src/include/commands/policy.h | 1 +
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 | 79 ++++++++
src/test/regress/sql/rowsecurity.sql | 41 ++++
10 files changed, 479 insertions(+), 15 deletions(-)
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 83056960fe4..c287c08f155 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -24,8 +24,10 @@
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_authid.h"
+#include "catalog/pg_depend.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"
@@ -755,6 +757,11 @@ CreatePolicy(CreatePolicyStmt *stmt)
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;
}
@@ -1277,3 +1284,55 @@ relation_has_policies(Relation rel)
return ret;
}
+
+/*
+ * PoliciesGetRelations -
+ * Collect all relations that this policy depends on.
+ * Since the policy's check qual or qual may reference other relations, we
+ * include those as well.
+ */
+List *
+PoliciesGetRelations(Oid policyId)
+{
+ List *result = NIL;
+ Relation depRel;
+ ScanKeyData key[2];
+ SysScanDesc depScan;
+ HeapTuple depTup;
+
+ /*
+ * We scan pg_depend to find those things that policy being depended on.
+ */
+ depRel = table_open(DependRelationId, AccessShareLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(PolicyRelationId));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(policyId));
+
+ depScan = systable_beginscan(depRel, DependDependerIndexId, true,
+ NULL, 2, key);
+ while (HeapTupleIsValid(depTup = systable_getnext(depScan)))
+ {
+ Form_pg_depend pg_depend = (Form_pg_depend) GETSTRUCT(depTup);
+
+ /* Prepend oid of the relation with this policy. */
+ if (pg_depend->refclassid == RelationRelationId)
+ {
+ if (pg_depend->deptype == DEPENDENCY_AUTO)
+ result = lcons_oid(pg_depend->refobjid, result);
+ else
+ result = lappend_oid(result, pg_depend->refobjid);
+ }
+ }
+ systable_endscan(depScan);
+
+ relation_close(depRel, AccessShareLock);
+
+ Assert(result != NIL);
+ return result;
+}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3be2e051d32..341f8be7299 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,
@@ -651,6 +656,7 @@ static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableT
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,
@@ -5449,6 +5455,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, (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)
@@ -6716,6 +6726,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;
@@ -9663,6 +9675,26 @@ 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 policies 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));
+
+ address = CreatePolicy(stmt);
+
+ return address;
+}
+
/*
* ALTER TABLE ADD CONSTRAINT USING INDEX
*
@@ -15136,22 +15168,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:
@@ -15393,6 +15411,33 @@ 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.
+ */
+ if (!list_member_oid(tab->changedPolicyOids, policyId))
+ {
+ /* OK, capture the policies's existing definition string */
+ char *defstring = pg_get_policy_def_command(policyId);
+
+ tab->changedPolicyOids = lappend_oid(tab->changedPolicyOids,
+ policyId);
+ tab->changedPolicyDefs = lappend(tab->changedPolicyDefs,
+ defstring);
+ }
+}
+
+
/*
* 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
@@ -15537,6 +15582,39 @@ 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);
+ List *relids;
+
+ relids = PoliciesGetRelations(oldId);
+ Assert(relids != NIL);
+
+ /*
+ * 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.
+ */
+ foreach_oid(relid, relids)
+ {
+ if (relid != tab->relid)
+ LockRelationOid(relid, AccessExclusiveLock);
+ }
+
+ 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
*/
@@ -15788,6 +15866,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 policies 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 9fd48acb1f8..e795bf171bb 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -5942,6 +5942,7 @@ CreatePolicyStmt:
CreatePolicyStmt *n = makeNode(CreatePolicyStmt);
n->policy_name = $3;
+ n->polcomment = NULL;
n->table = $5;
n->permissive = $6;
n->cmd_name = $7;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3d6e6bdbfd2..d5685be6770 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -33,6 +33,7 @@
#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"
@@ -57,6 +58,7 @@
#include "rewrite/rewriteHandler.h"
#include "rewrite/rewriteManip.h"
#include "rewrite/rewriteSupport.h"
+#include "utils/acl.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.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);
@@ -2611,6 +2614,188 @@ 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)
+{
+ return pg_get_policydef_worker(policyId, 0, false);
+}
+
+
+/*
+ * get_policy_applied_command -
+ * Helper function to convert char representation to their full command strings.
+ * Returned valid values are 'all', 'select', 'insert', 'update' and 'delete'.
+ */
+static 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;
+ }
+}
+
+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 = (Node *) stringToNode(str_value);
+ pfree(str_value);
+
+ exprsrc = deparse_expression_pretty(expr, context, false, false,
+ prettyFlags, 0);
+ appendStringInfo(&buf, " USING (%s) ", exprsrc);
+ }
+
+ /* Get WITH CHECK qual */
+ datum = heap_getattr(tup, Anum_pg_policy_polwithcheck,
+ RelationGetDescr(relation), &isnull);
+ if (!isnull)
+ {
+ str_value = TextDatumGetCString(datum);
+ expr = (Node *) 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 f06aa1df439..a39cda611cf 100644
--- a/src/include/commands/policy.h
+++ b/src/include/commands/policy.h
@@ -34,5 +34,6 @@ 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 List *PoliciesGetRelations(Oid policyId);
#endif /* POLICY_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 86a236bd58b..921c0a61520 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2477,6 +2477,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 */
@@ -3064,6 +3065,7 @@ typedef struct CreatePolicyStmt
char *policy_name; /* Policy's name */
RangeVar *table; /* the table name the policy applies to */
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 5f2ea2e4d0e..a82f83c6c72 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 193669f2bc1..f438936b719 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -308,6 +308,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 8c879509313..ac1e8a234ad 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -26,6 +26,45 @@ 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 also drop whole-row references policy
+CREATE TABLE rls_tbl (a int, b int, c int);
+CREATE POLICY p0 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 p1 ON rls_tbl AS RESTRICTIVE
+ FOR ALL
+ TO regress_rls_alice, regress_rls_carol, regress_rls_bob
+ USING (a < 30) WITH CHECK (c > 0 and a is not null);
+CREATE POLICY p2 ON rls_tbl AS RESTRICTIVE
+ FOR UPDATE
+ TO PUBLIC
+ USING (a < 40) WITH CHECK (c > 0 and a is not null);
+COMMENT ON POLICY p0 ON rls_tbl IS 'policy p1';
+COMMENT ON POLICY p1 ON rls_tbl IS 'policy p2';
+COMMENT ON POLICY p2 ON rls_tbl IS 'policy p3';
+ALTER TABLE rls_tbl ALTER COLUMN a SET DATA TYPE INT8, ALTER COLUMN c SET DATA TYPE INT8;
+SELECT polname, description
+FROM pg_description, pg_policy c
+WHERE classoid = 'pg_policy'::regclass
+AND objoid = c.oid AND c.polrelid = 'rls_tbl'::regclass
+ORDER BY polname;
+ polname | description
+---------+-------------
+ p0 | policy p1
+ p1 | policy p2
+ p2 | policy p3
+(3 rows)
+
+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 | p0 | 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 | p1 | RESTRICTIVE | {regress_rls_alice,regress_rls_bob,regress_rls_carol} | ALL | (a < 30) | ((c > 0) AND (a IS NOT NULL))
+ regress_rls_schema | rls_tbl | p2 | RESTRICTIVE | {public} | UPDATE | (a < 40) | ((c > 0) AND (a IS NOT NULL))
+(3 rows)
+
+DROP TABLE rls_tbl;
-- setup of malicious function
CREATE OR REPLACE FUNCTION f_leak(text) RETURNS bool
COST 0.0000001 LANGUAGE plpgsql
@@ -439,6 +478,46 @@ EXPLAIN (COSTS OFF) SELECT * FROM document NATURAL JOIN category WHERE f_leak(dt
Index Cond: (cid = document.cid)
(5 rows)
+SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document' ORDER BY policyname;
+ schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check
+--------------------+-----------+------------+-------------+--------------------+-----+------------------------------+------------
+ regress_rls_schema | document | p1 | PERMISSIVE | {public} | ALL | (dauthor = CURRENT_USER) |
+ regress_rls_schema | document | p1r | RESTRICTIVE | {regress_rls_dave} | ALL | (cid <> 44) |
+ regress_rls_schema | document | p2r | RESTRICTIVE | {regress_rls_dave} | ALL | ((cid <> 44) AND (cid < 50)) |
+(3 rows)
+
+RESET SESSION AUTHORIZATION;
+ALTER TABLE document ALTER COLUMN cid SET DATA TYPE INT8;
+SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document' ORDER BY policyname;
+ schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check
+--------------------+-----------+------------+-------------+--------------------+-----+------------------------------+------------
+ regress_rls_schema | document | p1 | PERMISSIVE | {public} | ALL | (dauthor = CURRENT_USER) |
+ regress_rls_schema | document | p1r | RESTRICTIVE | {regress_rls_dave} | ALL | (cid <> 44) |
+ regress_rls_schema | document | p2r | RESTRICTIVE | {regress_rls_dave} | ALL | ((cid <> 44) AND (cid < 50)) |
+(3 rows)
+
+-- viewpoint from rls_regres_carol again
+SET SESSION AUTHORIZATION regress_rls_carol;
+EXPLAIN (COSTS OFF) SELECT * FROM document WHERE f_leak(dtitle);
+ QUERY PLAN
+---------------------------------------------------------
+ Seq Scan on document
+ Filter: ((dauthor = CURRENT_USER) AND f_leak(dtitle))
+(2 rows)
+
+EXPLAIN (COSTS OFF) SELECT * FROM document NATURAL JOIN category WHERE f_leak(dtitle);
+ QUERY PLAN
+---------------------------------------------------------------
+ Nested Loop
+ -> Seq Scan on document
+ Filter: ((dauthor = CURRENT_USER) AND f_leak(dtitle))
+ -> Index Scan using category_pkey on category
+ Index Cond: (cid = document.cid)
+(5 rows)
+
+--change data type back
+RESET SESSION AUTHORIZATION;
+ALTER TABLE document ALTER COLUMN cid SET DATA TYPE INT4;
-- interaction of FK/PK constraints
SET SESSION AUTHORIZATION regress_rls_alice;
CREATE POLICY p2 ON category
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 21ac0ca51ee..4d930dcae2e 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -35,6 +35,35 @@ 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 also drop whole-row references policy
+CREATE TABLE rls_tbl (a int, b int, c int);
+CREATE POLICY p0 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 p1 ON rls_tbl AS RESTRICTIVE
+ FOR ALL
+ TO regress_rls_alice, regress_rls_carol, regress_rls_bob
+ USING (a < 30) WITH CHECK (c > 0 and a is not null);
+CREATE POLICY p2 ON rls_tbl AS RESTRICTIVE
+ FOR UPDATE
+ TO PUBLIC
+ USING (a < 40) WITH CHECK (c > 0 and a is not null);
+COMMENT ON POLICY p0 ON rls_tbl IS 'policy p1';
+COMMENT ON POLICY p1 ON rls_tbl IS 'policy p2';
+COMMENT ON POLICY p2 ON rls_tbl IS 'policy p3';
+
+ALTER TABLE rls_tbl ALTER COLUMN a SET DATA TYPE INT8, ALTER COLUMN c SET DATA TYPE INT8;
+
+SELECT polname, description
+FROM pg_description, pg_policy c
+WHERE classoid = 'pg_policy'::regclass
+AND objoid = c.oid AND c.polrelid = 'rls_tbl'::regclass
+ORDER BY polname;
+
+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
@@ -167,6 +196,18 @@ SELECT * FROM document NATURAL JOIN category WHERE f_leak(dtitle) ORDER by did;
EXPLAIN (COSTS OFF) SELECT * FROM document WHERE f_leak(dtitle);
EXPLAIN (COSTS OFF) SELECT * FROM document NATURAL JOIN category WHERE f_leak(dtitle);
+SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document' ORDER BY policyname;
+RESET SESSION AUTHORIZATION;
+ALTER TABLE document ALTER COLUMN cid SET DATA TYPE INT8;
+SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document' ORDER BY policyname;
+-- viewpoint from rls_regres_carol again
+SET SESSION AUTHORIZATION regress_rls_carol;
+EXPLAIN (COSTS OFF) SELECT * FROM document WHERE f_leak(dtitle);
+EXPLAIN (COSTS OFF) SELECT * FROM document NATURAL JOIN category WHERE f_leak(dtitle);
+--change data type back
+RESET SESSION AUTHORIZATION;
+ALTER TABLE document ALTER COLUMN cid SET DATA TYPE INT4;
+
-- interaction of FK/PK constraints
SET SESSION AUTHORIZATION regress_rls_alice;
CREATE POLICY p2 ON category
--
2.34.1
On Fri, Sep 12, 2025 at 4:19 PM jian he <jian.universality@gmail.com> wrote:
hi.
in [1],
RememberAllDependentForRebuilding
/*
* 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.
*/
After 11 year, I am trying to allow column type changes to cope with
security policy dependencies.CREATE TABLE s (a int, b int);
CREATE POLICY p2 ON s USING (s.b = 1);
--master branch will result error
ALTER TABLE s ALTER COLUMN b SET DATA TYPE INT8;
ERROR: cannot alter type of a column used in a policy definition
DETAIL: policy p2 on table s depends on column "b"with the attached patch, ALTER TABLE SET DATA TYPE can cope with columns that
have associated security policy.
The above ALTER TABLE SET DATA TYPE will just work fine.
The code roughly follows how statistics are recreated after a column
data type change.
v1 coding is not as aligned as with how statistics are recreated after
data changes.
For example, we have transformStatsStmt, but don't have transformPolicyStmt.
v2-0001 refactor CreatePolicy.
introduce transformPolicyStmt, very similar to transformStatsStmt,
and we don't need two ParseState (qual_pstate, with_check_pstate).
but we need one ParseState for recordDependencyOnExpr.
v2-0002 is the same as v1-0001 mostly, using transformPolicyStmt in
ATPostAlterTypeParse.
Attachments:
v2-0002-let-ALTER-COLUMN-SET-DATA-TYPE-cope-with-POLICY-dependency.patchtext/x-patch; charset=US-ASCII; name=v2-0002-let-ALTER-COLUMN-SET-DATA-TYPE-cope-with-POLICY-dependency.patchDownload
From 2f54d9ef0dcb62ea2ec6b10ff7f0a75ea53e795a Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Mon, 15 Sep 2025 02:29:07 +0800
Subject: [PATCH v2 2/2] let ALTER COLUMN SET DATA TYPE cope with POLICY
dependency
CREATE TABLE s (a int, b int);
CREATE POLICY p2 ON s USING (s.b = 1);
--no-error, while master branch will result error
ALTER TABLE s ALTER COLUMN b SET DATA TYPE INT8;
discussion: https://postgr.es/m/CACJufxE42vysVEDEmaoBGmGYLZTCgUAwh_h-c9dcSLDtD5jE3g@mail.gmail.com
---
src/backend/commands/policy.c | 59 ++++++
src/backend/commands/tablecmds.c | 135 +++++++++++--
src/backend/parser/gram.y | 1 +
src/backend/utils/adt/ruleutils.c | 189 ++++++++++++++++++
src/include/commands/policy.h | 1 +
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 | 50 ++++-
src/test/regress/sql/rowsecurity.sql | 38 ++++
10 files changed, 461 insertions(+), 18 deletions(-)
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 799e1e3968a..be0e4be37da 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -24,8 +24,10 @@
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_authid.h"
+#include "catalog/pg_depend.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"
@@ -738,6 +740,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;
}
@@ -1260,3 +1267,55 @@ relation_has_policies(Relation rel)
return ret;
}
+
+/*
+ * PoliciesGetRelations -
+ * Collect all relations this policy depends on.
+ * The policy's check qual or qual may reference other relations, we include
+ * those as well.
+ */
+List *
+PoliciesGetRelations(Oid policyId)
+{
+ List *result = NIL;
+ Relation depRel;
+ ScanKeyData key[2];
+ SysScanDesc depScan;
+ HeapTuple depTup;
+
+ /*
+ * We scan pg_depend to find those things that policy being depended on.
+ */
+ depRel = table_open(DependRelationId, AccessShareLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_classid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(PolicyRelationId));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(policyId));
+
+ depScan = systable_beginscan(depRel, DependDependerIndexId, true,
+ NULL, 2, key);
+ while (HeapTupleIsValid(depTup = systable_getnext(depScan)))
+ {
+ Form_pg_depend pg_depend = (Form_pg_depend) GETSTRUCT(depTup);
+
+ /* Prepend oid of the relation with this policy. */
+ if (pg_depend->refclassid == RelationRelationId)
+ {
+ if (pg_depend->deptype == DEPENDENCY_AUTO)
+ result = lcons_oid(pg_depend->refobjid, result);
+ else
+ result = lappend_oid(result, pg_depend->refobjid);
+ }
+ }
+ systable_endscan(depScan);
+
+ relation_close(depRel, AccessShareLock);
+
+ Assert(result != NIL);
+ return result;
+}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3be2e051d32..0508d42ed89 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,
@@ -651,6 +656,7 @@ static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableT
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,
@@ -5449,6 +5455,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, (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)
@@ -6716,6 +6726,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;
@@ -9663,6 +9675,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
*
@@ -15136,22 +15171,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:
@@ -15393,6 +15414,33 @@ 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.
+ */
+ if (!list_member_oid(tab->changedPolicyOids, policyId))
+ {
+ /* OK, capture the policies's existing definition string */
+ char *defstring = pg_get_policy_def_command(policyId);
+
+ tab->changedPolicyOids = lappend_oid(tab->changedPolicyOids,
+ policyId);
+ tab->changedPolicyDefs = lappend(tab->changedPolicyDefs,
+ defstring);
+ }
+}
+
+
/*
* 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
@@ -15537,6 +15585,39 @@ 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);
+ List *relids;
+
+ relids = PoliciesGetRelations(oldId);
+ Assert(relids != NIL);
+
+ /*
+ * 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.
+ */
+ foreach_oid(relid, relids)
+ {
+ if (relid != tab->relid)
+ LockRelationOid(relid, AccessExclusiveLock);
+ }
+
+ 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
*/
@@ -15585,8 +15666,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
@@ -15638,6 +15719,12 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
transformStatsStmt(oldRelId,
(CreateStatsStmt *) stmt,
cmd));
+ else if (IsA(stmt, CreatePolicyStmt))
+ querytree_list = lappend(querytree_list,
+ transformPolicyStmt(oldRelId,
+ (CreatePolicyStmt *) stmt,
+ cmd));
+
else
querytree_list = lappend(querytree_list, stmt);
}
@@ -15788,6 +15875,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 8016c58b49c..1d206bc37c5 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -5942,6 +5942,7 @@ CreatePolicyStmt:
CreatePolicyStmt *n = makeNode(CreatePolicyStmt);
n->policy_name = $3;
+ n->polcomment = NULL;
n->table = $5;
n->permissive = $6;
n->cmd_name = $7;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3d6e6bdbfd2..d17b4f8148c 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -33,6 +33,7 @@
#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"
@@ -57,6 +58,7 @@
#include "rewrite/rewriteHandler.h"
#include "rewrite/rewriteManip.h"
#include "rewrite/rewriteSupport.h"
+#include "utils/acl.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.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);
@@ -2611,6 +2614,192 @@ 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);
+}
+
+
+/*
+ * get_policy_applied_command -
+ * Helper function to convert pg_policy.polcmd char representation to their full
+ * command strings.
+ * Returned valid values are 'all', 'select', 'insert', 'update' and 'delete'.
+ */
+static 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;
+ }
+}
+
+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 = (Node *) 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 = (Node *) 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..8796f302e1b 100644
--- a/src/include/commands/policy.h
+++ b/src/include/commands/policy.h
@@ -34,5 +34,6 @@ 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 List *PoliciesGetRelations(Oid policyId);
#endif /* POLICY_H */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index a8cc660d5e6..aaf8127ed9e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2477,6 +2477,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 */
@@ -3064,6 +3065,7 @@ typedef struct CreatePolicyStmt
char *policy_name; /* Policy's name */
RangeVar *table; /* the table name the policy applies to */
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 5f2ea2e4d0e..a82f83c6c72 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 193669f2bc1..f438936b719 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -308,6 +308,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 1dc8e5c8f42..0314f851629 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -26,6 +26,25 @@ 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
@@ -2357,6 +2376,33 @@ SELECT * FROM document;
14 | 11 | 1 | regress_rls_bob | new novel |
(16 rows)
+CREATE POLICY p5 ON document 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 * FROM CTE WHERE EXISTS (
+ SELECT category FROM category WHERE EXISTS (SELECT uaccount FROM uaccount WHERE uaccount IS NULL))));
+COMMENT ON POLICY p5 ON document IS 'policy p5';
+CREATE TABLE pre_type_change AS SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document';
+ALTER TABLE document ALTER COLUMN cid SET DATA TYPE INT8;
+CREATE TABLE after_type_change AS SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document';
+--should return zero row.
+SELECT * FROM after_type_change
+EXCEPT
+SELECT * FROM after_type_change;
+ schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check
+------------+-----------+------------+------------+-------+-----+------+------------
+(0 rows)
+
+--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'::regclass
+ORDER BY polname;
+ polname | description
+---------+-------------
+ p5 | policy p5
+(1 row)
+
--
-- ROLE/GROUP
--
@@ -4824,7 +4870,7 @@ drop table rls_t, test_t;
--
RESET SESSION AUTHORIZATION;
DROP SCHEMA regress_rls_schema CASCADE;
-NOTICE: drop cascades to 30 other objects
+NOTICE: drop cascades to 32 other objects
DETAIL: drop cascades to function f_leak(text)
drop cascades to table uaccount
drop cascades to table category
@@ -4840,6 +4886,8 @@ drop cascades to table s2
drop cascades to view v2
drop cascades to table b1
drop cascades to view bv1
+drop cascades to table pre_type_change
+drop cascades to table after_type_change
drop cascades to table z1
drop cascades to table z2
drop cascades to table z1_blacklist
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 21ac0ca51ee..f740555fee5 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -35,6 +35,21 @@ 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
@@ -1021,6 +1036,29 @@ DROP POLICY p1 ON document;
-- Just check everything went per plan
SELECT * FROM document;
+CREATE POLICY p5 ON document 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 * FROM CTE WHERE EXISTS (
+ SELECT category FROM category WHERE EXISTS (SELECT uaccount FROM uaccount WHERE uaccount IS NULL))));
+
+COMMENT ON POLICY p5 ON document IS 'policy p5';
+
+CREATE TABLE pre_type_change AS SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document';
+ALTER TABLE document ALTER COLUMN cid SET DATA TYPE INT8;
+CREATE TABLE after_type_change AS SELECT * FROM pg_policies WHERE schemaname = 'regress_rls_schema' AND tablename = 'document';
+
+--should return zero row.
+SELECT * FROM after_type_change
+EXCEPT
+SELECT * FROM after_type_change;
+
+--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'::regclass
+ORDER BY polname;
+
--
-- ROLE/GROUP
--
--
2.34.1
v2-0001-refactor-CreatePolicy.patchtext/x-patch; charset=US-ASCII; name=v2-0001-refactor-CreatePolicy.patchDownload
From 777ec1a215c7e4865618f70c32dcb819192b0b0a Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Mon, 15 Sep 2025 10:49:39 +0800
Subject: [PATCH v2 1/2] refactor CreatePolicy
discussion: https://postgr.es/m/CACJufxE42vysVEDEmaoBGmGYLZTCgUAwh_h-c9dcSLDtD5jE3g@mail.gmail.com
---
src/backend/commands/policy.c | 53 +++++++-------------
src/backend/parser/gram.y | 1 +
src/backend/parser/parse_utilcmd.c | 59 +++++++++++++++++++++++
src/backend/tcop/utility.c | 2 +-
src/include/commands/policy.h | 2 +-
src/include/nodes/parsenodes.h | 1 +
src/include/parser/parse_utilcmd.h | 2 +
src/test/regress/expected/rowsecurity.out | 2 +
8 files changed, 85 insertions(+), 37 deletions(-)
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 83056960fe4..799e1e3968a 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"
@@ -566,7 +567,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
* stmt - the CreatePolicyStmt that describes the policy to create.
*/
ObjectAddress
-CreatePolicy(CreatePolicyStmt *stmt)
+CreatePolicy(CreatePolicyStmt *stmt, const char *queryString)
{
Relation pg_policy_rel;
Oid policy_id;
@@ -576,8 +577,7 @@ CreatePolicy(CreatePolicyStmt *stmt)
Datum *role_oids;
int nitems = 0;
ArrayType *role_ids;
- ParseState *qual_pstate;
- ParseState *with_check_pstate;
+ ParseState *pstate;
ParseNamespaceItem *nsitem;
Node *qual;
Node *with_check_qual;
@@ -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));
@@ -628,35 +624,23 @@ CreatePolicy(CreatePolicyStmt *stmt)
0,
RangeVarCallbackForPolicy,
stmt);
+ if (!stmt->transformed)
+ stmt = transformPolicyStmt(table_id, stmt, queryString);
- /* Open target_table to build quals. No additional lock is necessary. */
+ qual = stmt->qual;
+ with_check_qual = stmt->with_check;
+
+ /* we'll need the pstate->rtable for recordDependencyOnExpr */
+ pstate = make_parsestate(NULL);
+ pstate->p_sourcetext = queryString;
+
+ /* No additional lock is necessary. */
target_table = relation_open(table_id, NoLock);
- /* Add for the regular security quals */
- nsitem = addRangeTableEntryForRelation(qual_pstate, target_table,
+ nsitem = addRangeTableEntryForRelation(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);
+ addNSItemToQuery(pstate, nsitem, false, true, true);
/* Open pg_policy catalog */
pg_policy_rel = table_open(PolicyRelationId, RowExclusiveLock);
@@ -724,11 +708,11 @@ CreatePolicy(CreatePolicyStmt *stmt)
recordDependencyOn(&myself, &target, DEPENDENCY_AUTO);
- recordDependencyOnExpr(&myself, qual, qual_pstate->p_rtable,
+ recordDependencyOnExpr(&myself, qual, pstate->p_rtable,
DEPENDENCY_NORMAL);
recordDependencyOnExpr(&myself, with_check_qual,
- with_check_pstate->p_rtable, DEPENDENCY_NORMAL);
+ pstate->p_rtable, DEPENDENCY_NORMAL);
/* Register role dependencies */
target.classId = AuthIdRelationId;
@@ -749,8 +733,7 @@ CreatePolicy(CreatePolicyStmt *stmt)
/* Clean up. */
heap_freetuple(policy_tuple);
- free_parsestate(qual_pstate);
- free_parsestate(with_check_pstate);
+ free_parsestate(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 9fd48acb1f8..8016c58b49c 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -5948,6 +5948,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 e96b38a59d5..394a037e817 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3205,6 +3205,65 @@ transformStatsStmt(Oid relid, CreateStatsStmt *stmt, const char *queryString)
return stmt;
}
+/*
+ * transformPolicyStmt - parse analysis for CREATE POLICY
+ * mainly parse analysis for qual and check qual of the policy.
+ *
+ * To avoid race conditions, it's important that this function relies only on
+ * the passed-in relid (and not on stmt->table) to determine the target
+ * relation.
+ */
+CreatePolicyStmt *
+transformPolicyStmt(Oid relid, CreatePolicyStmt *stmt, const char *queryString)
+{
+ ParseState *pstate;
+ ParseNamespaceItem *nsitem;
+ Relation rel;
+
+ /* Nothing to do if statement already transformed. */
+ if (stmt->transformed)
+ return stmt;
+
+ /* Set up pstate */
+ pstate = make_parsestate(NULL);
+ pstate->p_sourcetext = queryString;
+
+ /*
+ * Put the parent table into the rtable so that the expressions can refer
+ * to its fields without qualification. Caller is responsible for locking
+ * relation, but we still need to open it.
+ */
+ rel = relation_open(relid, NoLock);
+ nsitem = addRangeTableEntryForRelation(pstate, rel,
+ AccessShareLock,
+ NULL, false, true);
+
+ /* no to join list, yes to namespaces */
+ addNSItemToQuery(pstate, nsitem, false, true, true);
+
+ stmt->qual = transformWhereClause(pstate,
+ stmt->qual,
+ EXPR_KIND_POLICY,
+ "POLICY");
+
+ stmt->with_check = transformWhereClause(pstate,
+ stmt->with_check,
+ EXPR_KIND_POLICY,
+ "POLICY");
+ /* Fix up collation information */
+ assign_expr_collations(pstate, stmt->qual);
+ assign_expr_collations(pstate, stmt->with_check);
+
+ free_parsestate(pstate);
+
+ /* Close relation */
+ table_close(rel, 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 5f442bc3bd4..e8b3deca825 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1818,7 +1818,7 @@ ProcessUtilitySlow(ParseState *pstate,
break;
case T_CreatePolicyStmt: /* CREATE POLICY */
- address = CreatePolicy((CreatePolicyStmt *) parsetree);
+ address = CreatePolicy((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 86a236bd58b..a8cc660d5e6 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3068,6 +3068,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 9f2b58de797..7a3562b88c2 100644
--- a/src/include/parser/parse_utilcmd.h
+++ b/src/include/parser/parse_utilcmd.h
@@ -28,6 +28,8 @@ extern IndexStmt *transformIndexStmt(Oid relid, IndexStmt *stmt,
const char *queryString);
extern CreateStatsStmt *transformStatsStmt(Oid relid, CreateStatsStmt *stmt,
const char *queryString);
+extern CreatePolicyStmt *transformPolicyStmt(Oid relid, CreatePolicyStmt *stmt,
+ const char *queryString);
extern void transformRuleStmt(RuleStmt *stmt, const char *queryString,
List **actions, Node **whereClause);
extern List *transformCreateSchemaStmtElements(List *schemaElts,
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 8c879509313..1dc8e5c8f42 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -4084,6 +4084,8 @@ BEGIN;
CREATE TABLE t (c) AS VALUES ('bar'::text);
CREATE POLICY p ON t USING (max(c)); -- fails: aggregate functions are not allowed in policy expressions
ERROR: aggregate functions are not allowed in policy expressions
+LINE 1: CREATE POLICY p ON t USING (max(c));
+ ^
ROLLBACK;
--
-- Non-target relations are only subject to SELECT policies
--
2.34.1
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]/messages/by-id/CACJufxFuEOB-i2z2qhyCG=dGwDf7g6Fs_o8cz=BUi76UuUFSOA@mail.gmail.com too.
[1]: /messages/by-id/CACJufxFuEOB-i2z2qhyCG=dGwDf7g6Fs_o8cz=BUi76UuUFSOA@mail.gmail.com
Attachments:
v3-0003-ALTER-TABLE-command-to-change-policy.patchtext/x-patch; charset=UTF-8; name=v3-0003-ALTER-TABLE-command-to-change-policy.patchDownload
From 56992a0fc4df5c4e3f55601afc4df9853235e9ee Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
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-c9dcSLDtD5jE3g@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
v3-0002-refactoring-CreatePolicy-split-policy-qual-check-qual.patchtext/x-patch; charset=US-ASCII; name=v3-0002-refactoring-CreatePolicy-split-policy-qual-check-qual.patchDownload
From 75d8d77852af358e76710d27b4019da6ec376973 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
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-c9dcSLDtD5jE3g@mail.gmail.com
discussion: https://postgr.es/m/CACJufxFuEOB-i2z2qhyCG=dGwDf7g6Fs_o8cz=BUi76UuUFSOA@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
v3-0001-add-RangeTblEntry-to-CreatePolicyStmt.patchtext/x-patch; charset=UTF-8; name=v3-0001-add-RangeTblEntry-to-CreatePolicyStmt.patchDownload
From cfe1a262bc5d9ea5b8cd8eba6005c52b56e2ebbf Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
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-c9dcSLDtD5jE3g@mail.gmail.com
discussion: https://postgr.es/m/CACJufxFuEOB-i2z2qhyCG=dGwDf7g6Fs_o8cz=BUi76UuUFSOA@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