From 1ec12f88281f4fb669cf9d20f6d36b5290a9913d Mon Sep 17 00:00:00 2001 From: jian he Date: Sat, 28 Feb 2026 15:07:21 +0800 Subject: [PATCH v8 1/3] 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 will 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 let performMultipleDeletions do the deleation. 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 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/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com commitfest: https://commitfest.postgresql.org/patch/6055 --- src/backend/commands/tablecmds.c | 221 +++++++++++++++++++++- src/test/regress/expected/constraints.out | 21 ++ src/test/regress/expected/indexing.out | 35 ++++ src/test/regress/sql/constraints.sql | 16 ++ src/test/regress/sql/indexing.sql | 20 ++ 5 files changed, 310 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b04b0dbd2a0..da43e9efed4 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 @@ -9391,6 +9394,21 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, ReleaseSysCache(tuple); + object.classId = RelationRelationId; + object.objectId = RelationGetRelid(rel); + object.objectSubId = attnum; + + /* + * We also need remove indexes or constraints that contain whole-row Var + * reference. Using recordWholeRowDependencyOnOrError to establish a + * dependency between the column and any CHECK constraint or index + * contains whole-row Vars reference. Function 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 @@ -9484,9 +9502,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) @@ -14772,6 +14787,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 @@ -14867,6 +14883,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,192 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Restore the userid and security context. */ SetUserIdAndSecContext(save_userid, save_sec_context); } + +static void +recordDependencyOnOrError(Relation rel, const ObjectAddress *depender, + const ObjectAddress *referenced, bool error_out, + DependencyType behavior) +{ + if (error_out) + 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 + recordDependencyOn(depender, referenced, behavior); +} + +/* + * Record dependencies between whole-row Var referenced objects (indexes, CHECK + * constraints, etc) and the relation's ObjectAddress. + * + * error_out means can not install such dependency, we need 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; + HeapTuple indexTuple; + Bitmapset *expr_attrs = NULL; + Datum exprDatum; + char *exprString; + bool isnull; + bool have_wholerow = false; + List *wholerow_idxoids = NIL; + 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); + + 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; + + 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); + + have_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, + expr_attrs); + bms_free(expr_attrs); + expr_attrs = NULL; + + if (have_wholerow) + { + con_obj.classId = ConstraintRelationId; + con_obj.objectId = conform->oid; + con_obj.objectSubId = 0; + + recordDependencyOnOrError(rel, &con_obj, object, error_out, + 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))) + { + ObjectAddress idx_obj; + 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)) + { + 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); + + have_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, + expr_attrs); + bms_free(expr_attrs); + expr_attrs = NULL; + + if (have_wholerow) + { + idx_obj.classId = RelationRelationId; + idx_obj.objectId = index->indexrelid; + idx_obj.objectSubId = 0; + + wholerow_idxoids = lappend_oid(wholerow_idxoids, index->indexrelid); + + recordDependencyOnOrError(rel, &idx_obj, object, error_out, + DEPENDENCY_AUTO); + + continue; + } + } + + if (!heap_attisnull(indexTuple, Anum_pg_index_indpred, 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); + + have_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, + expr_attrs); + bms_free(expr_attrs); + expr_attrs = NULL; + + if (have_wholerow) + { + idx_obj.classId = RelationRelationId; + idx_obj.objectId = index->indexrelid; + idx_obj.objectSubId = 0; + + wholerow_idxoids = lappend_oid(wholerow_idxoids, index->indexrelid); + + recordDependencyOnOrError(rel, &idx_obj, object, error_out, + 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 a6fa9cacb72..39600331b35 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -254,6 +254,27 @@ 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 all check constraints that have whole-row references. +-- +-- Change column data type should error out because the whole-row referenced +-- check constraint is still needed. +-- no need to worry about partitioned tables, since the whole-row check constraint +-- can not span multi relation. +CREATE TABLE wholerow_check_tbl ( + city 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: 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 +--------+------+-----------+----------+--------- + +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 dc629928c8f..b76c9ffaefb 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -654,6 +654,41 @@ alter table idxpart2 drop column c; b | integer | | | drop table idxpart, idxpart2; +-- +-- Drop column also drop all indexes that have whole-row references. +-- +-- Change column data type should error out because the whole-row referenced +-- indexex is still needed. +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 int8; --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 int8; --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; +\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 b7f6efdd814..cb2f3fc7d5e 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -165,6 +165,22 @@ 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 all check constraints that have whole-row references. +-- +-- Change column data type should error out because the whole-row referenced +-- check constraint is still needed. +-- no need to worry about partitioned tables, since the whole-row check constraint +-- can not span multi relation. +CREATE TABLE wholerow_check_tbl ( + city 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; +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 b5cb01c2d70..0dfef988d0a 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -295,6 +295,26 @@ alter table idxpart2 drop column c; \d idxpart2 drop table idxpart, idxpart2; +-- +-- Drop column also drop all indexes that have whole-row references. +-- +-- Change column data type should error out because the whole-row referenced +-- indexex is still needed. +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 int8; --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 int8; --error +alter table idxpart drop column c; + +\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