From 5d3ad72059977c6c3576c92e1ba25684f80b628d Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 15 Sep 2025 20:19:29 +0800 Subject: [PATCH v4 3/3] disallow change or drop column when wholerow referenced policy exists demo: CREATE TABLE rls_tbl (a int, b int, c int); CREATE POLICY p1 ON rls_tbl USING (rls_tbl >= ROW(1,1,1)); 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 rls_tbl DROP COLUMN b CASCADE; --ok NOTICE: drop cascades to policy p1 on table rls_tbl ALTER TABLE discussion: https://postgr.es/m/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com --- src/backend/commands/tablecmds.c | 91 ++++++++++++++++++++++- src/backend/optimizer/util/var.c | 60 +++++++++++++++ src/include/optimizer/optimizer.h | 1 + src/test/regress/expected/rowsecurity.out | 17 +++++ src/test/regress/sql/rowsecurity.sql | 12 +++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 181 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4949d7204a0..403b8a1adb8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -22096,7 +22096,7 @@ GetAttributeStorage(Oid atttypid, const char *storagemode) } /* - * Record dependencies between whole-row objects (indexes, constraints) + * Record dependencies between whole-row objects (indexes, constraints or policies) * associated with relation and relation's ObjectAddress. * error_out means can not install such dependency, we have to error out explicitly. */ @@ -22107,9 +22107,15 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo List *indexlist = NIL; ObjectAddress con_obj; ObjectAddress idx_obj; + ObjectAddress pol_obj; bool find_wholerow = false; TupleConstr *constr = RelationGetDescr(rel)->constr; + Relation pg_policy; + ScanKeyData skey[1]; + SysScanDesc sscan; + HeapTuple policy_tuple; + Oid reltypid; if (constr && constr->num_check > 0) { @@ -22265,4 +22271,87 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo } ReleaseSysCache(indexTuple); } + + find_wholerow = false; + reltypid = get_rel_type_id(RelationGetRelid(rel)); + + pg_policy = table_open(PolicyRelationId, AccessShareLock); + ScanKeyInit(&skey[0], + Anum_pg_policy_polrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + sscan = systable_beginscan(pg_policy, + PolicyPolrelidPolnameIndexId, true, NULL, 1, + skey); + while (HeapTupleIsValid(policy_tuple = systable_getnext(sscan))) + { + Datum datum; + bool isnull; + char *str_value; + Node *polexpr; + + Form_pg_policy policy = (Form_pg_policy) GETSTRUCT(policy_tuple); + + /* Get policy qual */ + datum = heap_getattr(policy_tuple, Anum_pg_policy_polqual, + RelationGetDescr(pg_policy), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + polexpr = (Node *) stringToNode(str_value); + pfree(str_value); + + find_wholerow = ExprContainWholeRow(polexpr, reltypid); + if (find_wholerow && !error_out) + { + pol_obj.classId = PolicyRelationId; + pol_obj.objectId = policy->oid; + pol_obj.objectSubId = 0; + + /* record dependency for policies that references whole-row Var */ + recordDependencyOn(&pol_obj, object, DEPENDENCY_NORMAL); + continue; + } + + if (find_wholerow && error_out) + ereport(ERROR, + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("cannot alter table \"%s\" because security policy \"%s\" uses its row type", + RelationGetRelationName(rel), + NameStr(policy->polname)), + errhint("You might need to drop policy \"%s\" first", + NameStr(policy->polname))); + } + + datum = heap_getattr(policy_tuple, Anum_pg_policy_polwithcheck, + RelationGetDescr(pg_policy), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + polexpr = (Node *) stringToNode(str_value); + pfree(str_value); + + find_wholerow = ExprContainWholeRow(polexpr, reltypid); + if (find_wholerow && !error_out) + { + pol_obj.classId = PolicyRelationId; + pol_obj.objectId = policy->oid; + pol_obj.objectSubId = 0; + + /* record dependency for policies that references whole-row Var */ + recordDependencyOn(&pol_obj, object, DEPENDENCY_NORMAL); + } + + if (find_wholerow && error_out) + ereport(ERROR, + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("cannot alter table \"%s\" because security policy \"%s\" uses its row type", + RelationGetRelationName(rel), + NameStr(policy->polname)), + errhint("You might need to drop policy \"%s\" first", + NameStr(policy->polname))); + } + } + systable_endscan(sscan); + table_close(pg_policy, AccessShareLock); } diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c index 8065237a189..36de0a3bfca 100644 --- a/src/backend/optimizer/util/var.c +++ b/src/backend/optimizer/util/var.c @@ -48,6 +48,10 @@ typedef struct List *vars; int sublevels_up; } pull_vars_context; +typedef struct +{ + Oid reltypid; +} contain_wholerow_context; typedef struct { @@ -73,6 +77,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 +332,61 @@ 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)) + { + /* Recurse into RTE subquery or not-yet-planned sublink subquery */ + 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. If the node contains sublinks (unplanned subqueries), + * the check must instead rely on the whole-row type OID. Therefore, reltypid is + * used consistently to determine the presence of a whole-row Var. + */ +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 37bc13c2cbd..d31d1c49c75 100644 --- a/src/include/optimizer/optimizer.h +++ b/src/include/optimizer/optimizer.h @@ -205,6 +205,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 8c879509313..b2b5333f8dc 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -2357,6 +2357,23 @@ SELECT * FROM document; 14 | 11 | 1 | regress_rls_bob | new novel | (16 rows) +--check drop column restrict or alter column data type will fail because of whole-row +--referenced security policy exists. +ALTER TABLE document ADD COLUMN dummy 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 document ALTER COLUMN dummy SET DATA TYPE BIGINT; --error +ERROR: cannot alter table "document" because security policy "p7" uses its row type +HINT: You might need to drop policy "p7" 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 document DROP COLUMN dummy CASCADE; --ok +NOTICE: drop cascades to policy p7 on table document -- -- ROLE/GROUP -- diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 21ac0ca51ee..0cdd5a82ca3 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -1021,6 +1021,18 @@ DROP POLICY p1 ON document; -- Just check everything went per plan SELECT * FROM document; +--check drop column restrict or alter column data type will fail because of whole-row +--referenced security policy exists. +ALTER TABLE document ADD COLUMN dummy 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 document ALTER COLUMN dummy SET DATA TYPE BIGINT; --error +ALTER TABLE document DROP COLUMN dummy; --error +ALTER TABLE document DROP COLUMN dummy CASCADE; --ok + -- -- ROLE/GROUP -- diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index a13e8162890..81b97cc622e 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3521,6 +3521,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