public inbox for [email protected]  

help / color / mirror / Atom feed

From: =?GBK?B?vfAg?= <[email protected]>
To: jian he <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: let ALTER TABLE DROP COLUMN drop whole-row referenced object
Date: Mon, 19 Jan 2026 17:59:48 +0800 (CST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CACJufxHvZbH6p+h1HSQXn6D2aVNP+xg5WQedYSaTuBkbd2jHdg@mail.gmail.com>
References: <CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com>
	<CACJufxFMqpraCreGKTWWB7cyPAYP+yehwTMFm-CSx+rzWdHf+A@mail.gmail.com>
	<[email protected]>
	<CACJufxHmmsg7GTD_LpGGam09+Aq4KkYHe2dfPgtGjrUELqbK=w@mail.gmail.com>
	<[email protected]>
	<CACJufxGfNBFtziA3V=j9qY4UAVd2KfkT7v0JRieb=gTegs6ofg@mail.gmail.com>
	<CACJufxE-PJyKmoDnZDEt+anKdtV1MvfA+u9x_S9CvorYUQ9Bww@mail.gmail.com>
	<CACJufxEApboeVKP4jdYOkikOij22HrHWnuSb=u-cJ7AHMBcOxA@mail.gmail.com>
	<CACJufxHvZbH6p+h1HSQXn6D2aVNP+xg5WQedYSaTuBkbd2jHdg@mail.gmail.com>

At 2026-01-19 17:09:54, "jian he" <[email protected]> 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

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: let ALTER TABLE DROP COLUMN drop whole-row referenced object
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox