let ALTER TABLE DROP COLUMN drop whole-row referenced object
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t52253psql -h localhost -U postgresBuilt from patchset v15 (message #15), August 26, 2026 at 06:17 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t52253_15 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t52253_15 && git checkout t52253_15Patchset v15 (message #15) is on t52253_15
hi.
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;
in the master, ``ALTER TABLE ts DROP COLUMN a;``
will not drop constraint cc, index tsi3, tsi4;
with the attached patch,
``ALTER TABLE ts DROP COLUMN a;``
will drop above all indexes on the table "ts" and also remove the
constraints "cc" and "cc1".
as per the documentation[1]https://www.postgresql.org/docs/devel/sql-altertable.html#SQL-ALTERTABLE-DESC-DROP-COLUMN, quote:
"""
DROP COLUMN [ IF EXISTS ]
This form drops a column from a table. Indexes and table constraints involving
the column will be automatically dropped as well.
"""
so I think it's expected behavior to drop the entire
whole-row referenced indexes and constraints.
[1]: https://www.postgresql.org/docs/devel/sql-altertable.html#SQL-ALTERTABLE-DESC-DROP-COLUMN
Attachments:
v1-0001-let-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchtext/x-patch; charset=US-ASCII; name=v1-0001-let-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchDownload+184-1
hi.
I found a new way to solve this problem.
CREATE TABLE ts (a int, c int, b int
constraint cc check((ts = ROW(1,1,1))),
constraint cc1 check((ts.a = 1)));
for constraint cc, there is no extra dependency between column a and
constraint cc.
see find_expr_references_walker below comments:
/*
* A whole-row Var references no specific columns, so adds no new
* dependency. (We assume that there is a whole-table dependency
* arising from each underlying rangetable entry. While we could
* record such a dependency when finding a whole-row Var that
* references a relation directly, it's quite unclear how to extend
* that to whole-row Vars for JOINs, so it seems better to leave the
* responsibility with the range table. Note that this poses some
* risks for identifying dependencies of stand-alone expressions:
* whole-table references may need to be created separately.)
*/
Ideally, for constraint "cc", there should be three pg_depend entries
corresponding to column a, column b, and column c, but those entries are
missing, but we didn't.
so, in ATExecDropColumn, instead of adding another object to the deletion
list (``add_exact_object_address(&object, addrs)``) like what we did v1,
we first call recordDependencyOn to explicitly record the dependency between
constraint cc and column a, and then rely on performMultipleDeletions to handle
the deletion properly
demo:
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;
CREATE POLICY p1 ON ts USING (ts >= ROW(1,1,1));
CREATE POLICY p2 ON ts USING (ts.a = 1);
ALTER TABLE ts DROP COLUMN a CASCADE;
will drop above all indexes, constraints and policies on the table ts.
Attachments:
v2-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchtext/x-patch; charset=US-ASCII; name=v2-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchDownload+298-1
On Sep 9, 2025, at 11:12, jian he <jian.universality@gmail.com> wrote:
hi.
I found a new way to solve this problem.CREATE TABLE ts (a int, c int, b int
constraint cc check((ts = ROW(1,1,1))),
constraint cc1 check((ts.a = 1)));ALTER TABLE ts DROP COLUMN a CASCADE;
will drop above all indexes, constraints and policies on the table ts.
<v2-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patch>
I agree we should delete those constraints and indices on the whole row, otherwise, with cc (ts=ROW(1,1,1)), once a column is dropped, it won’t be able to insert data anymore:
```
evantest=# insert into ts values (2, 3);
ERROR: new row for relation "ts" violates check constraint "cc"
DETAIL: Failing row contains (2, 3).
evantest=# insert into ts values (1, 1);
ERROR: cannot compare record types with different numbers of columns
```
But v2 needs a rebase, I cannot apply it to master.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Thu, Sep 11, 2025 at 9:27 AM Chao Li <li.evan.chao@gmail.com> 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.
Attachments:
v3-0002-disallow-ALTER-COLUMN-SET-DATA-TYPE-when-wholerow-referenced-cons.patchtext/x-patch; charset=US-ASCII; name=v3-0002-disallow-ALTER-COLUMN-SET-DATA-TYPE-when-wholerow-referenced-cons.patchDownload+61-1
v3-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchtext/x-patch; charset=US-ASCII; name=v3-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchDownload+190-1
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.
<v3-0002-disallow-ALTER-COLUMN-SET-DATA-TYPE-when-wholerow-referenced-cons.patch><v3-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patch>
1 - 0001
```
+ * ALTER TABLE DROP COLUMN also need drop indexes or constraints that
```
Nit: need -> needs to
2 - 0001
```
+ tmpobject.classId = RelationRelationId;
+ tmpobject.objectId = RelationGetRelid(rel);
+ tmpobject.objectSubId = attnum;
```
Originally “object” points to the column to delete, but with is patch, you are using “object” for index/constrain to delete and “tmpobject” for the column to delete, which could be misleading.
I’d suggest keep the meaning of “object” unchanged, you need to pull up of initialization of “object” to the place now you initiate “tmpobject”. And only define “tmpobject” in sections where it is needed. So you can name it “idxObject” or “consObject”, which will be more clearly meaning. I think you may also rename “object” to “colObject”.
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.
4 - 0001
```
+ if (!heap_attisnull(indexTuple, Anum_pg_index_indpred, NULL))
+ {
….
+ }
+
+ if (!found_whole_row && !heap_attisnull(indexTuple, Anum_pg_index_indexprs, NULL))
+ {
…
+ }
```
These two pieces of code are exactly the same expect operating different Anum_pg_index_indpred/indexprs. I think we can create a static function to avoid duplicate code.
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()?
6 - 0002
```
+ 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)),
```
I think the second relation name is quite duplicate. We can just say “because constraint “xx” uses whole-row type".
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Mon, Sep 15, 2025 at 11:48 AM Chao Li <li.evan.chao@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.
Attachments:
v4-0003-disallow-change-or-drop-column-when-wholerow-referenced-policy-ex.patchtext/x-patch; charset=US-ASCII; name=v4-0003-disallow-change-or-drop-column-when-wholerow-referenced-policy-ex.patchDownload+181-2
v4-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchtext/x-patch; charset=US-ASCII; name=v4-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchDownload+216-4
v4-0002-disallow-ALTER-COLUMN-SET-DATA-TYPE-when-wholerow-referenced-cons.patchtext/x-patch; charset=US-ASCII; name=v4-0002-disallow-ALTER-COLUMN-SET-DATA-TYPE-when-wholerow-referenced-cons.patchDownload+69-1
On Mon, Sep 15, 2025 at 8:40 PM jian he <jian.universality@gmail.com> wrote:
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.
in v4, I use
+ TupleConstr *constr = RelationGetDescr(rel)->constr;
+
+ if (constr && constr->num_check > 0)
+{
+ systable_beginscan
+}
to check if a relation's check constraint expression contains a whole-row or
not. however this will have multiple systable_beginscan if multiple check
constraints contain wholerow expr.
I changed it to systable_beginscan pg_constraint once and check if the scan
returned pg_constraint tuple meets our condition or not.
and some minor adjustments to regression tests.
Attachments:
v5-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchtext/x-patch; charset=US-ASCII; name=v5-0001-ALTER-TABLE-DROP-COLUMN-drop-wholerow-referenced-object.patchDownload+220-4
v5-0002-disallow-ALTER-COLUMN-SET-DATA-TYPE-when-wholerow-referenced-cons.patchtext/x-patch; charset=US-ASCII; name=v5-0002-disallow-ALTER-COLUMN-SET-DATA-TYPE-when-wholerow-referenced-cons.patchDownload+76-1
v5-0003-disallow-change-or-drop-column-when-wholerow-referenced-policy-ex.patchtext/x-patch; charset=US-ASCII; name=v5-0003-disallow-change-or-drop-column-when-wholerow-referenced-policy-ex.patchDownload+198-3
Hi,
I have consolidated the work into two patches.
0001 handles indexes and CHECK constraints that contain whole-row references.
0002 handles policies that contain whole-row references.
The difference is that, for policy objects, we cannot use pull_varattnos to find
whole-row references, since we need recurse to Sublink node, Also, a policy’s
whole-row reference may point to an arbitrary relation, while index, check
constraint can only reference the relation it is associated with.
so the previous v5-0003 scans pg_policy.polrelid to find out whether it's safe
to drop one relation is wrong, we should use pg_depend.
summary:
For objects (indexes, constraints, policies) that contain whole-row references:
ALTER TABLE DROP COLUMN will drop these objects too.
ALTER COLUMN SET DATA TYPE will error out, saying that the data type cannot be
changed because whole-row–dependent objects exist.
Attachments:
v6-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchtext/x-patch; charset=UTF-8; name=v6-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchDownload+327-4
v6-0002-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patchtext/x-patch; charset=US-ASCII; name=v6-0002-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patchDownload+251-4
hi.
CREATE FUNCTION dummy_trigger() RETURNS TRIGGER AS $$
BEGIN
RETURN NULL;
END
$$ language plpgsql;
create table main_table(a int);
CREATE TRIGGER before_ins_stmt_trig BEFORE INSERT ON main_table
FOR EACH ROW
WHEN (new.a > 0)
EXECUTE PROCEDURE dummy_trigger();
ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE INT8; --error
ALTER TABLE main_table DROP COLUMN a; --error
Dropping a column or changing its data type will fail if the column is
referenced in a trigger’s WHEN clause, that's the current behavior.
I think we should expand that to a whole-row reference WHEN clause in trigger.
DROP TRIGGER before_ins_stmt_trig ON main_table;
CREATE TRIGGER before_ins_stmt_trig BEFORE INSERT ON main_table
FOR EACH ROW
WHEN (new is null)
EXECUTE PROCEDURE dummy_trigger();
ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE INT8; --expect to error
ALTER TABLE main_table DROP COLUMN a; --expect to error
new summary:
For (constraints, indexes, policies, triggers) that contain whole-row
references:
ALTER TABLE DROP COLUMN [CASCADE] will drop these objects too.
ALTER COLUMN SET DATA TYPE will error out because whole-row–dependent objects
exist.
Attachments:
v7-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchapplication/x-patch; name=v7-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchDownload+317-4
v7-0003-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patchapplication/x-patch; name=v7-0003-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patchDownload+228-3
v7-0002-disallow-drop-or-change-column-if-wholerow-trigger-exists.patchapplication/x-patch; name=v7-0002-disallow-drop-or-change-column-if-wholerow-trigger-exists.patchDownload+101-1
On Sat, 27 Dec 2025 at 08:01, jian he <jian.universality@gmail.com> wrote:
hi.
CREATE FUNCTION dummy_trigger() RETURNS TRIGGER AS $$
BEGIN
RETURN NULL;
END
$$ language plpgsql;create table main_table(a int);
CREATE TRIGGER before_ins_stmt_trig BEFORE INSERT ON main_table
FOR EACH ROW
WHEN (new.a > 0)
EXECUTE PROCEDURE dummy_trigger();ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE INT8; --error
ALTER TABLE main_table DROP COLUMN a; --errorDropping a column or changing its data type will fail if the column is
referenced in a trigger’s WHEN clause, that's the current behavior.
I think we should expand that to a whole-row reference WHEN clause in trigger.DROP TRIGGER before_ins_stmt_trig ON main_table;
CREATE TRIGGER before_ins_stmt_trig BEFORE INSERT ON main_table
FOR EACH ROW
WHEN (new is null)
EXECUTE PROCEDURE dummy_trigger();
ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE INT8; --expect to error
ALTER TABLE main_table DROP COLUMN a; --expect to errornew summary:
For (constraints, indexes, policies, triggers) that contain whole-row
references:
ALTER TABLE DROP COLUMN [CASCADE] will drop these objects too.ALTER COLUMN SET DATA TYPE will error out because whole-row–dependent objects
exist.
Hi!
I did take a look at v7.
In 0001:
+ indscan = systable_beginscan(pg_index, IndexIndrelidIndexId, true, + NULL, 1, &skey); + while (HeapTupleIsValid(htup = systable_getnext(indscan))) + { + Form_pg_index index = (Form_pg_index) GETSTRUCT(htup); + + /* add index's OID to result list */ + indexlist = lappend_oid(indexlist, index->indexrelid); + } + systable_endscan(indscan); + + table_close(pg_index, AccessShareLock); + + foreach_oid(indexoid, indexlist) + {
Hmm, why is this not just a one cycle? Also, not sure how many
relations can be returned by pg_index scan. Maybe it is worth adding
CHECK_FOR_INTERRUPTS() here?
--
Best regards,
Kirill Reshke
At 2026-01-19 17:09:54, "jian he" <jian.universality@gmail.com> wrote:
hi.
CREATE FUNCTION dummy_trigger() RETURNS TRIGGER AS $$
BEGIN
RETURN NULL;
END
$$ language plpgsql;create table main_table(a int);
CREATE TRIGGER before_ins_stmt_trig BEFORE INSERT ON main_table
FOR EACH ROW
WHEN (new.a > 0)
EXECUTE PROCEDURE dummy_trigger();ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE INT8; --error
ALTER TABLE main_table DROP COLUMN a; --errorDropping a column or changing its data type will fail if the column is
referenced in a trigger’s WHEN clause, that's the current behavior.
I think we should expand that to a whole-row reference WHEN clause in trigger.DROP TRIGGER before_ins_stmt_trig ON main_table;
CREATE TRIGGER before_ins_stmt_trig BEFORE INSERT ON main_table
FOR EACH ROW
WHEN (new is null)
EXECUTE PROCEDURE dummy_trigger();
ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE INT8; --expect to error
ALTER TABLE main_table DROP COLUMN a; --expect to errornew summary:
For (constraints, indexes, policies, triggers) that contain whole-row
references:
ALTER TABLE DROP COLUMN [CASCADE] will drop these objects too.ALTER COLUMN SET DATA TYPE will error out because whole-row–dependent objects
exist.
Hello,
I did take a look at v7-0001 and v7-0002.
In v7-0002:
1.
+ pull_varattnos(expr, PRS2_OLD_VARNO, &expr_attrs); + pull_varattnos(expr, PRS2_NEW_VARNO, &expr_attrs);
The function "pull_varattnos" was called twice, with different varno parameters each time (first using PRS2_NEW_VARNO, then using PRS2_OLD_VARNO).
I think it would be better to use PRS2_NEW_VARNO | PRS2_OLD_VARNO in a single call.
pull_varattnos(expr, PRS2_NEW_VARNO | PRS2_OLD_VARNO, &expr_attrs);
2.
+ if (trig->tgqual != NULL)
Should we first check if TRIGGER_FOR_ROW(trig->tgtype) before processing trig->tgqual are more appropriate?
3.
Another issue is the Bitmapset set allocated by pull_varattnos will never be released. This will cause a memory leak.
Please add the statement bms_free(expr_attrs); after each usage within the loops in recordWholeRowDependencyOnOrError.
In v7-0001:
As in v7-0002, the Bitmapset returned by pull_varattnos is never released.
Please add the statement bms_free(expr_attrs);
Regards,
Jinbinge
On Mon, Jan 19, 2026 at 2:56 AM Kirill Reshke <reshkekirill@gmail.com> wrote:
Hi!
I did take a look at v7.In 0001:
+ indscan = systable_beginscan(pg_index, IndexIndrelidIndexId, true, + NULL, 1, &skey); + while (HeapTupleIsValid(htup = systable_getnext(indscan))) + { + Form_pg_index index = (Form_pg_index) GETSTRUCT(htup); + + /* add index's OID to result list */ + indexlist = lappend_oid(indexlist, index->indexrelid); + } + systable_endscan(indscan); + + table_close(pg_index, AccessShareLock); + + foreach_oid(indexoid, indexlist) + {Hmm, why is this not just a one cycle? Also, not sure how many
relations can be returned by pg_index scan. Maybe it is worth adding
CHECK_FOR_INTERRUPTS() here?
I refactored this part, I realized that v7 has some duplicated code in
recordWholeRowDependencyOnOrError.
After refactoring, CHECK_FOR_INTERRUPTS is not needed, I think.
On Mon, Jan 19, 2026 at 6:00 PM 金 <jinbinge@126.com> wrote:
Hello,
I did take a look at v7-0001 and v7-0002.
In v7-0002:
1.
+ pull_varattnos(expr, PRS2_OLD_VARNO, &expr_attrs); + pull_varattnos(expr, PRS2_NEW_VARNO, &expr_attrs);The function "pull_varattnos" was called twice, with different varno parameters each time (first using PRS2_NEW_VARNO, then using PRS2_OLD_VARNO).
I think it would be better to use PRS2_NEW_VARNO | PRS2_OLD_VARNO in a single call.
pull_varattnos(expr, PRS2_NEW_VARNO | PRS2_OLD_VARNO, &expr_attrs);
We are checking if the trig->tgqual expression referenced Var->varno
equals PRS2_NEW_VARNO or PRS2_NEW_VARNO.
PRS2_NEW_VARNO | PRS2_OLD_VARNO is equal to 3,
So, I don't think it will work, see pull_varattnos_walker.
2.
+ if (trig->tgqual != NULL)
Should we first check if TRIGGER_FOR_ROW(trig->tgtype) before processing trig->tgqual are more appropriate?
I see.
However, unconditionally checking the trigger's WHEN clause whole-row
references seems better, IMHO.
3.
Another issue is the Bitmapset set allocated by pull_varattnos will never be released. This will cause a memory leak.
Please add the statement bms_free(expr_attrs); after each usage within the loops in recordWholeRowDependencyOnOrError.In v7-0001:
As in v7-0002, the Bitmapset returned by pull_varattnos is never released.
Please add the statement bms_free(expr_attrs);
Yech, In this case, we need ``bms_free(expr_attrs);`` because in the function
recordWholeRowDependencyOnOrError, we use the variable `expr_attrs` constantly.
Attachments:
v8-0002-disallow-drop-or-change-column-if-wholerow-trigger-exists.patchtext/x-patch; charset=US-ASCII; name=v8-0002-disallow-drop-or-change-column-if-wholerow-trigger-exists.patchDownload+101-1
v8-0003-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patchtext/x-patch; charset=US-ASCII; name=v8-0003-disallow-ALTER-TABLE-ALTER-COLUMN-when-wholerow-referenced-policy.patchDownload+223-3
v8-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchtext/x-patch; charset=UTF-8; name=v8-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchDownload+310-4
Hi,
I tested the latest v8-0001 patch on current master and wanted to share my
observations.
Before applying the patch, when dropping a column, constraints and indexes
that referenced the whole row (for example, CHECK (ts = ROW(...)) or
expressions using ts) were not removed. This left the table in an
inconsistent state and resulted in errors during inserts.
After applying the patch, these objects are correctly detected and removed
when the column is dropped. The table remains clean, and inserts work as
expected.
I also tried a few additional scenarios:
-
DROP COLUMN with CASCADE—behaved as expected, no leftover objects
-
normal column-level constraints — still handled correctly (no regression)
-
multiple whole-row constraints — all removed properly
-
ALTER COLUMN TYPE — correctly throws an error when a whole-row
constraint exists
Overall, the behavior looks correct and consistent based on these tests.
Thanks for working on this!
lakshmi
Hi.
[1]: https://commitfest.postgresql.org/patch/6755
this thread addresses whole-row dependencies for ALTER TABLE DROP
COLUMN and ALTER COLUMN SET DATA TYPE.
Overall, the attached v9 doesn't include any major changes.
It just contains some refactoring to make the coding style consistent with [1]https://commitfest.postgresql.org/patch/6755.
Attachments:
t52253_14v9-0003-fix-DDL-wholerow-referenced-policies.patchtext/x-patch; charset=US-ASCII; name=v9-0003-fix-DDL-wholerow-referenced-policies.patchDownload+232-3
v9-0002-fix-DDL-wholerow-referenced-triggers.patchtext/x-patch; charset=US-ASCII; name=v9-0002-fix-DDL-wholerow-referenced-triggers.patchDownload+107-1
v9-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchtext/x-patch; charset=UTF-8; name=v9-0001-fix-DDL-wholerow-referenced-constraints-and-indexes.patchDownload+344-1
On Mon, May 25, 2026 at 2:43 PM jian he <jian.universality@gmail.com> wrote:
Overall, the attached v9 doesn't include any major changes.
It just contains some refactoring to make the coding style consistent with [1].
Hi.
Rebase because of
https://git.postgresql.org/cgit/postgresql.git/commit/?id=a4639d64e2199885f8e995395b6fe874cb7228bf
I also simplified the code a little bit, polished the comments, and
merged 3 patches into one, no major changes.
Below is the commit message:
Subject: [PATCH v14 1/1] Whole-row fixes for DROP COLUMN, SET COLUMN DATA TYPE
ALTER TABLE DROP COLUMN should remove indexes or constraints contain whole-row
references, just like non-whole-row column.
ALTER TABLE DROP COLUMN should fail if a trigger WHEN clause or row-level
security policy contains a whole-row reference. To do this, record a dependency
between the relation and the trigger or policy in
RememberWholeRowDependentForRebuilding; performMultipleDeletions then handles
the deletion checks.
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: /messages/by-id/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com
commitfest: https://commitfest.postgresql.org/patch/6055
----------------------------------------------------------------------------------------------
Hi Jian,
I tested the latest v14 patch on current master.Everything worked as
expected in my testing. I verified that whole-row referenced constraints
and indexes are handled correctly when dropping a column, and that ALTER
COLUMN SET DATA TYPE still reports an error when whole-row dependent
objects exist. I also tested the newly merged trigger and row-level
security policy cases. ALTER TABLE ... DROP COLUMN correctly reports
dependency errors when a whole-row reference exists, and using CASCADE
removes the dependent trigger or policy as expected. I also verified that
the remaining table definition is correct after the operation, and I didn't
notice any unexpected behavior during testing.
Thanks for the update.
regards
lakshmi
On Tue, 14 Jul 2026 at 06:16, jian he <jian.universality@gmail.com> wrote:
On Mon, May 25, 2026 at 2:43 PM jian he <jian.universality@gmail.com> wrote:
Overall, the attached v9 doesn't include any major changes.
It just contains some refactoring to make the coding style consistent with [1].Hi.
Rebase because of
https://git.postgresql.org/cgit/postgresql.git/commit/?id=a4639d64e2199885f8e995395b6fe874cb7228bf
I also simplified the code a little bit, polished the comments, and
merged 3 patches into one, no major changes.Below is the commit message:
Subject: [PATCH v14 1/1] Whole-row fixes for DROP COLUMN, SET COLUMN DATA TYPEALTER TABLE DROP COLUMN should remove indexes or constraints contain whole-row
references, just like non-whole-row column.ALTER TABLE DROP COLUMN should fail if a trigger WHEN clause or row-level
security policy contains a whole-row reference. To do this, record a dependency
between the relation and the trigger or policy in
RememberWholeRowDependentForRebuilding; performMultipleDeletions then handles
the deletion checks.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.
I think the current suggested approach with scanning
pg_index/pg_constraint/etc. is horrible for performance, as it uses
O(total_dependent_objects) to figure out who has whole-row references,
rather than just O(n_whole_row_dependencies). I think the better
approach is to properly register whole-row Vars in pg_depends as their
own objsubid, allowing invalidation of the dependencies whenever the
whole-row definition changes.
Additionally, I don't think that dropping whole-row indexes is proper
when the column is dropped or changes data type; the index definition
is still correct, it just needs to be rebuilt. See my fix for this
class of issues at [0]/messages/by-id/CAEze2WjDaDyvztdXh3Cb2J=11CMVRp4NWAW0E6dBRKa1T9w5ag@mail.gmail.com (cf [1]https://commitfest.postgresql.org/patch/7071/, it implements the approach I
described.
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
[0]: /messages/by-id/CAEze2WjDaDyvztdXh3Cb2J=11CMVRp4NWAW0E6dBRKa1T9w5ag@mail.gmail.com
[1]: https://commitfest.postgresql.org/patch/7071/