From 0bfe5dd6cc313eac75a81aaf122ef090fdd26a98 Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 9 Sep 2025 10:54:56 +0800 Subject: [PATCH v2 1/1] ALTER TABLE DROP COLUMN drop wholerow referenced object CREATE TABLE ts (a int, c int, b int constraint cc check((ts = ROW(1,1,1))), constraint cc1 check((ts.a = 1))); CREATE INDEX tsi on ts (a) where a = 1; CREATE INDEX tsi2 on ts ((a is null)); CREATE INDEX tsi3 on ts ((ts is null)); CREATE INDEX tsi4 on ts (b) where ts is not null; CREATE POLICY p1 ON ts USING (ts >= ROW(1,1,1)); CREATE POLICY p2 ON ts USING (ts.a = 1); ALTER TABLE ts DROP COLUMN a CASCADE; will drop above all indexes, constraints and policies on the table ts. now \d ts Table "public.ts" Column | Type | Collation | Nullable | Default --------+---------+-----------+----------+--------- c | integer | | | b | integer | | | discussion: https://postgr.es/m/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com --- src/backend/commands/tablecmds.c | 206 ++++++++++++++++++++++ src/test/regress/expected/constraints.out | 17 ++ src/test/regress/expected/indexing.out | 25 +++ src/test/regress/expected/rowsecurity.out | 21 +++ src/test/regress/sql/constraints.sql | 11 ++ src/test/regress/sql/indexing.sql | 9 + src/test/regress/sql/rowsecurity.sql | 9 + 7 files changed, 298 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 082a3575d62..b21f20ca8fd 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9256,7 +9256,15 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, AttrNumber attnum; List *children; ObjectAddress object; + ObjectAddress tmpobject; bool is_expr; + Node *expr; + List *indexlist = NIL; + TupleConstr *constr = RelationGetDescr(rel)->constr; + Relation pg_policy; + ScanKeyData skey[1]; + SysScanDesc sscan; + HeapTuple policy_tuple; /* At top level, permission check was done in ATPrepCmd, else do it */ if (recursing) @@ -9329,6 +9337,204 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, ReleaseSysCache(tuple); + tmpobject.classId = RelationRelationId; + tmpobject.objectId = RelationGetRelid(rel); + tmpobject.objectSubId = attnum; + + /* + * ALTER TABLE DROP COLUMN also need drop indexes or constraints that + * include whole-row reference expressions. However, there is no direct + * dependency between whole-row Var references and individual table columns, + * and adding such dependencies would lead to catalog bloat (see + * find_expr_references_walker). + * Therefore, here, we explicitly use recordDependencyOn to create a + * dependency between the table column and any constraint or index that + * contains whole-row Var references. + * Later performMultipleDeletions will do the job. + */ + if (constr && constr->num_check > 0) + { + ConstrCheck *check = constr->check; + + for (int i = 0; i < constr->num_check; i++) + { + Bitmapset *expr_attrs = NULL; + char *constr_name = check[i].ccname; + + expr = stringToNode(check[i].ccbin); + + /* Find all attributes referenced */ + pull_varattnos(expr, 1, &expr_attrs); + + if (bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, expr_attrs)) + { + Relation conDesc; + SysScanDesc conscan; + ScanKeyData skey[3]; + HeapTuple contuple; + + /* Search for a pg_constraint entry with same name and relation */ + conDesc = table_open(ConstraintRelationId, AccessShareLock); + + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(constr_name)); + + conscan = systable_beginscan(conDesc, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); + if (!HeapTupleIsValid(contuple = systable_getnext(conscan))) + elog(ERROR, "constraint \"%s\" of relation \"%s\" does not exist", + constr_name, RelationGetRelationName(rel)); + + object.classId = ConstraintRelationId; + object.objectId = ((Form_pg_constraint) GETSTRUCT(contuple))->oid; + object.objectSubId = 0; + + /* record dependency for constraints that references whole-row */ + recordDependencyOn(&object, &tmpobject, DEPENDENCY_AUTO); + + systable_endscan(conscan); + table_close(conDesc, AccessShareLock); + } + } + } + + indexlist = RelationGetIndexList(rel); + foreach_oid(indexoid, indexlist) + { + HeapTuple indexTuple; + Form_pg_index indexStruct; + Node *node; + bool found_whole_row = false; + + 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_indpred, NULL)) + { + Datum predDatum; + char *predString; + Bitmapset *expr_attrs = NULL; + + /* Convert text string to node tree */ + predDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple, + Anum_pg_index_indpred); + predString = TextDatumGetCString(predDatum); + node = (Node *) stringToNode(predString); + pfree(predString); + + pull_varattnos(node, 1, &expr_attrs); + if (bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, expr_attrs)) + { + object.classId = RelationRelationId; + object.objectId = indexStruct->indexrelid; + object.objectSubId = 0; + /* record dependency for indexes that references whole-row */ + recordDependencyOn(&object, &tmpobject, DEPENDENCY_AUTO); + found_whole_row = true; + } + } + + if (!found_whole_row && !heap_attisnull(indexTuple, Anum_pg_index_indexprs, NULL)) + { + Datum exprDatum; + char *exprString; + Bitmapset *expr_attrs = NULL; + + /* Convert text string to node tree */ + exprDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple, + Anum_pg_index_indexprs); + exprString = TextDatumGetCString(exprDatum); + node = (Node *) stringToNode(exprString); + pfree(exprString); + + pull_varattnos(node, 1, &expr_attrs); + + if (bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, expr_attrs)) + { + object.classId = RelationRelationId; + object.objectId = indexStruct->indexrelid; + object.objectSubId = 0; + /* record dependency for indexes that references whole-row */ + recordDependencyOn(&object, &tmpobject, DEPENDENCY_AUTO); + } + } + ReleaseSysCache(indexTuple); + } + + 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; + Bitmapset *expr_attrs = NULL; + + 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); + expr = (Node *) stringToNode(str_value); + pfree(str_value); + + pull_varattnos(expr, 1, &expr_attrs); + if (bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, expr_attrs)) + { + object.classId = PolicyRelationId; + object.objectId = policy->oid; + object.objectSubId = 0; + + recordDependencyOn(&object, &tmpobject, DEPENDENCY_NORMAL); + /* we only need one dependency entry */ + continue; + } + } + + datum = heap_getattr(policy_tuple, Anum_pg_policy_polwithcheck, + RelationGetDescr(pg_policy), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + expr = (Node *) stringToNode(str_value); + pfree(str_value); + + pull_varattnos(expr, 1, &expr_attrs); + if (bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, expr_attrs)) + { + object.classId = PolicyRelationId; + object.objectId = policy->oid; + object.objectSubId = 0; + + recordDependencyOn(&object, &tmpobject, DEPENDENCY_NORMAL); + } + } + } + systable_endscan(sscan); + table_close(pg_policy, AccessShareLock); + 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 diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index 3590d3274f0..ce2fb02971f 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -254,6 +254,23 @@ 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 the associated Check constraints and whole-row referenced check constraint +-- +CREATE TABLE DROP_COL_CHECK_TBL ( + city text, state text, is_capital bool, altitude int, + CONSTRAINT cc CHECK (city is not null), + CONSTRAINT cc1 CHECK (DROP_COL_CHECK_TBL is not null)); +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 | | | + is_capital | boolean | | | + altitude | integer | | | + +DROP TABLE DROP_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..ec32543c1e4 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -654,6 +654,31 @@ 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 on idxpart((idxpart is not null)); +create index on idxpart(a) where idxpart is not null; +\d idxpart + Table "public.idxpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | + c | integer | | | +Indexes: + "idxpart_a_idx" btree (a) WHERE idxpart.* IS NOT NULL + "idxpart_c_idx" btree (c) + "idxpart_expr_idx" btree ((idxpart.* IS NOT NULL)) + +alter table idxpart drop column c; +\d idxpart + Table "public.idxpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | + +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/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 8c879509313..febe6538e47 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 drop column also drop whole-row references policy +CREATE TABLE rls_tbl (a int, b int, c int, CONSTRAINT cc1 CHECK (rls_tbl IS NOT NULL)); +CREATE POLICY p1 ON rls_tbl USING (rls_tbl >= ROW(1,1,1)); +CREATE POLICY p2 ON rls_tbl USING (rls_tbl.b = 1); +ALTER TABLE rls_tbl DROP COLUMN b; +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 +policy p2 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 DROP COLUMN b CASCADE; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to policy p1 on table rls_tbl +drop cascades to policy p2 on table rls_tbl +\d rls_tbl + Table "regress_rls_schema.rls_tbl" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + c | integer | | | + +DROP TABLE rls_tbl; -- setup of malicious function CREATE OR REPLACE FUNCTION f_leak(text) RETURNS bool COST 0.0000001 LANGUAGE plpgsql diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index 1f6dc8fd69f..545f8fa17a3 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -165,6 +165,17 @@ 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 the associated Check constraints and whole-row referenced check constraint +-- +CREATE TABLE DROP_COL_CHECK_TBL ( + city text, state text, is_capital bool, altitude int, + CONSTRAINT cc CHECK (city is not null), + CONSTRAINT cc1 CHECK (DROP_COL_CHECK_TBL is not null)); +ALTER TABLE DROP_COL_CHECK_TBL DROP COLUMN city; +\d DROP_COL_CHECK_TBL +DROP TABLE DROP_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..825625b01b6 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -295,6 +295,15 @@ 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 on idxpart((idxpart is not null)); +create index on idxpart(a) where idxpart is not null; +\d idxpart +alter table idxpart drop column c; +\d idxpart +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/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 21ac0ca51ee..21784f86246 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -35,6 +35,15 @@ CREATE SCHEMA regress_rls_schema; GRANT ALL ON SCHEMA regress_rls_schema to public; SET search_path = regress_rls_schema; +--check drop column also drop whole-row references policy +CREATE TABLE rls_tbl (a int, b int, c int, CONSTRAINT cc1 CHECK (rls_tbl IS NOT NULL)); +CREATE POLICY p1 ON rls_tbl USING (rls_tbl >= ROW(1,1,1)); +CREATE POLICY p2 ON rls_tbl USING (rls_tbl.b = 1); +ALTER TABLE rls_tbl DROP COLUMN b; +ALTER TABLE rls_tbl DROP COLUMN b CASCADE; +\d rls_tbl +DROP TABLE rls_tbl; + -- setup of malicious function CREATE OR REPLACE FUNCTION f_leak(text) RETURNS bool COST 0.0000001 LANGUAGE plpgsql -- 2.34.1