Hi, I have consolidated the work into two patches. 0001 handles indexes and CHECK constraints that contain whole-row references. 0002 handles policies that contain whole-row references.
The difference is that, for policy objects, we cannot use pull_varattnos to find whole-row references, since we need recurse to Sublink node, Also, a policy’s whole-row reference may point to an arbitrary relation, while index, check constraint can only reference the relation it is associated with. so the previous v5-0003 scans pg_policy.polrelid to find out whether it's safe to drop one relation is wrong, we should use pg_depend. summary: For objects (indexes, constraints, policies) that contain whole-row references: ALTER TABLE DROP COLUMN will drop these objects too. ALTER COLUMN SET DATA TYPE will error out, saying that the data type cannot be changed because whole-row–dependent objects exist. -- jian https://www.enterprisedb.com/
From 63bcfe1baad72a166f957c1b6ed70d88f6b469cd Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Mon, 22 Dec 2025 23:28:00 +0800 Subject: [PATCH v6 1/2] fix DDL wholerow referenced constraints and indexes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. ALTER TABLE DROP COLUMN. ALTER TABLE DROP COLUMN should remove indexes or constraints contain whole-row expression. We enumerate all constraints and indexes and check one by one to determine whether it contains a whole-row Var reference. If such a reference is found, we record the dependency and later performMultipleDeletions will do the job. for example: CREATE TABLE ts (a int, constraint cc check((ts = ROW(1)))); CREATE INDEX tsi3 on ts ((ts is null)); ALTER TABLE DROP COLUMN should drop above all indexes, constraints on table ts. 2. ALTER COLUMN SET DATA TYPE ALTER COLUMN SET DATA TYPE should error out when whole-row referenced object (index, constraints, etc) exists. ALTER COLUMN SET DATA TYPE fundamentally changes the table’s record type; At present, we cannot compare records that contain columns of dissimilar types, see function record_eq. As a result, ALTER COLUMN SET DATA TYPE does not work for whole-row reference objects (such as constraints and indexes), and must therefore raise an error. For example, below ALTER COLUMN SET DATA TYPE should fail. CREATE TABLE ts (a int, CONSTRAINT cc CHECK ((ts = ROW(1)))); CREATE INDEX ON ts ((ts IS NOT NULL)); ALTER TABLE ts ALTER COLUMN a SET DATA TYPE int8; discussion: https://postgr.es/m/cacjufxga6kvqy7dbhglvw9s9kkmpgyzt5me6c7kefjdpr2w...@mail.gmail.com --- src/backend/commands/tablecmds.c | 248 +++++++++++++++++++++- src/test/regress/expected/constraints.out | 26 +++ src/test/regress/expected/indexing.out | 22 ++ src/test/regress/sql/constraints.sql | 21 ++ src/test/regress/sql/indexing.sql | 13 ++ 5 files changed, 327 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6b1a00ed477..a541b795d48 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -745,6 +745,9 @@ static void ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation static void ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, PartitionCmd *cmd, AlterTableUtilityContext *context); +static void recordWholeRowDependencyOnOrError(Relation rel, + const ObjectAddress *object, + bool error_out); /* ---------------------------------------------------------------- * DefineRelation @@ -9393,6 +9396,21 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, ReleaseSysCache(tuple); + object.classId = RelationRelationId; + object.objectId = RelationGetRelid(rel); + object.objectSubId = attnum; + + /* + * We should also remove indexes or constraints that contain whole-row + * expression. Using recordWholeRowDependencyOnOrError to establish a + * dependency between the column and any constraint or index involving + * whole-row Vars. performMultipleDeletions will then take care of + * removing them later. + */ + recordWholeRowDependencyOnOrError(rel, &object, false); + + CommandCounterIncrement(); + /* * Propagate to children as appropriate. Unlike most other ALTER * routines, we have to do this one level of recursion at a time; we can't @@ -9486,9 +9504,6 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, } /* Add object to delete */ - object.classId = RelationRelationId; - object.objectId = RelationGetRelid(rel); - object.objectSubId = attnum; add_exact_object_address(&object, addrs); if (!recursing) @@ -14773,6 +14788,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, SysScanDesc scan; HeapTuple depTup; ObjectAddress address; + ObjectAddress object; /* * Clear all the missing values if we're rewriting the table, since this @@ -14868,6 +14884,16 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ RememberAllDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName); + object.classId = RelationRelationId; + object.objectId = RelationGetRelid(rel); + object.objectSubId = attnum; + + /* + * Check for whole-row referenced objects (constraints, indexes etc) -- + * can't cope + */ + recordWholeRowDependencyOnOrError(rel, &object, true); + /* * Now scan for dependencies of this column on other things. The only * things we should find are the dependency on the column datatype and @@ -23337,3 +23363,219 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Restore the userid and security context. */ SetUserIdAndSecContext(save_userid, save_sec_context); } + +/* + * Record dependencies between whole-row objects (indexes, CHECK constraints) + * and the relation's ObjectAddress. + * + * error_out means can not install such dependency, error out explicitly. + */ +static void +recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, bool error_out) +{ + Node *expr = NULL; + ScanKeyData skey; + Relation pg_index; + SysScanDesc indscan; + HeapTuple htup; + ObjectAddress idx_obj; + HeapTuple indexTuple; + Form_pg_index indexStruct; + List *indexlist = NIL; + Bitmapset *expr_attrs = NULL; + Datum exprDatum; + char *exprString; + bool isnull; + bool find_wholerow = false; + TupleConstr *constr = RelationGetDescr(rel)->constr; + + /* + * Loop through each CHECK constraint, see if it contain whole-row + * references or not + */ + if (constr && constr->num_check > 0) + { + Relation pg_constraint; + SysScanDesc conscan; + ObjectAddress con_obj; + + pg_constraint = table_open(ConstraintRelationId, AccessShareLock); + + /* Search pg_constraint for relevant entries */ + ScanKeyInit(&skey, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + + conscan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true, + NULL, 1, &skey); + while (HeapTupleIsValid(htup = systable_getnext(conscan))) + { + Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(htup); + + if (conform->contype != CONSTRAINT_CHECK) + continue; + + /* Grab and test conbin is actually set */ + exprDatum = fastgetattr(htup, + Anum_pg_constraint_conbin, + RelationGetDescr(pg_constraint), &isnull); + if (isnull) + elog(WARNING, "null conbin for relation \"%s\"", + RelationGetRelationName(rel)); + else + { + char *s = TextDatumGetCString(exprDatum); + + expr = stringToNode(s); + pfree(s); + + /* Find all attributes referenced */ + pull_varattnos(expr, 1, &expr_attrs); + + find_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, + expr_attrs); + + if (find_wholerow) + { + if (error_out) + ereport(ERROR, + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("cannot alter table \"%s\" because constraint \"%s\" uses its row type", + RelationGetRelationName(rel), + NameStr(conform->conname)), + errhint("You might need to drop constraint \"%s\" first", + NameStr(conform->conname))); + else + { + con_obj.classId = ConstraintRelationId; + con_obj.objectId = conform->oid; + con_obj.objectSubId = 0; + + /* + * record dependency for constraints that references + * whole-row + */ + recordDependencyOn(&con_obj, object, DEPENDENCY_AUTO); + } + } + } + } + systable_endscan(conscan); + table_close(pg_constraint, AccessShareLock); + } + + /* now checking indexes contain whole-row references or not */ + /* Prepare to scan pg_index for entries having indrelid = this rel */ + ScanKeyInit(&skey, + Anum_pg_index_indrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + + pg_index = table_open(IndexRelationId, AccessShareLock); + + indscan = systable_beginscan(pg_index, IndexIndrelidIndexId, true, + NULL, 1, &skey); + while (HeapTupleIsValid(htup = systable_getnext(indscan))) + { + Form_pg_index index = (Form_pg_index) GETSTRUCT(htup); + + /* add index's OID to result list */ + indexlist = lappend_oid(indexlist, index->indexrelid); + } + systable_endscan(indscan); + + table_close(pg_index, AccessShareLock); + + foreach_oid(indexoid, indexlist) + { + indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexoid)); + if (!HeapTupleIsValid(indexTuple)) + elog(ERROR, "cache lookup failed for index %u", indexoid); + indexStruct = (Form_pg_index) GETSTRUCT(indexTuple); + + if (!heap_attisnull(indexTuple, Anum_pg_index_indexprs, NULL)) + { + expr_attrs = NULL; + + /* Convert text string to node tree */ + exprDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple, + Anum_pg_index_indexprs); + exprString = TextDatumGetCString(exprDatum); + expr = (Node *) stringToNode(exprString); + pfree(exprString); + + pull_varattnos(expr, 1, &expr_attrs); + + find_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, + expr_attrs); + + if (find_wholerow) + { + if (error_out) + ereport(ERROR, + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("cannot alter table \"%s\" because index \"%s\" uses its row type", + RelationGetRelationName(rel), + get_rel_name(indexStruct->indexrelid)), + errhint("You might need to drop index \"%s\" first", + get_rel_name(indexStruct->indexrelid))); + else + { + idx_obj.classId = RelationRelationId; + idx_obj.objectId = indexStruct->indexrelid; + idx_obj.objectSubId = 0; + + /* record dependency for indexes that references whole-row */ + recordDependencyOn(&idx_obj, object, DEPENDENCY_AUTO); + + ReleaseSysCache(indexTuple); + + continue; + } + } + } + + if (!heap_attisnull(indexTuple, Anum_pg_index_indpred, NULL)) + { + expr_attrs = NULL; + + /* Convert text string to node tree */ + exprDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple, + Anum_pg_index_indpred); + exprString = TextDatumGetCString(exprDatum); + expr = (Node *) stringToNode(exprString); + pfree(exprString); + + pull_varattnos(expr, 1, &expr_attrs); + + find_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, + expr_attrs); + if (find_wholerow) + { + if (error_out) + ereport(ERROR, + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("cannot alter table \"%s\" because index \"%s\" uses its row type", + RelationGetRelationName(rel), + get_rel_name(indexStruct->indexrelid)), + errhint("You might need to drop index \"%s\" first", + get_rel_name(indexStruct->indexrelid))); + else + { + idx_obj.classId = RelationRelationId; + idx_obj.objectId = indexStruct->indexrelid; + idx_obj.objectSubId = 0; + + /* record dependency for indexes that references whole-row */ + recordDependencyOn(&idx_obj, object, DEPENDENCY_AUTO); + + ReleaseSysCache(indexTuple); + + continue; + } + } + } + ReleaseSysCache(indexTuple); + } +} diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index 1bbf59cca02..6e26e9fbcfc 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -254,6 +254,32 @@ ERROR: system column "ctid" reference in check constraint is invalid LINE 3: CHECK (NOT (is_capital AND ctid::text = 'sys_col_check... ^ -- +-- Drop column also drop check constraints that have whole-row reference +-- +CREATE TABLE DROP_COL_CHECK_TBL ( + city TEXT, state TEXT, + CONSTRAINT cc0 CHECK (DROP_COL_CHECK_TBL is null) NOT ENFORCED, + CONSTRAINT cc1 CHECK (DROP_COL_CHECK_TBL is not null) NOT ENFORCED); +ALTER TABLE DROP_COL_CHECK_TBL DROP COLUMN city; +\d DROP_COL_CHECK_TBL + Table "public.drop_col_check_tbl" + Column | Type | Collation | Nullable | Default +--------+------+-----------+----------+--------- + state | text | | | + +DROP TABLE DROP_COL_CHECK_TBL; +-- +-- Change column data type should error out because the whole-row referenced +-- check constraint is still need it. +-- +CREATE TABLE ALTER_COL_CHECK_TBL ( + city INT, state TEXT, + CONSTRAINT cc1 CHECK (ALTER_COL_CHECK_TBL is not null) NOT ENFORCED); +ALTER TABLE ALTER_COL_CHECK_TBL ALTER COLUMN city SET DATA TYPE int8; +ERROR: cannot alter table "alter_col_check_tbl" because constraint "cc1" uses its row type +HINT: You might need to drop constraint "cc1" first +DROP TABLE ALTER_COL_CHECK_TBL; +-- -- Check inheritance of defaults and constraints -- CREATE TABLE INSERT_CHILD (cx INT default 42, diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index 4d29fb85293..4b683c679a8 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -654,6 +654,28 @@ alter table idxpart2 drop column c; b | integer | | | drop table idxpart, idxpart2; +create table idxpart (a int, b int, c int); +create index on idxpart(c); +create index idxpart_idx1 on idxpart((idxpart is not null)); +create index idxpart_idx2 on idxpart(a) where idxpart is not null; +alter table idxpart drop column c; +\d idxpart + Table "public.idxpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | + +create index idxpart_idx1 on idxpart((idxpart is not null)); +create index idxpart_idx2 on idxpart(a) where idxpart is not null; +alter table idxpart alter column a set data type int8; --error +ERROR: cannot alter table "idxpart" because index "idxpart_idx1" uses its row type +HINT: You might need to drop index "idxpart_idx1" first +drop index idxpart_idx1; +alter table idxpart alter column b set data type int8; --error +ERROR: cannot alter table "idxpart" because index "idxpart_idx2" uses its row type +HINT: You might need to drop index "idxpart_idx2" first +drop table idxpart; -- Verify that expression indexes inherit correctly create table idxpart (a int, b int) partition by range (a); create table idxpart1 (like idxpart); diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index 733a1dbccfe..c7ae3fb6641 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -165,6 +165,27 @@ CREATE TABLE SYS_COL_CHECK_TBL (city text, state text, is_capital bool, altitude int, CHECK (NOT (is_capital AND ctid::text = 'sys_col_check_tbl'))); +-- +-- Drop column also drop check constraints that have whole-row reference +-- +CREATE TABLE DROP_COL_CHECK_TBL ( + city TEXT, state TEXT, + CONSTRAINT cc0 CHECK (DROP_COL_CHECK_TBL is null) NOT ENFORCED, + CONSTRAINT cc1 CHECK (DROP_COL_CHECK_TBL is not null) NOT ENFORCED); +ALTER TABLE DROP_COL_CHECK_TBL DROP COLUMN city; +\d DROP_COL_CHECK_TBL +DROP TABLE DROP_COL_CHECK_TBL; + +-- +-- Change column data type should error out because the whole-row referenced +-- check constraint is still need it. +-- +CREATE TABLE ALTER_COL_CHECK_TBL ( + city INT, state TEXT, + CONSTRAINT cc1 CHECK (ALTER_COL_CHECK_TBL is not null) NOT ENFORCED); +ALTER TABLE ALTER_COL_CHECK_TBL ALTER COLUMN city SET DATA TYPE int8; +DROP TABLE ALTER_COL_CHECK_TBL; + -- -- Check inheritance of defaults and constraints -- diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index b5cb01c2d70..b7c08c9ff5a 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -295,6 +295,19 @@ alter table idxpart2 drop column c; \d idxpart2 drop table idxpart, idxpart2; +create table idxpart (a int, b int, c int); +create index on idxpart(c); +create index idxpart_idx1 on idxpart((idxpart is not null)); +create index idxpart_idx2 on idxpart(a) where idxpart is not null; +alter table idxpart drop column c; +\d idxpart +create index idxpart_idx1 on idxpart((idxpart is not null)); +create index idxpart_idx2 on idxpart(a) where idxpart is not null; +alter table idxpart alter column a set data type int8; --error +drop index idxpart_idx1; +alter table idxpart alter column b set data type int8; --error +drop table idxpart; + -- Verify that expression indexes inherit correctly create table idxpart (a int, b int) partition by range (a); create table idxpart1 (like idxpart); -- 2.34.1
From 96c1fb6ca674368d4d9aea0980a6af1658abd5cd Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Mon, 22 Dec 2025 23:25:07 +0800 Subject: [PATCH v6 2/2] 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/cacjufxga6kvqy7dbhglvw9s9kkmpgyzt5me6c7kefjdpr2w...@mail.gmail.com --- src/backend/commands/tablecmds.c | 147 +++++++++++++++++++++- 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, 251 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a541b795d48..6fdacab7c66 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 *GetAllPoliciesRelations(Relation rel); + /* ---------------------------------------------------------------- * DefineRelation * Creates a new relation. @@ -23365,7 +23367,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, } /* - * Record dependencies between whole-row objects (indexes, CHECK constraints) + * Record dependencies between whole-row objects (indexes, CHECK constraints, policies) * and the relation's ObjectAddress. * * error_out means can not install such dependency, error out explicitly. @@ -23386,6 +23388,9 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo Datum exprDatum; char *exprString; bool isnull; + List *pols = NIL; + Relation pg_policy; + Oid reltypid; bool find_wholerow = false; TupleConstr *constr = RelationGetDescr(rel)->constr; @@ -23578,4 +23583,144 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo } ReleaseSysCache(indexTuple); } + + reltypid = get_rel_type_id(RelationGetRelid(rel)); + + pg_policy = table_open(PolicyRelationId, AccessShareLock); + + pols = GetAllPoliciesRelations(rel); + + foreach_oid(policyoid, pols) + { + ObjectAddress pol_obj; + 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); + + /* 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 && !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 security policy \"%s\" first", + NameStr(policy->polname))); + } + + 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 && !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 security policy \"%s\" first", + NameStr(policy->polname))); + } + } + systable_endscan(sscan); + } + table_close(pg_policy, AccessShareLock); +} + +static List * +GetAllPoliciesRelations(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..1eff6e55ef3 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 security policy "p7" uses its row type +HINT: You might need to drop security policy "p7" first +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 security 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 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 04845d5e680..1fc231b0c60 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3574,6 +3574,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
