Re: Rename of triggers for partitioned tables

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Arne Roland <A(dot)Roland(at)index(dot)de>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Rename of triggers for partitioned tables
Date: 2021-07-22 17:41:33
Message-ID: 202107221741.6gb46lytggyj@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021-Jul-22, Arne Roland wrote:

> I just noticed that apparently the
> ALTER TABLE [ IF EXISTS ] [ ONLY ] name [ ENABLE | DISABLE ] TRIGGER ...
> syntax albeit looking totally different and it already recurses, it
> has precisely the same issue with pg_dump. In that case the ONLY
> syntax isn't optional and the 2. from your above post does inevitably
> apply.
>
> Since it is sort of the same problem, I think it might be worthwhile
> to address it as well within this patch. Adding two to four ereports
> doesn't sound like scope creeping to me, even though it touches
> completely different code. I'll look into that as well.

Oh! For some reason I failed to notice that you were talking about
ENABLE / DISABLE, not RENAME anymore. It turns out that I recently
spent some time in this area; see commits below (these are from branch
REL_11_STABLE). Are you talking about something that need to be handled
*beyond* these fixes?

commit ccfc3cbb341abf515d930097c4851d9e2152504f
Author: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> [Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>]
AuthorDate: Fri Jul 16 17:29:22 2021 -0400
CommitDate: Fri Jul 16 17:29:22 2021 -0400

Fix pg_dump for disabled triggers on partitioned tables

pg_dump failed to preserve the 'enabled' flag (which can be not only
disabled, but also REPLICA or ALWAYS) for partitions which had it
changed from their respective parents. Attempt to handle that by
including a definition for such triggers in the dump, but replace the
standard CREATE TRIGGER line with an ALTER TRIGGER line.

Backpatch to 11, where these triggers can exist. In branches 11 and 12,
pick up a few test lines from commit b9b408c48724 to verify that
pg_upgrade is okay with these arrangements.

Co-authored-by: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Co-authored-by: Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com

commit fed35bd4a65032f714af6bc88c102d0e90422dee
Author: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> [Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>]
AuthorDate: Fri Jul 16 13:01:43 2021 -0400
CommitDate: Fri Jul 16 13:01:43 2021 -0400

Preserve firing-on state when cloning row triggers to partitions

When triggers are cloned from partitioned tables to their partitions,
the 'tgenabled' flag (origin/replica/always/disable) was not propagated.
Make it so that the flag on the trigger on partition is initially set to
the same value as on the partitioned table.

Add a test case to verify the behavior.

Backpatch to 11, where this appeared in commit 86f575948c77.

Author: Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Reported-by: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com

commit bbb927b4db9b3b449ccd0f76c1296de382a2f0c1
Author: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> [Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>]
AuthorDate: Tue Oct 20 19:22:09 2020 -0300
CommitDate: Tue Oct 20 19:22:09 2020 -0300

Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion

More precisely, correctly handle the ONLY flag indicating not to
recurse. This was implemented in 86f575948c77 by recursing in
trigger.c, but that's the wrong place; use ATSimpleRecursion instead,
which behaves properly. However, because legacy inheritance has never
recursed in that situation, make sure to do that only for new-style
partitioning.

I noticed this problem while testing a fix for another bug in the
vicinity.

This has been wrong all along, so backpatch to 11.

Discussion: https://postgr.es/m/20201016235925.GA29829@alvherre.pgsql

--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Arne Roland 2021-07-22 17:45:49 Re: Rename of triggers for partitioned tables
Previous Message Alvaro Herrera 2021-07-22 17:33:10 Re: Rename of triggers for partitioned tables