Date: Wed, 7 Mar 2018 22:57:08 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
	Pg Hackers <pgsql-hackers@postgresql.org>,
	Simon Riggs <simon@2ndquadrant.com>
Subject: Re: FOR EACH ROW triggers on partitioned tables
Message-ID: <20180308015708.kc755cmaadpxgpyg@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180307171753.27xnxnjhlphur6bs@alvherre.pgsql>
User-Agent: NeoMutt/20170306-137-4415bd-dirty (1.8.0)
Precedence: bulk

Alvaro Herrera wrote:

> I reserve the right to revise this further, as I'm going to spend a
> couple of hours looking at it this afternoon, particularly to see how
> concurrent DDL behaves, but I don't see anything obviously wrong with
> it.

I do now.  TLDR; I'm afraid this cute idea crashed and burned, so I'm
back to the idea of just cloning the pg_trigger row for each partition.

The reason for the failure is pg_trigger->tgqual, which is an expression
tree.  In most cases, when set, that expression will contain references
to columns of the table, in the form of a varattno.  But this varattno
references the column number of the partitioned table; and if the
partition happens to require some attribute mapping, we're screwed
because there is no way to construct that without forming the
partitioned table's tuple descriptor.  But we can't do that without
grabbing a lock on the partitioned table; and we can't do that because
we would incur the deadlock risk Robert was talking about.

An example that causes the problem is:

create table parted_irreg (fd int, a int, fd2 int, b text) partition by range (b);
alter table parted_irreg drop column fd, drop column fd2;
create table parted1_irreg (b text, fd int, a int);
alter table parted1_irreg drop column fd;
alter table parted_irreg attach partition parted1_irreg for values from ('aaaa') to ('bbbb');
create trigger parted_trig after insert on parted_irreg for each row when (new.a % 1 = 0) execute procedure trigger_notice_irreg();
insert into parted_irreg values (1, 'aardvark');
insert into parted1_irreg values ('aardwolf', 2);
drop table parted_irreg;
drop function trigger_notice_irreg();

Both inserts fail thusly:

ERROR:  attribute 2 of type parted1_irreg has been dropped

Now, I can fix the first failure by taking advantage of
ResultRelInfo->ri_PartitionRoot during trigger execution; it's easy and
trouble-free to call map_variable_attnos() using that relation.  But in
the second insert, ri_PartitionRoot is null (because of inserting into
the partition directly), so we have no relation to refer to for
map_variable_attnos().  I think it gets worse: if you have a three-level
partitioning scheme, and define the trigger in the second one, there is
no relation either.

Another option I think would be to always keep in the trigger descriptor
("somehow"), an open Relation on which the trigger is defined.  But this
has all sorts of problems also, so I'm not doing that.

I guess another option is to store a column map somewhere.


So, unless someone has a brilliant idea on how to construct a column
mapping from partitioned table to partition, I'm going back to the
design I was proposing earlier, ie., creating individual pg_trigger rows
for each partition that are essentially adjusted copies of the ones for
the partitioned table.  The only missing thing in that one was having
ALTER TABLE ENABLE/DISABLE for a trigger on the partitioned table
cascade to the partitions; I'll see about that.

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