Date: Tue, 20 Oct 2020 21:54:53 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@postgresql.org,
	Amit Langote <amitlangote09@gmail.com>
Subject: Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for
 inherited row triggers
Message-ID: <20201021005453.GA18765@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201020220116.GH9241@telsasoft.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Oct-20, Justin Pryzby wrote:

> On Tue, Oct 20, 2020 at 03:56:30PM -0400, Tom Lane wrote:
> > Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > > Hmm, next question: should we backpatch a fix for this?  (This applies
> > > all the way back to 11.)  If we do, then we would change behavior of
> > > partition creation.  It's hard to see that the current behavior is
> > > desirable ... and I think anybody who would have come across this, would
> > > wish it behaved the other way.  But still -- it would definitely be a
> > > behavior change.
> > 
> > The behavior change seems like it'd be an improvement in a vacuum,
> > but I wonder how it would interact with catalog contents left behind
> > by the old misbehavior.  Also, would we expect pg_dump to try to do
> > anything to clean up the mess?  If so, allowing a back branch to have
> > had more than one behavior would complicate that greatly.
> 
> I don't think there's a problem with catalog content ?
> I think it's fine if there's an enabled child trigger inheriting from a
> disabled parent?  This changes the initial tgenabled for new partitions.

I don't think we'd need to do anything special here ... particularly
considering the discovery that pg_dump does not preserve the disable
status of trigger on partitions:

> However...it looks like pg_dump should ALTER the child trigger state if it
> differ from its parent.  Or maybe it needs to CREATE child triggers with the
> proper state before attaching the child table ?

I guess *something* needs to be done, but I'm not clear on what it is.
Creating the trigger on partition beforehand does not work: an error is
raised on attach that the trigger already exists.

The only way I see to do this, is to have pg_dump extract tgenabled for
all child triggers that doesn't have the same tgenabled as the parent,
and append ALTER .. DISABLE commands for each one to the parent table
trigger creation command.  So pg_dump.c's getTriggers would have to
obtain the list of altered child triggers, and then dumpTrigger would
have to append the ALTER TABLE ONLY <partition> .. ENABLE/DISABLE
command for that particular trigger.