From edd9bb976d37dbd00c66d8ffeb6b91da49cf83ec Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 25 May 2026 14:22:45 +0800 Subject: [PATCH v9 1/3] fix DDL wholerow referenced constraints and indexes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ALTER TABLE DROP COLUMN should remove indexes or constraints contain whole-row references. 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. discussion: https://postgr.es/m/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com commitfest: https://commitfest.postgresql.org/patch/6055 --- src/backend/commands/tablecmds.c | 255 ++++++++++++++++++++++ src/test/regress/expected/constraints.out | 23 ++ src/test/regress/expected/indexing.out | 32 +++ src/test/regress/sql/constraints.sql | 17 ++ src/test/regress/sql/indexing.sql | 17 ++ 5 files changed, 344 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1e0bacf85fc..4aa582511bc 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -786,6 +786,9 @@ static void ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, static List *collectPartitionIndexExtDeps(List *partitionOids); static void applyPartitionIndexExtDeps(Oid newPartOid, List *extDepState); static void freePartitionIndexExtDeps(List *extDepState); +static void RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, + Relation rel, AttrNumber attnum, + const char *colName); /* ---------------------------------------------------------------- * DefineRelation @@ -9395,6 +9398,10 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, List *children; ObjectAddress object; bool is_expr; + AlteredTableInfo *tab; + + /* Find or create work queue entry for this table */ + tab = ATGetQueueEntry(wqueue, rel); /* At top level, permission check was done in ATPrepCmd, else do it */ if (recursing) @@ -9467,6 +9474,15 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, ReleaseSysCache(tuple); + /* + * Record dependencies between this relation and any objects containing + * whole-row Var references. performMultipleDeletions will take care of + * removing them dependencies later. + */ + RememberWholeRowDependentForRebuilding(tab, AT_DropColumn, rel, attnum, colName); + + 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 @@ -15118,6 +15134,15 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ RememberAllDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName); + /* + * Record dependencies between this relation and any objects containing + * whole-row Var references. We either error out saying such a dependency + * is not allowed, or we install the dependencies. + */ + RememberWholeRowDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName); + + CommandCounterIncrement(); + /* * Now scan for dependencies of this column on other things. The only * things we should find are the dependency on the column datatype and @@ -23902,3 +23927,233 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Restore the userid and security context. */ SetUserIdAndSecContext(save_userid, save_sec_context); } + +/* + * Record dependencies between objects contains whole-row Var references + * (indexes, CHECK constraints, etc.) and the relation, or report an + * error. +*/ +static void +RememberWholeRowDependent(AlteredTableInfo *tab, AlterTableType subtype, + Relation rel, AttrNumber attnum, + const ObjectAddress *depender, + DependencyType behavior) +{ + if (subtype == AT_AlterColumnType) + ereport(ERROR, + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("cannot alter table \"%s\" because %s uses its row type", + RelationGetRelationName(rel), + getObjectDescription(depender, false)), + errhint("You might need to drop %s first", + getObjectDescription(depender, false))); + else + { + ObjectAddress referenced; + + ObjectAddressSubSet(referenced, RelationRelationId, + RelationGetRelid(rel), attnum); + + recordDependencyOn(depender, &referenced, behavior); + } +} + +/* + * Record dependencies between objects contains whole-row Var references + * (indexes, CHECK constraints, etc.) and the relation, or report an + * error. + * + * See also RememberAllDependentForRebuilding, which handles non-whole-row Var + * references. + * + * Currently used by ALTER COLUMN SET DATA TYPE and ALTER TABLE DROP COLUMN. + */ +static void +RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, + Relation rel, AttrNumber attnum, const char *colName) +{ + Node *expr = NULL; + ScanKeyData skey; + Relation pg_index; + Relation pg_constraint; + SysScanDesc indscan; + SysScanDesc conscan; + HeapTuple constrTuple; + HeapTuple indexTuple; + Datum exprDatum; + char *exprString = NULL; + bool isnull; + List *wholerow_idxoids = NIL; + + Assert(subtype == AT_AlterColumnType || subtype == AT_DropColumn); + + /* + * Checking CHECK constraint with whole-row references, now. + */ + if (RelationGetDescr(rel)->constr && + RelationGetDescr(rel)->constr->num_check > 0) + { + pg_constraint = table_open(ConstraintRelationId, AccessShareLock); + + ScanKeyInit(&skey, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + + conscan = systable_beginscan(pg_constraint, + ConstraintRelidTypidNameIndexId, + true, + NULL, + 1, + &skey); + while (HeapTupleIsValid(constrTuple = systable_getnext(conscan))) + { + Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(constrTuple); + + if (conform->contype != CONSTRAINT_CHECK) + continue; + + exprDatum = fastgetattr(constrTuple, + Anum_pg_constraint_conbin, + RelationGetDescr(pg_constraint), + &isnull); + if (isnull) + elog(WARNING, "null conbin for relation \"%s\"", + RelationGetRelationName(rel)); + else + { + Bitmapset *expr_attrs = NULL; + + exprString = TextDatumGetCString(exprDatum); + expr = (Node *) stringToNode(exprString); + pfree(exprString); + + /* Find all attributes referenced */ + pull_varattnos(expr, 1, &expr_attrs); + + /* + * If the CHECK constraint contains whole-row reference then + * remember it + */ + if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, + expr_attrs)) + { + ObjectAddress con_obj; + + ObjectAddressSet(con_obj, ConstraintRelationId, conform->oid); + + /* + * The dependency between the CHECK constraint and its + * relation is DEPENDENCY_AUTO + */ + RememberWholeRowDependent(tab, subtype, rel, attnum, + &con_obj, DEPENDENCY_AUTO); + } + } + } + systable_endscan(conscan); + table_close(pg_constraint, AccessShareLock); + } + + /* + * Now checking index whole-row references. 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(indexTuple = systable_getnext(indscan))) + { + Form_pg_index index = (Form_pg_index) GETSTRUCT(indexTuple); + + if (list_member_oid(wholerow_idxoids, index->indexrelid)) + continue; + + if (!heap_attisnull(indexTuple, Anum_pg_index_indexprs, NULL)) + { + Bitmapset *expr_attrs = NULL; + + exprDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple, + Anum_pg_index_indexprs); + exprString = TextDatumGetCString(exprDatum); + expr = (Node *) stringToNode(exprString); + pfree(exprString); + + /* Find all attributes referenced */ + pull_varattnos(expr, 1, &expr_attrs); + + /* + * If the index expression contains whole-row reference then + * remember it + */ + if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, + expr_attrs)) + { + ObjectAddress idx_obj; + + ObjectAddressSet(idx_obj, RelationRelationId, + index->indexrelid); + + wholerow_idxoids = lappend_oid(wholerow_idxoids, + index->indexrelid); + + /* + * The index has a DEPENDENCY_AUTO relationship with its + * relation + */ + RememberWholeRowDependent(tab, subtype, rel, attnum, + &idx_obj, DEPENDENCY_AUTO); + + continue; + } + } + + if (!heap_attisnull(indexTuple, Anum_pg_index_indpred, NULL)) + { + Bitmapset *expr_attrs = NULL; + + exprDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple, + Anum_pg_index_indpred); + exprString = TextDatumGetCString(exprDatum); + expr = (Node *) stringToNode(exprString); + pfree(exprString); + + /* Find all attributes referenced */ + pull_varattnos(expr, 1, &expr_attrs); + + /* + * If the index predicate expression contains whole-row reference + * then remember it + */ + if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, + expr_attrs)) + { + ObjectAddress idx_obj; + + ObjectAddressSet(idx_obj, RelationRelationId, index->indexrelid); + + wholerow_idxoids = lappend_oid(wholerow_idxoids, + index->indexrelid); + + /* + * The index has a DEPENDENCY_AUTO relationship with its + * relation + */ + RememberWholeRowDependent(tab, subtype, rel, attnum, + &idx_obj, DEPENDENCY_AUTO); + } + } + } + systable_endscan(indscan); + table_close(pg_index, AccessShareLock); +} diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index e54fec7fb57..658dd421dba 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -254,6 +254,29 @@ ERROR: system column "ctid" reference in check constraint is invalid LINE 3: CHECK (NOT (is_capital AND ctid::text = 'sys_col_check... ^ -- +-- Drop column should also drop all check constraints that contains whole-row references +-- +-- Change column data type should fail since whole-row referenced check constraint still exists +-- +-- No need to worry about partitioned tables, since the whole-row check constraint +-- can not span multi relations +CREATE TABLE wholerow_check_tbl ( + city int, + state int, + CONSTRAINT cc0 CHECK (wholerow_check_tbl is null) NOT ENFORCED, + CONSTRAINT cc1 CHECK (wholerow_check_tbl is not null) NOT ENFORCED); +ALTER TABLE wholerow_check_tbl ALTER COLUMN city SET DATA TYPE INT8; -- error +ERROR: cannot alter table "wholerow_check_tbl" because constraint cc0 on table wholerow_check_tbl uses its row type +HINT: You might need to drop constraint cc0 on table wholerow_check_tbl first +ALTER TABLE wholerow_check_tbl DROP COLUMN city; -- ok +\d wholerow_check_tbl + Table "public.wholerow_check_tbl" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + state | integer | | | + +DROP TABLE wholerow_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 929feda6fa3..424f7ea3f68 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -759,6 +759,38 @@ alter table idxpart2 drop column c; b | integer | | | drop table idxpart, idxpart2; +-- Drop column should also drop all indexes that contains whole-row references +-- Change column data type should fail if whole-row referenced indexes exists +create table idxpart (a int, b int, c int) partition by range (a); +create table idxpart1 partition of idxpart for values from (2000) to (3000); +create index idxpart_idx1 on idxpart1((idxpart1 is not null)); +alter table idxpart alter column b set data type bigint; -- error +ERROR: cannot alter table "idxpart1" because index idxpart_idx1 uses its row type +HINT: You might need to drop index idxpart_idx1 first +drop index idxpart_idx1; +create index idxpart_idx2 on idxpart1(a) where idxpart1 is not null; +alter table idxpart alter column c set data type bigint; -- error +ERROR: cannot alter table "idxpart1" because index idxpart_idx2 uses its row type +HINT: You might need to drop index idxpart_idx2 first +alter table idxpart drop column c; -- ok +\d idxpart + Partitioned table "public.idxpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Partition key: RANGE (a) +Number of partitions: 1 (Use \d+ to list them.) + +\d idxpart1 + Table "public.idxpart1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Partition of: idxpart FOR VALUES FROM (2000) TO (3000) + +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 dc133b124bb..2600c9f518d 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -165,6 +165,23 @@ 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 should also drop all check constraints that contains whole-row references +-- +-- Change column data type should fail since whole-row referenced check constraint still exists +-- +-- No need to worry about partitioned tables, since the whole-row check constraint +-- can not span multi relations +CREATE TABLE wholerow_check_tbl ( + city int, + state int, + CONSTRAINT cc0 CHECK (wholerow_check_tbl is null) NOT ENFORCED, + CONSTRAINT cc1 CHECK (wholerow_check_tbl is not null) NOT ENFORCED); +ALTER TABLE wholerow_check_tbl ALTER COLUMN city SET DATA TYPE INT8; -- error +ALTER TABLE wholerow_check_tbl DROP COLUMN city; -- ok +\d wholerow_check_tbl +DROP TABLE wholerow_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 3d43af3323c..9bc5b83a5e6 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -354,6 +354,23 @@ alter table idxpart2 drop column c; \d idxpart2 drop table idxpart, idxpart2; +-- Drop column should also drop all indexes that contains whole-row references +-- Change column data type should fail if whole-row referenced indexes exists +create table idxpart (a int, b int, c int) partition by range (a); +create table idxpart1 partition of idxpart for values from (2000) to (3000); + +create index idxpart_idx1 on idxpart1((idxpart1 is not null)); +alter table idxpart alter column b set data type bigint; -- error +drop index idxpart_idx1; + +create index idxpart_idx2 on idxpart1(a) where idxpart1 is not null; +alter table idxpart alter column c set data type bigint; -- error +alter table idxpart drop column c; -- ok + +\d idxpart +\d idxpart1 +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