From 5d0eb5f8091c666fe20eefa7b1fbd7666c966b55 Mon Sep 17 00:00:00 2001 From: jian he Date: Fri, 12 Sep 2025 16:30:18 +0800 Subject: [PATCH v3 1/2] 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; ALTER TABLE ts DROP COLUMN a CASCADE; will drop above all indexes, constraints 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 | 141 ++++++++++++++++++++++ src/test/regress/expected/constraints.out | 17 +++ src/test/regress/expected/indexing.out | 13 ++ src/test/regress/sql/constraints.sql | 11 ++ src/test/regress/sql/indexing.sql | 8 ++ 5 files changed, 190 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3be2e051d32..409bc65e53f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9260,7 +9260,11 @@ 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; /* At top level, permission check was done in ATPrepCmd, else do it */ if (recursing) @@ -9333,6 +9337,143 @@ 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); + } + 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..cac1cca3a1f 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -654,6 +654,19 @@ 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; +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/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..2bad9555112 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -295,6 +295,14 @@ 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; +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); -- 2.34.1