Re: let ALTER TABLE DROP COLUMN drop whole-row referenced object

At 2026-01-19 17:09:54, "jian he" <jian(dot)universality(at)gmail(dot)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; --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.
>
>

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