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-22 23:52:12
Message-ID: 20180222235212.qcdznloyxepn2be3@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera wrote:

> 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 have written this, and it seems to work fine; it's attached.

Generally speaking, I like this better than my previously proposed
patch: having duplicate pg_trigger rows seems lame, in hindsight.

I haven't measured the performance loss, but we now scan pg_inherits
each time we build a relcache entry and relhastriggers=on. Can't be
good. In general, the pg_inherits stuff looks generally unnatural --
manually doing scans upwards (find parents) and downwards (find
partitions). It's messy and there are no nice abstractions.
Partitioning looks too much bolted-on still.

We could mitigate the performance loss to some extent by adding more to
RelationData. For example, a "is_partition" boolean would help: skip
searching pg_inherits for a relation that is not a partition. The
indexing patch already added some "has_superclass()" calls and they look
somewhat out of place. Also, we could add a syscache to pg_inherits.

Regarding making partitioning feel more natural, we could add some API
"list all ancestors", "list all descendents". Maybe I should have used
find_inheritance_children.

Some cutesy: scanning multiple parents looking for potential triggers
means the order of indexscan results no longer guarantees the correct
ordering. I had to add a qsort() there.

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

Attachment Content-Type Size
v4-0001-Allow-FOR-EACH-ROW-triggers-on-partitioned-tables.patch text/plain 57.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-02-22 23:59:58 Re: ERROR: left and right pathkeys do not match in mergejoin
Previous Message Robert Haas 2018-02-22 23:32:15 Re: Translations contributions urgently needed