Re: BEFORE ROW triggers for partitioned tables

On 2020-Mar-11, Ashutosh Bapat wrote:

> On Thu, Feb 27, 2020 at 10:22 PM Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:

> > * The new function I added, ReportTriggerPartkeyChange(), contains one
> > serious bug (namely: it doesn't map attribute numbers properly if
> > partitions are differently defined).
> 
> IIUC the code in your patch, it seems you are just looking at
> partnatts. But partition key can contain expressions also which are
> stored in partexprs. So, I think the code won't catch change in the
> partition key values when it contains expression. Using
> RelationGetPartitionQual() will fix this problem and also problem of
> attribute remapping across the partition hierarchy.

Oh, of course.

In fact, I don't need to deal with PartitionQual directly; I can just
use ExecPartitionCheck(), since in ExecBRInsertTriggers et al we already
have all we need.  v2 attached.

Here's some example output.  With my previous patch, this was the error
we reported:

 insert into parted values (1, 1, 'uno uno v2');    -- fail
 ERROR:  changing partitioning columns in a before trigger is not supported
 DETAIL:  Column "a" was changed by trigger "t".

Now, passing emitError=true to ExecPartitionCheck, I get this:

 insert into parted values (1, 1, 'uno uno v2');    -- fail
 ERROR:  new row for relation "parted_1_1" violates partition constraint
 DETAIL:  Failing row contains (2, 1, uno uno v2).

Note the discrepancy in the table named in the INSERT vs. the one in the
error message.  This is a low-quality error IMO.  So I'd instead pass
emitError=false, and produce my own error.  It's useful to report the
trigger name and original partition name:

 insert into parted values (1, 1, 'uno uno v2');    -- fail
 ERROR:  moving row to another partition during a BEFORE trigger is not supported
 DETAIL:  Before trigger "t", row was to be in partition "public.parted_1_1"

Note that in this implementation I no longer know which column is the
problematic one, but I suppose users have clue enough.  Wording
suggestions welcome.

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