Обсуждение: let ALTER TABLE DROP COLUMN drop whole-row referenced object

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], 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

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.

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.

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.

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.