Re: FOR EACH ROW triggers on partitioned tables

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: FOR EACH ROW triggers on partitioned tables
Date: 2018-02-14 21:26:24
Message-ID: 20180214212624.hm7of76flesodamf@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas wrote:
> On Tue, Jan 23, 2018 at 5:10 PM, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

> >> Also, does ALTER TABLE ... ENABLE/DISABLE TRIGGER do the right things on
> >> partitioned tables?
> >
> > Haven't done this yet, either. I like Simon's suggestion of outright
> > disallowing this.
>
> Why not just make it work?

I haven't had as much time to work on this as I wished, so progress has
been a bit slow. That is to say, this version is almost identical to
the one I last posted. I added a test for enable/disable trigger, which
currently fails because the code to support it is not implemented. I
added a report of the trigger name to the relevant test, for improved
visibility of what is happening. (I intend to push that one, since it's
a trivial improvement.)

Now one way to fix that would be to do as Amit suggests elsewhere, ie.,
to add a link to parent trigger from child trigger, so we can search for
children whenever the parent is disabled. We'd also need a new index on
that column so that the searches are fast, and perhaps a boolean flag
("trghaschildren") to indicate that searches must be done.
(We could add an array of children OID instead, but designwise that
seems much worse.)

Another option is to rethink this feature from the ground up: instead of
cloning catalog rows for each children, maybe we should have the trigger
lookup code, when running DML on the child relation (the partition),
obtain trigger entries not only for the child relation itself but also
for its parents recursively -- so triggers defined in the parent are
fired for the partitions, too. I'm not sure what implications this has
for constraint triggers.

The behavior should be the same, except that you cannot modify the
trigger (firing conditions, etc) on the partition individually -- it
works at the level of the whole partitioned table instead.

For foreign key triggers to work properly, I think I'd propose that this
occurs only for non-internal triggers. For internal triggers,
particularly FK triggers, we continue with the current approach in that
patch which is to create trigger clones.

This seems more promising to me.

Thoughts?

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
v3-0001-Mention-trigger-name-in-test.patch text/plain 5.3 KB
v3-0002-Allow-FOR-EACH-ROW-triggers-on-partitioned-tables.patch text/plain 33.8 KB
v3-0003-test-alter-table-enable-disable-trigger.patch text/plain 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-02-14 21:49:13 Re: tapeblocks is uninitialized in logtape.c
Previous Message Tom Lane 2018-02-14 21:19:33 Re: Cached/global query plans, autopreparation