v7-0003-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patch
application/x-patch
Filename: v7-0003-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patch
Type: application/x-patch
Part: 1
From 5e557d6c7c15bedc086576a5fdcfc7d47c9e2c8a Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 25 Dec 2025 22:59:19 +0800
Subject: [PATCH v7 3/3] disallow ALTER TABLE ALTER COLUMN when wholerow
referenced policy exists
Policy have a DEPENDENCY_NORMAL type with their source table. Policy's qual and
with check qual are quite unconstrained (allowing subqueries), we can't reliably
use pull_varattnos to detect if they contain subqueries. A further complication
is that the qual and with check qual whole-row Var may not only references their
own table but also for other unrelated tables.
Therefore We should check pg_depend, not pg_policy, to see if dropping this
table affects any policy objects. After collecting the policies impacted by the
ALTER TABLE command, check each policy qual and with check qual, see if
whole-row references or not.
demo:
CREATE TABLE rls_tbl (a int, b int, c int);
CREATE TABLE t (a int);
CREATE POLICY p1 ON rls_tbl USING (rls_tbl >= ROW(1,1,1) and (select t is null from t));
ALTER TABLE t DROP COLUMN a; --error
ERROR: cannot drop column a of table t because other objects depend on it
DETAIL: policy p1 on table rls_tbl depends on column a of table t
HINT: Use DROP ... CASCADE to drop the dependent objects too.
ALTER TABLE rls_tbl DROP COLUMN b; --error
ERROR: cannot drop column b of table rls_tbl because other objects depend on it
DETAIL: policy p1 on table rls_tbl depends on column b of table rls_tbl
HINT: Use DROP ... CASCADE to drop the dependent objects too.
ALTER TABLE rls_tbl ALTER COLUMN b SET DATA TYPE BIGINT; --error
ERROR: cannot alter table "rls_tbl" because security policy "p1" uses its row type
HINT: You might need to drop policy "p1" first
ALTER TABLE t ALTER COLUMN a SET DATA TYPE BIGINT; --error
ERROR: cannot alter table "t" because security policy "p1" uses its row type
HINT: You might need to drop security policy "p1" first
discussion: https://postgr.es/m/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com
---
src/backend/commands/tablecmds.c | 123 ++++++++++++++++++++++
src/backend/optimizer/util/var.c | 59 +++++++++++
src/include/optimizer/optimizer.h | 1 +
src/test/regress/expected/rowsecurity.out | 29 ++++-
src/test/regress/sql/rowsecurity.sql | 17 +++
src/tools/pgindent/typedefs.list | 1 +
6 files changed, 228 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1e28f3af50b..95ed5e8876f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -749,6 +749,8 @@ static void recordWholeRowDependencyOnOrError(Relation rel,
const ObjectAddress *object,
bool error_out);
+static List *GetRelPolicies(Relation rel);
+
/* ----------------------------------------------------------------
* DefineRelation
* Creates a new relation.
@@ -23402,6 +23404,9 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object,
Datum exprDatum;
char *exprString;
bool isnull;
+ List *pols = NIL;
+ Relation pg_policy;
+ Oid reltypid;
bool find_wholerow = false;
TupleConstr *constr = RelationGetDescr(rel)->constr;
@@ -23593,4 +23598,122 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object,
}
}
}
+
+ reltypid = get_rel_type_id(RelationGetRelid(rel));
+
+ pg_policy = table_open(PolicyRelationId, AccessShareLock);
+
+ pols = GetRelPolicies(rel);
+
+ foreach_oid(policyoid, pols)
+ {
+ ObjectAddress pol_obj;
+ SysScanDesc sscan;
+ HeapTuple policy_tuple;
+ ScanKeyData polskey[1];
+
+ find_wholerow = false;
+
+ ScanKeyInit(&polskey[0],
+ Anum_pg_policy_oid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(policyoid));
+ sscan = systable_beginscan(pg_policy,
+ PolicyOidIndexId, true, NULL, 1,
+ polskey);
+ while (HeapTupleIsValid(policy_tuple = systable_getnext(sscan)))
+ {
+ Form_pg_policy policy = (Form_pg_policy) GETSTRUCT(policy_tuple);
+
+ /* Get policy qual */
+ exprDatum = heap_getattr(policy_tuple, Anum_pg_policy_polqual,
+ RelationGetDescr(pg_policy), &isnull);
+ if (!isnull)
+ {
+ exprString = TextDatumGetCString(exprDatum);
+ expr = (Node *) stringToNode(exprString);
+ pfree(exprString);
+
+ find_wholerow = ExprContainWholeRow(expr, reltypid);
+
+ if (find_wholerow)
+ {
+ pol_obj.classId = PolicyRelationId;
+ pol_obj.objectId = policy->oid;
+ pol_obj.objectSubId = 0;
+
+ recordDependencyOnOrError(rel, &pol_obj, object, error_out,
+ DEPENDENCY_NORMAL);
+
+ continue;
+ }
+ }
+
+ exprDatum = heap_getattr(policy_tuple, Anum_pg_policy_polwithcheck,
+ RelationGetDescr(pg_policy), &isnull);
+ if (!isnull)
+ {
+ exprString = TextDatumGetCString(exprDatum);
+ expr = (Node *) stringToNode(exprString);
+ pfree(exprString);
+
+ find_wholerow = ExprContainWholeRow(expr, reltypid);
+
+ if (find_wholerow)
+ {
+ pol_obj.classId = PolicyRelationId;
+ pol_obj.objectId = policy->oid;
+ pol_obj.objectSubId = 0;
+
+ recordDependencyOnOrError(rel, &pol_obj, object, error_out,
+ DEPENDENCY_NORMAL);
+ }
+ }
+ }
+ systable_endscan(sscan);
+ }
+ table_close(pg_policy, AccessShareLock);
+}
+
+static List *
+GetRelPolicies(Relation rel)
+{
+ Relation depRel;
+ ScanKeyData key[3];
+ SysScanDesc scan;
+ HeapTuple depTup;
+ List *result = NIL;
+
+ depRel = table_open(DependRelationId, RowExclusiveLock);
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_refclassid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(RelationRelationId));
+ ScanKeyInit(&key[1],
+ Anum_pg_depend_refobjid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ ScanKeyInit(&key[2],
+ Anum_pg_depend_refobjsubid,
+ BTEqualStrategyNumber, F_INT4EQ,
+ Int32GetDatum((int32) 0));
+
+ scan = systable_beginscan(depRel, DependReferenceIndexId, true,
+ NULL, 3, key);
+ while (HeapTupleIsValid(depTup = systable_getnext(scan)))
+ {
+ Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(depTup);
+ ObjectAddress foundObject;
+
+ foundObject.classId = foundDep->classid;
+ foundObject.objectId = foundDep->objid;
+ foundObject.objectSubId = foundDep->objsubid;
+
+ if (foundObject.classId == PolicyRelationId)
+ result = list_append_unique_oid(result, foundObject.objectId);
+ }
+ systable_endscan(scan);
+ table_close(depRel, NoLock);
+
+ return result;
}
diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 8065237a189..244c866c170 100644
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -49,6 +49,11 @@ typedef struct
int sublevels_up;
} pull_vars_context;
+typedef struct
+{
+ Oid reltypid; /* the whole-row typeid */
+} contain_wholerow_context;
+
typedef struct
{
int var_location;
@@ -73,6 +78,7 @@ typedef struct
static bool pull_varnos_walker(Node *node,
pull_varnos_context *context);
static bool pull_varattnos_walker(Node *node, pull_varattnos_context *context);
+static bool ExprContainWholeRow_walker(Node *node, contain_wholerow_context *context);
static bool pull_vars_walker(Node *node, pull_vars_context *context);
static bool contain_var_clause_walker(Node *node, void *context);
static bool contain_vars_of_level_walker(Node *node, int *sublevels_up);
@@ -327,6 +333,59 @@ pull_varattnos_walker(Node *node, pull_varattnos_context *context)
return expression_tree_walker(node, pull_varattnos_walker, context);
}
+static bool
+ExprContainWholeRow_walker(Node *node, contain_wholerow_context *context)
+{
+ if (node == NULL)
+ return false;
+
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ if (var->varattno == InvalidAttrNumber &&
+ var->vartype == context->reltypid)
+ return true;
+
+ return false;
+ }
+
+ if (IsA(node, Query))
+ {
+ bool result;
+
+ result = query_tree_walker((Query *) node, ExprContainWholeRow_walker,
+ context, 0);
+ return result;
+ }
+
+ return expression_tree_walker(node, ExprContainWholeRow_walker, context);
+}
+
+/*
+ * ExprContainWholeRow -
+ *
+ * Determine whether an expression contains a whole-row Var, recursing as needed.
+ * For simple expressions without sublinks, pull_varattnos is usually sufficient
+ * to detect a whole-row Var. But if the node contains sublinks (unplanned
+ * subqueries), the check must instead rely on the whole-row type OID.
+ * Use ExprContainWholeRow to check whole-row var existsence when in doubt.
+ */
+bool
+ExprContainWholeRow(Node *node, Oid reltypid)
+{
+ contain_wholerow_context context;
+
+ context.reltypid = reltypid;
+
+ Assert(OidIsValid(reltypid));
+
+ return query_or_expression_tree_walker(node,
+ ExprContainWholeRow_walker,
+ &context,
+ 0);
+}
+
/*
* pull_vars_of_level
diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index 44ec5296a18..34b8e7facb7 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -199,6 +199,7 @@ extern SortGroupClause *get_sortgroupref_clause_noerr(Index sortref,
extern Bitmapset *pull_varnos(PlannerInfo *root, Node *node);
extern Bitmapset *pull_varnos_of_level(PlannerInfo *root, Node *node, int levelsup);
extern void pull_varattnos(Node *node, Index varno, Bitmapset **varattnos);
+extern bool ExprContainWholeRow(Node *node, Oid reltypid);
extern List *pull_vars_of_level(Node *node, int levelsup);
extern bool contain_var_clause(Node *node);
extern bool contain_vars_of_level(Node *node, int levelsup);
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index c958ef4d70a..0bb74356fa7 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2637,6 +2637,32 @@ SELECT * FROM document;
14 | 11 | 1 | regress_rls_bob | new novel |
(16 rows)
+--check drop column (no CASCADE) or alter column data type will fail because of
+--whole-row referenced security policy exists.
+DROP TABLE part_document;
+ALTER TABLE document ADD COLUMN dummy INT4, ADD COLUMN dummy1 INT4;
+CREATE POLICY p7 ON document AS PERMISSIVE
+ USING (cid IS NOT NULL AND
+ (WITH cte AS (SELECT TRUE FROM uaccount
+ WHERE EXISTS (SELECT document FROM uaccount WHERE uaccount IS NULL))
+ SELECT * FROM cte));
+ALTER TABLE uaccount ALTER COLUMN seclv SET DATA TYPE BIGINT; --errror
+ERROR: cannot alter table "uaccount" because policy p7 on table document uses its row type
+HINT: You might need to drop policy p7 on table document first
+ALTER TABLE document ALTER COLUMN dummy SET DATA TYPE BIGINT; --error
+ERROR: cannot alter table "document" because policy p7 on table document uses its row type
+HINT: You might need to drop policy p7 on table document first
+ALTER TABLE document DROP COLUMN dummy; --error
+ERROR: cannot drop column dummy of table document because other objects depend on it
+DETAIL: policy p7 on table document depends on column dummy of table document
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+ALTER TABLE uaccount DROP COLUMN seclv; --error
+ERROR: cannot drop column seclv of table uaccount because other objects depend on it
+DETAIL: policy p7 on table document depends on column seclv of table uaccount
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+ALTER TABLE document DROP COLUMN dummy CASCADE; --ok
+NOTICE: drop cascades to policy p7 on table document
+ALTER TABLE uaccount DROP COLUMN seclv CASCADE; --ok
--
-- ROLE/GROUP
--
@@ -5105,12 +5131,11 @@ 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 29 other objects
DETAIL: drop cascades to function f_leak(text)
drop cascades to table uaccount
drop cascades to table category
drop cascades to table document
-drop cascades to table part_document
drop cascades to table dependent
drop cascades to table rec1
drop cascades to table rec2
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 5d923c5ca3b..76487b5e4ba 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -1163,6 +1163,23 @@ DROP POLICY p1 ON document;
-- Just check everything went per plan
SELECT * FROM document;
+--check drop column (no CASCADE) or alter column data type will fail because of
+--whole-row referenced security policy exists.
+DROP TABLE part_document;
+ALTER TABLE document ADD COLUMN dummy INT4, ADD COLUMN dummy1 INT4;
+CREATE POLICY p7 ON document AS PERMISSIVE
+ USING (cid IS NOT NULL AND
+ (WITH cte AS (SELECT TRUE FROM uaccount
+ WHERE EXISTS (SELECT document FROM uaccount WHERE uaccount IS NULL))
+ SELECT * FROM cte));
+ALTER TABLE uaccount ALTER COLUMN seclv SET DATA TYPE BIGINT; --errror
+ALTER TABLE document ALTER COLUMN dummy SET DATA TYPE BIGINT; --error
+
+ALTER TABLE document DROP COLUMN dummy; --error
+ALTER TABLE uaccount DROP COLUMN seclv; --error
+ALTER TABLE document DROP COLUMN dummy CASCADE; --ok
+ALTER TABLE uaccount DROP COLUMN seclv CASCADE; --ok
+
--
-- ROLE/GROUP
--
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 5c88fa92f4e..a6118cc3f32 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3575,6 +3575,7 @@ conn_oauth_scope_func
conn_sasl_state_func
contain_aggs_of_level_context
contain_placeholder_references_context
+contain_wholerow_context
convert_testexpr_context
copy_data_dest_cb
copy_data_source_cb
--
2.34.1