v9-0003-fix-DDL-wholerow-referenced-policies.patch
text/x-patch
Filename: v9-0003-fix-DDL-wholerow-referenced-policies.patch
Type: text/x-patch
Part: 1
From 575265b67813e860a382971d5e62fae9c745ba72 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Sat, 23 May 2026 21:40:23 +0800
Subject: [PATCH v9 3/3] fix DDL wholerow referenced policies
ALTER TABLE DROP COLUMN must fail if any policy's WHEN clause contains a
whole-row reference. To achieve this, we record a dependency between the
relation and the dependent policy in RememberWholeRowDependentForRebuilding.
Later, performMultipleDeletions handles the deletion logic.
ALTER COLUMN SET DATA TYPE fundamentally changes the table's row type. Rows
with different column data types cannot be compared (see record_eq). Therefore,
we must error out otherwise, any policy WHEN clause contains whole-row
references may constantly error.
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.
discussion: https://postgr.es/m/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com
commitfest: https://commitfest.postgresql.org/patch/6055
---
src/backend/commands/tablecmds.c | 130 ++++++++++++++++++++++
src/backend/optimizer/util/var.c | 56 ++++++++++
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, 232 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index bd8e29de1b1..ed30c372239 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -790,6 +790,8 @@ static void RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterT
Relation rel, AttrNumber attnum,
const char *colName);
+static List *GetAllRelAssociatedPolicies(Relation rel);
+
/* ----------------------------------------------------------------
* DefineRelation
* Creates a new relation.
@@ -23976,6 +23978,7 @@ RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType sub
ScanKeyData skey;
Relation pg_index;
Relation pg_constraint;
+ Relation pg_policy;
SysScanDesc indscan;
SysScanDesc conscan;
HeapTuple constrTuple;
@@ -23984,6 +23987,8 @@ RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType sub
char *exprString = NULL;
bool isnull;
List *wholerow_idxoids = NIL;
+ List *pols = NIL;
+ Oid reltypid;
Assert(subtype == AT_AlterColumnType || subtype == AT_DropColumn);
@@ -24194,4 +24199,129 @@ RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType sub
}
}
}
+
+ /* Now checking policy whole-row references */
+ reltypid = get_rel_type_id(RelationGetRelid(rel));
+
+ pg_policy = table_open(PolicyRelationId, AccessShareLock);
+
+ pols = GetAllRelAssociatedPolicies(rel);
+
+ foreach_oid(policyoid, pols)
+ {
+ SysScanDesc sscan;
+ HeapTuple policy_tuple;
+ ScanKeyData polskey[1];
+
+ 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);
+
+ exprDatum = heap_getattr(policy_tuple, Anum_pg_policy_polqual,
+ RelationGetDescr(pg_policy),
+ &isnull);
+ if (!isnull)
+ {
+ exprString = TextDatumGetCString(exprDatum);
+ expr = (Node *) stringToNode(exprString);
+ pfree(exprString);
+
+ if (expr_contain_wholerow(expr, reltypid))
+ {
+ ObjectAddress pol_obj;
+
+ ObjectAddressSet(pol_obj, PolicyRelationId, policy->oid);
+
+ /*
+ * The dependency between the policy and it's relation is
+ * DEPENDENCY_NORMAL
+ */
+ RememberWholeRowDependent(tab, subtype, rel, attnum,
+ &pol_obj, 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);
+
+ if (expr_contain_wholerow(expr, reltypid))
+ {
+ ObjectAddress pol_obj;
+
+ ObjectAddressSet(pol_obj, PolicyRelationId, policy->oid);
+
+ /*
+ * The dependency between the policy and it's relation is
+ * DEPENDENCY_NORMAL
+ */
+ RememberWholeRowDependent(tab, subtype, rel, attnum,
+ &pol_obj, DEPENDENCY_NORMAL);
+ }
+ }
+ }
+ systable_endscan(sscan);
+ }
+ table_close(pg_policy, AccessShareLock);
+}
+
+/*
+ * GetAllRelAssociatedPolicies
+ *
+ * Returns a list of OIDs of all row-level security policies associated with the
+ * given relation.
+ */
+static List *
+GetAllRelAssociatedPolicies(Relation rel)
+{
+ Relation depRel;
+ ScanKeyData key[3];
+ SysScanDesc scan;
+ HeapTuple depTup;
+ List *result = NIL;
+
+ depRel = table_open(DependRelationId, AccessShareLock);
+ 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);
+
+ if (foundDep->classid == PolicyRelationId)
+ result = list_append_unique_oid(result, foundDep->objid);
+ }
+ systable_endscan(scan);
+ table_close(depRel, AccessShareLock);
+
+ return result;
}
diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 907a255c36f..75f19271974 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 expr_contain_wholerow_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,56 @@ pull_varattnos_walker(Node *node, pull_varattnos_context *context)
return expression_tree_walker(node, pull_varattnos_walker, context);
}
+static bool
+expr_contain_wholerow_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))
+ return query_tree_walker((Query *) node, expr_contain_wholerow_walker,
+ context, 0);
+
+ return expression_tree_walker(node, expr_contain_wholerow_walker, context);
+}
+
+/*
+ * expr_contain_wholerow -
+ *
+ * Determine whether an expression contains whole-row Var reference, 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 expr_contain_wholerow to check whole-row var existsence when in doubt.
+ */
+bool
+expr_contain_wholerow(Node *node, Oid reltypid)
+{
+ contain_wholerow_context context;
+
+ context.reltypid = reltypid;
+
+ Assert(OidIsValid(reltypid));
+
+ return query_or_expression_tree_walker(node,
+ expr_contain_wholerow_walker,
+ &context,
+ 0);
+}
+
+
/*
* pull_vars_of_level
diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index cb6241e2bdd..0491059c7cc 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -206,6 +206,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 expr_contain_wholerow(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 3a5e82c35bd..6b6fe994033 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2731,6 +2731,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; -- error
+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
--
@@ -5199,12 +5225,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 6b3566271df..32f2dda73a4 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -1219,6 +1219,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; -- error
+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 8cf40c87043..9236525b44d 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3692,6 +3692,7 @@ config_handle
config_var_value
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