On Mon, Sep 15, 2025 at 11:48 AM Chao Li <li.evan.c...@gmail.com> wrote:
>
> 3 - 0001
> ```
> + }
> + }
> + ReleaseSysCache(indexTuple);
> + }
> + CommandCounterIncrement();
> ```
>
> Why CommandCounterIncrement() is needed? In current code, there is a
> CommandCounterIncrement() after CatalogTupleUpdate(), which is necessary. But
> for your new code, maybe you considered “recordDependencyOn()” needs
> CommandCounterIncrement(). I searched over all places when
> “recordDependencyOn()” is called, I don’t see CommandCounterIncrement() is
> called.
>
My thought is that CommandCounterIncrement may be needed;
because recordDependencyOn inserts many tuples to pg_depend,
then later performMultipleDeletions will interact with pg_depend.
>
> 5 - 0001
> ···
> + 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));
> ···
>
> Should we continue after elog()?
>
if "elog(ERROR," happens, then it will abort, so there is no need to
"continue", I think.
Summary of attached v4:
v4-0001: Handles ALTER TABLE DROP COLUMN when whole-row Vars are
referenced in check constraints and indexes.
v4-0002: Handles ALTER TABLE ALTER COLUMN SET DATA TYPE when whole-row
Vars are referenced in check constraints and indexes.
v4-0003: Handle ALTER TABLE ALTER COLUMN SET DATA TYPE and ALTER TABLE DROP
COLUMN when policy objects reference whole-row Vars. Policy quals and check
quals may contain whole-row Vars and can include sublinks (unplanned
subqueries), pull_varattnos is not enough to locate whole-row Var. Instead,
obtain the whole-row type OID and recursively check each Var in expression node
to see if its vartype matches the whole-row type OID.
From 5d3ad72059977c6c3576c92e1ba25684f80b628d Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Mon, 15 Sep 2025 20:19:29 +0800
Subject: [PATCH v4 3/3] disallow change or drop column when wholerow
referenced policy exists
demo:
CREATE TABLE rls_tbl (a int, b int, c int);
CREATE POLICY p1 ON rls_tbl USING (rls_tbl >= ROW(1,1,1));
ALTER TABLE rls_tbl DROP COLUMN b; --error
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
HINT: Use DROP ... CASCADE to drop the dependent objects too.
ALTER TABLE rls_tbl ALTER COLUMN b SET DATA TYPE BIGINT; --error
ERROR: cannot alter table "rls_tbl" because security policy "p1" uses its row type
HINT: You might need to drop policy "p1" first
ALTER TABLE rls_tbl DROP COLUMN b CASCADE; --ok
NOTICE: drop cascades to policy p1 on table rls_tbl
ALTER TABLE
discussion: https://postgr.es/m/cacjufxga6kvqy7dbhglvw9s9kkmpgyzt5me6c7kefjdpr2w...@mail.gmail.com
---
src/backend/commands/tablecmds.c | 91 ++++++++++++++++++++++-
src/backend/optimizer/util/var.c | 60 +++++++++++++++
src/include/optimizer/optimizer.h | 1 +
src/test/regress/expected/rowsecurity.out | 17 +++++
src/test/regress/sql/rowsecurity.sql | 12 +++
src/tools/pgindent/typedefs.list | 1 +
6 files changed, 181 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4949d7204a0..403b8a1adb8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -22096,7 +22096,7 @@ GetAttributeStorage(Oid atttypid, const char *storagemode)
}
/*
- * Record dependencies between whole-row objects (indexes, constraints)
+ * Record dependencies between whole-row objects (indexes, constraints or policies)
* associated with relation and relation's ObjectAddress.
* error_out means can not install such dependency, we have to error out explicitly.
*/
@@ -22107,9 +22107,15 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo
List *indexlist = NIL;
ObjectAddress con_obj;
ObjectAddress idx_obj;
+ ObjectAddress pol_obj;
bool find_wholerow = false;
TupleConstr *constr = RelationGetDescr(rel)->constr;
+ Relation pg_policy;
+ ScanKeyData skey[1];
+ SysScanDesc sscan;
+ HeapTuple policy_tuple;
+ Oid reltypid;
if (constr && constr->num_check > 0)
{
@@ -22265,4 +22271,87 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo
}
ReleaseSysCache(indexTuple);
}
+
+ find_wholerow = false;
+ reltypid = get_rel_type_id(RelationGetRelid(rel));
+
+ 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;
+ Node *polexpr;
+
+ 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);
+ polexpr = (Node *) stringToNode(str_value);
+ pfree(str_value);
+
+ find_wholerow = ExprContainWholeRow(polexpr, reltypid);
+ if (find_wholerow && !error_out)
+ {
+ pol_obj.classId = PolicyRelationId;
+ pol_obj.objectId = policy->oid;
+ pol_obj.objectSubId = 0;
+
+ /* record dependency for policies that references whole-row Var */
+ recordDependencyOn(&pol_obj, object, DEPENDENCY_NORMAL);
+ continue;
+ }
+
+ if (find_wholerow && error_out)
+ ereport(ERROR,
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("cannot alter table \"%s\" because security policy \"%s\" uses its row type",
+ RelationGetRelationName(rel),
+ NameStr(policy->polname)),
+ errhint("You might need to drop policy \"%s\" first",
+ NameStr(policy->polname)));
+ }
+
+ datum = heap_getattr(policy_tuple, Anum_pg_policy_polwithcheck,
+ RelationGetDescr(pg_policy), &isnull);
+ if (!isnull)
+ {
+ str_value = TextDatumGetCString(datum);
+ polexpr = (Node *) stringToNode(str_value);
+ pfree(str_value);
+
+ find_wholerow = ExprContainWholeRow(polexpr, reltypid);
+ if (find_wholerow && !error_out)
+ {
+ pol_obj.classId = PolicyRelationId;
+ pol_obj.objectId = policy->oid;
+ pol_obj.objectSubId = 0;
+
+ /* record dependency for policies that references whole-row Var */
+ recordDependencyOn(&pol_obj, object, DEPENDENCY_NORMAL);
+ }
+
+ if (find_wholerow && error_out)
+ ereport(ERROR,
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("cannot alter table \"%s\" because security policy \"%s\" uses its row type",
+ RelationGetRelationName(rel),
+ NameStr(policy->polname)),
+ errhint("You might need to drop policy \"%s\" first",
+ NameStr(policy->polname)));
+ }
+ }
+ systable_endscan(sscan);
+ table_close(pg_policy, AccessShareLock);
}
diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 8065237a189..36de0a3bfca 100644
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -48,6 +48,10 @@ typedef struct
List *vars;
int sublevels_up;
} pull_vars_context;
+typedef struct
+{
+ Oid reltypid;
+} contain_wholerow_context;
typedef struct
{
@@ -73,6 +77,7 @@ typedef struct
static bool pull_varnos_walker(Node *node,
pull_varnos_context *context);
static bool pull_varattnos_walker(Node *node, pull_varattnos_context *context);
+static bool ExprContainWholeRow_walker(Node *node, contain_wholerow_context *context);
static bool pull_vars_walker(Node *node, pull_vars_context *context);
static bool contain_var_clause_walker(Node *node, void *context);
static bool contain_vars_of_level_walker(Node *node, int *sublevels_up);
@@ -327,6 +332,61 @@ pull_varattnos_walker(Node *node, pull_varattnos_context *context)
return expression_tree_walker(node, pull_varattnos_walker, context);
}
+static bool
+ExprContainWholeRow_walker(Node *node, contain_wholerow_context *context)
+{
+ if (node == NULL)
+ return false;
+
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ if (var->varattno == InvalidAttrNumber &&
+ var->vartype == context->reltypid)
+ return true;
+
+ return false;
+ }
+
+ if (IsA(node, Query))
+ {
+ /* Recurse into RTE subquery or not-yet-planned sublink subquery */
+ bool result;
+
+ result = query_tree_walker((Query *) node, ExprContainWholeRow_walker,
+ context, 0);
+ return result;
+ }
+
+ return expression_tree_walker(node, ExprContainWholeRow_walker, context);
+}
+
+
+/*
+ * ExprContainWholeRow -
+ *
+ * Determine whether an expression contains a whole-row Var, recursing as needed.
+ * For simple expressions without sublinks, pull_varattnos is usually sufficient
+ * to detect a whole-row Var. If the node contains sublinks (unplanned subqueries),
+ * the check must instead rely on the whole-row type OID. Therefore, reltypid is
+ * used consistently to determine the presence of a whole-row Var.
+ */
+bool
+ExprContainWholeRow(Node *node, Oid reltypid)
+{
+ contain_wholerow_context context;
+
+ context.reltypid = reltypid;
+
+ Assert(OidIsValid(reltypid));
+
+ return query_or_expression_tree_walker(node,
+ ExprContainWholeRow_walker,
+ &context,
+ 0);
+}
+
/*
* pull_vars_of_level
diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index 37bc13c2cbd..d31d1c49c75 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -205,6 +205,7 @@ extern SortGroupClause *get_sortgroupref_clause_noerr(Index sortref,
extern Bitmapset *pull_varnos(PlannerInfo *root, Node *node);
extern Bitmapset *pull_varnos_of_level(PlannerInfo *root, Node *node, int levelsup);
extern void pull_varattnos(Node *node, Index varno, Bitmapset **varattnos);
+extern bool ExprContainWholeRow(Node *node, Oid reltypid);
extern List *pull_vars_of_level(Node *node, int levelsup);
extern bool contain_var_clause(Node *node);
extern bool contain_vars_of_level(Node *node, int levelsup);
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 8c879509313..b2b5333f8dc 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -2357,6 +2357,23 @@ SELECT * FROM document;
14 | 11 | 1 | regress_rls_bob | new novel |
(16 rows)
+--check drop column restrict or alter column data type will fail because of whole-row
+--referenced security policy exists.
+ALTER TABLE document ADD COLUMN dummy INT4;
+CREATE POLICY p7 ON document AS PERMISSIVE
+ USING (cid IS NOT NULL AND
+ (WITH cte AS (SELECT TRUE FROM uaccount
+ WHERE EXISTS (SELECT document FROM uaccount WHERE uaccount IS NULL))
+ SELECT * FROM cte));
+ALTER TABLE document ALTER COLUMN dummy SET DATA TYPE BIGINT; --error
+ERROR: cannot alter table "document" because security policy "p7" uses its row type
+HINT: You might need to drop policy "p7" first
+ALTER TABLE document DROP COLUMN dummy; --error
+ERROR: cannot drop column dummy of table document because other objects depend on it
+DETAIL: policy p7 on table document depends on column dummy of table document
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+ALTER TABLE document DROP COLUMN dummy CASCADE; --ok
+NOTICE: drop cascades to policy p7 on table document
--
-- ROLE/GROUP
--
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 21ac0ca51ee..0cdd5a82ca3 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -1021,6 +1021,18 @@ DROP POLICY p1 ON document;
-- Just check everything went per plan
SELECT * FROM document;
+--check drop column restrict or alter column data type will fail because of whole-row
+--referenced security policy exists.
+ALTER TABLE document ADD COLUMN dummy INT4;
+CREATE POLICY p7 ON document AS PERMISSIVE
+ USING (cid IS NOT NULL AND
+ (WITH cte AS (SELECT TRUE FROM uaccount
+ WHERE EXISTS (SELECT document FROM uaccount WHERE uaccount IS NULL))
+ SELECT * FROM cte));
+ALTER TABLE document ALTER COLUMN dummy SET DATA TYPE BIGINT; --error
+ALTER TABLE document DROP COLUMN dummy; --error
+ALTER TABLE document DROP COLUMN dummy CASCADE; --ok
+
--
-- ROLE/GROUP
--
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a13e8162890..81b97cc622e 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3521,6 +3521,7 @@ conn_oauth_scope_func
conn_sasl_state_func
contain_aggs_of_level_context
contain_placeholder_references_context
+contain_wholerow_context
convert_testexpr_context
copy_data_dest_cb
copy_data_source_cb
--
2.34.1
From edf09b4d7da937d946f5c6ef6ca6806019d37fe0 Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Mon, 15 Sep 2025 15:32:49 +0800
Subject: [PATCH v4 1/3] 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))));
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 | 170 +++++++++++++++++++++-
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, 216 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3be2e051d32..7f5e58da62f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -740,6 +740,7 @@ static List *GetParentedForeignKeyRefs(Relation partition);
static void ATDetachCheckNoForeignKeyRefs(Relation partition);
static char GetAttributeCompression(Oid atttypid, const char *compression);
static char GetAttributeStorage(Oid atttypid, const char *storagemode);
+static void recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, bool error_out);
/* ----------------------------------------------------------------
@@ -9333,6 +9334,26 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
ReleaseSysCache(tuple);
+ object.classId = RelationRelationId;
+ object.objectId = RelationGetRelid(rel);
+ object.objectSubId = attnum;
+
+ /*
+ * ALTER TABLE DROP COLUMN must also remove indexes or constraints that
+ * contain whole-row Var reference expressions. Since there is no direct
+ * dependency recorded between whole-row Vars and individual columns, and
+ * creating such dependencies would cause catalog bloat (see
+ * find_expr_references_walker).
+ *
+ * Here we handle this explicitly. We call recordWholeRowDependencyOnOrError
+ * to establish a dependency between the column and any constraint or index
+ * involving whole-row Vars. performMultipleDeletions will then take care
+ * of removing them.
+ */
+ 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
@@ -9426,9 +9447,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)
@@ -22062,3 +22080,149 @@ GetAttributeStorage(Oid atttypid, const char *storagemode)
return cstorage;
}
+
+/*
+ * Record dependencies between whole-row objects (indexes, constraints)
+ * associated with relation and relation's ObjectAddress.
+ * error_out means can not install such dependency, we have to error out explicitly.
+ */
+static void
+recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, bool error_out)
+{
+ Node *expr;
+ List *indexlist = NIL;
+ ObjectAddress con_obj;
+ ObjectAddress idx_obj;
+ bool find_wholerow = false;
+
+ TupleConstr *constr = RelationGetDescr(rel)->constr;
+
+ 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);
+
+ find_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber,
+ expr_attrs);
+ if (find_wholerow && !error_out)
+ {
+ 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));
+
+ con_obj.classId = ConstraintRelationId;
+ con_obj.objectId = ((Form_pg_constraint) GETSTRUCT(contuple))->oid;
+ con_obj.objectSubId = 0;
+
+ /* record dependency for constraints that references whole-row */
+ recordDependencyOn(&con_obj, object, DEPENDENCY_AUTO);
+
+ systable_endscan(conscan);
+ table_close(conDesc, AccessShareLock);
+ }
+ }
+ }
+
+ find_wholerow = false;
+ indexlist = RelationGetIndexList(rel);
+ foreach_oid(indexoid, indexlist)
+ {
+ HeapTuple indexTuple;
+ Form_pg_index indexStruct;
+ Node *node;
+
+ 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);
+ find_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber,
+ expr_attrs);
+ if (find_wholerow && !error_out)
+ {
+ idx_obj.classId = RelationRelationId;
+ idx_obj.objectId = indexStruct->indexrelid;
+ idx_obj.objectSubId = 0;
+ /* record dependency for indexes that references whole-row */
+ recordDependencyOn(&idx_obj, object, DEPENDENCY_AUTO);
+
+ ReleaseSysCache(indexTuple);
+ continue;
+ }
+ }
+
+ if (!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);
+
+ find_wholerow = bms_is_member(0 - FirstLowInvalidHeapAttributeNumber,
+ expr_attrs);
+ if (find_wholerow && !error_out)
+ {
+ idx_obj.classId = RelationRelationId;
+ idx_obj.objectId = indexStruct->indexrelid;
+ idx_obj.objectSubId = 0;
+ /* record dependency for indexes that references whole-row */
+ recordDependencyOn(&idx_obj, object, DEPENDENCY_AUTO);
+ }
+ }
+ ReleaseSysCache(indexTuple);
+ }
+}
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index 3590d3274f0..8930b4da7ce 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 check constraints that have whole-row reference
+--
+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..676d8736101 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 check constraints that have whole-row reference
+--
+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
From 07ac16212729fb71b0f3fa8557688e0788477625 Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Mon, 15 Sep 2025 15:53:59 +0800
Subject: [PATCH v4 2/3] 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 "ts" because constraint "cc" uses its row type
HINT: You might need to drop constraint "cc" first
create index on ts((ts is not null));
alter table ts alter column a set data type int8; --error
ERROR: cannot alter table "ts" because index "ts_expr_idx" uses its row type
HINT: You might need to drop index "ts_expr_idx" first
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/expected/indexing.out | 6 ++++
src/test/regress/sql/constraints.sql | 10 ++++++
src/test/regress/sql/indexing.sql | 2 ++
5 files changed, 69 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7f5e58da62f..4949d7204a0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14541,6 +14541,20 @@ 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)
+ {
+ ObjectAddress object;
+
+ object.classId = RelationRelationId;
+ object.objectId = RelationGetRelid(rel);
+ object.objectSubId = attnum;
+ recordWholeRowDependencyOnOrError(rel, &object, true);
+ }
+
ReleaseSysCache(tuple);
/*
@@ -22152,6 +22166,14 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo
systable_endscan(conscan);
table_close(conDesc, AccessShareLock);
}
+ if (find_wholerow && error_out)
+ ereport(ERROR,
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("cannot alter table \"%s\" because constraint \"%s\" uses its row type",
+ RelationGetRelationName(rel),
+ constr_name),
+ errhint("You might need to drop constraint \"%s\" first",
+ constr_name));
}
}
@@ -22195,6 +22217,15 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo
ReleaseSysCache(indexTuple);
continue;
}
+
+ if (find_wholerow && error_out)
+ ereport(ERROR,
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("cannot alter table \"%s\" because index \"%s\" uses its row type",
+ RelationGetRelationName(rel),
+ get_rel_name(indexStruct->indexrelid)),
+ errhint("You might need to drop index \"%s\" first",
+ get_rel_name(indexStruct->indexrelid)));
}
if (!heap_attisnull(indexTuple, Anum_pg_index_indexprs, NULL))
@@ -22222,6 +22253,15 @@ recordWholeRowDependencyOnOrError(Relation rel, const ObjectAddress *object, boo
/* record dependency for indexes that references whole-row */
recordDependencyOn(&idx_obj, object, DEPENDENCY_AUTO);
}
+
+ if (find_wholerow && error_out)
+ ereport(ERROR,
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("cannot alter table \"%s\" because index \"%s\" uses its row type",
+ RelationGetRelationName(rel),
+ get_rel_name(indexStruct->indexrelid)),
+ errhint("You might need to drop index \"%s\" first",
+ get_rel_name(indexStruct->indexrelid)));
}
ReleaseSysCache(indexTuple);
}
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index 8930b4da7ce..4527016f319 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 int, 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 int8;
+ERROR: cannot alter table "alter_col_check_tbl" because constraint "cc1" uses its row type
+HINT: You might need to drop constraint "cc1" first
+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/expected/indexing.out b/src/test/regress/expected/indexing.out
index cac1cca3a1f..e544141ad9c 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -657,7 +657,13 @@ 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));
+alter table idxpart alter column a set data type int8; --error
+ERROR: cannot alter table "idxpart" because index "idxpart_expr_idx" uses its row type
+HINT: You might need to drop index "idxpart_expr_idx" first
create index on idxpart(a) where idxpart is not null;
+alter table idxpart alter column a set data type int8; --error
+ERROR: cannot alter table "idxpart" because index "idxpart_expr_idx" uses its row type
+HINT: You might need to drop index "idxpart_expr_idx" first
alter table idxpart drop column c;
\d idxpart
Table "public.idxpart"
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index 676d8736101..bbe8328fbf9 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 int, 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 int8;
+DROP TABLE ALTER_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 2bad9555112..0711860443e 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -298,7 +298,9 @@ 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));
+alter table idxpart alter column a set data type int8; --error
create index on idxpart(a) where idxpart is not null;
+alter table idxpart alter column a set data type int8; --error
alter table idxpart drop column c;
\d idxpart
drop table idxpart;
--
2.34.1
