On Thu, Sep 11, 2025 at 9:27 AM Chao Li <[email protected]> wrote: > > But v2 needs a rebase, I cannot apply it to master. >
hi. please check attached v3-0001, v3-0002. v3-0001: ALTER TABLE DROP COLUMN cascade to drop any whole-row referenced constraints and indexes. v3-0002: ALTER COLUMN SET DATA TYPE error out when whole-row referenced constraint exists with the v3-0002 patch, CREATE TABLE ts (a int, c int, b int constraint cc check((ts = ROW(1,1,1)))); ALTER TABLE ts ALTER COLUMN b SET DATA TYPE INT8; ERROR: cannot alter table column "ts"."b" to type bigint because constraint "cc" uses table "ts" row type HINT: You might need to drop constraint "cc" first to change column "ts"."b" data type Of course, even if we do not error out, regular insert will fail too. insert into ts values(1,1,1); ERROR: cannot compare dissimilar column types bigint and integer at record column 3 src7=# \errverbose ERROR: 42804: cannot compare dissimilar column types bigint and integer at record column 3 LOCATION: record_eq, rowtypes.c:1193 then you need debug to find out the root error cause is constraint cc is not being satisfied; and you still need to handle the corrupted constraint cc afterward. With the v3-0002 patch, ALTER TABLE SET DATA TYPE provides an explicit error message that helps quickly identify the problem. So I guess it should be helpful. -------------------------------- index expression/predicate and check constraint expression can not contain subquery, that's why using pull_varattnos to test whole-row containment works fine. but pull_varattnos can not cope with subquery, see pull_varattnos comments. row security policy can have subquery, for example: CREATE POLICY p1 ON document AS PERMISSIVE USING (dlevel <= (SELECT seclv FROM uaccount WHERE pguser = current_user)); so I am still working on whole-row referenced policies interacting with ALTER TABLE SET DATA TYPE/ALTER TABLE DROP COLUMN.
From 090a087da9b7fb072acd7e9683faf9ba2b5c76af Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Fri, 12 Sep 2025 17:10:19 +0800 Subject: [PATCH v3 2/2] disallow ALTER COLUMN SET DATA TYPE when wholerow referenced constraint exists CREATE TABLE ts (a int, c int, b int constraint cc check((ts = ROW(1,1,1)))); INSERT INTO ts values (1,1,1); ALTER TABLE ts ALTER COLUMN b SET DATA TYPE INT8; ERROR: cannot alter table column "ts"."b" to type bigint because constraint "cc" uses table "ts" row type HINT: You might need to drop constraint "cc" first to change column "ts"."b" data type No need to worry about the index; index rebuild will fail automatically. discussion: https://postgr.es/m/cacjufxga6kvqy7dbhglvw9s9kkmpgyzt5me6c7kefjdpr2w...@mail.gmail.com --- src/backend/commands/tablecmds.c | 40 +++++++++++++++++++++++ src/test/regress/expected/constraints.out | 11 +++++++ src/test/regress/sql/constraints.sql | 10 ++++++ 3 files changed, 61 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 409bc65e53f..62c84726c4e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14664,6 +14664,46 @@ ATPrepAlterColumnType(List **wqueue, find_composite_type_dependencies(rel->rd_rel->reltype, rel, NULL); } + /* + * If the table has a whole-row referenced CHECK constraint, then changing + * any column data type is not allowed. + */ + if (targettype != attTup->atttypid || targettypmod != attTup->atttypmod) + { + TupleConstr *constr = RelationGetDescr(rel)->constr; + + if (constr && constr->num_check > 0) + { + ConstrCheck *check = constr->check; + Node *check_expr; + + for (int i = 0; i < constr->num_check; i++) + { + Bitmapset *expr_attrs = NULL; + + char *constr_name = check[i].ccname; + check_expr = stringToNode(check[i].ccbin); + + /* Find all attributes referenced */ + pull_varattnos(check_expr, 1, &expr_attrs); + + if (bms_is_member(0 - FirstLowInvalidHeapAttributeNumber, expr_attrs)) + ereport(ERROR, + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("cannot alter table column \"%s\".\"%s\" to type %s because constraint \"%s\" uses table \"%s\" row type", + RelationGetRelationName(rel), + colName, + format_type_with_typemod(targettype, targettypmod), + constr_name, + RelationGetRelationName(rel)), + errhint("You might need to drop constraint \"%s\" first to change column \"%s\".\"%s\" data type", + constr_name, + RelationGetRelationName(rel), + colName)); + } + } + } + ReleaseSysCache(tuple); /* diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index ce2fb02971f..6de0cc4ec33 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -271,6 +271,17 @@ ALTER TABLE DROP_COL_CHECK_TBL DROP COLUMN city; 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 text, state text, is_capital bool, altitude int, + CONSTRAINT cc1 CHECK (ALTER_COL_CHECK_TBL is not null)); +ALTER TABLE ALTER_COL_CHECK_TBL ALTER COLUMN city SET DATA TYPE name; +ERROR: cannot alter table column "alter_col_check_tbl"."city" to type name because constraint "cc1" uses table "alter_col_check_tbl" row type +HINT: You might need to drop constraint "cc1" first to change column "alter_col_check_tbl"."city" data type +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/sql/constraints.sql b/src/test/regress/sql/constraints.sql index 545f8fa17a3..3d33d3e2d9a 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -176,6 +176,16 @@ 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 text, state text, is_capital bool, altitude int, + CONSTRAINT cc1 CHECK (ALTER_COL_CHECK_TBL is not null)); +ALTER TABLE ALTER_COL_CHECK_TBL ALTER COLUMN city SET DATA TYPE name; +DROP TABLE ALTER_COL_CHECK_TBL; + -- -- Check inheritance of defaults and constraints -- -- 2.34.1
From 5d0eb5f8091c666fe20eefa7b1fbd7666c966b55 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> 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/cacjufxga6kvqy7dbhglvw9s9kkmpgyzt5me6c7kefjdpr2w...@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
