Re: [HACKERS] BEFORE trigger can cause undetected partitionconstraint violation

On 2017/06/02 10:36, Robert Haas wrote:
> On Thu, Jun 1, 2017 at 6:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Without having checked the code, I imagine the reason for this is
>> that BEFORE triggers are fired after tuple routing occurs.
> 
> Yep.
> 
>> Re-ordering that seems problematic, because what if you have different
>> triggers on different partitions?  We'd have to forbid that, probably
>> by saying that only the parent table's BEFORE ROW triggers are fired,
>> but that seems not very nice.
> 
> The parent hasn't got any triggers; that's forbidden.
> 
>> The alternative is to forbid BEFORE triggers on partitions to alter
>> the routing result, probably by rechecking the partition constraint
>> after trigger firing.
> 
> That's what we need to do.  Until we do tuple routing, we don't know
> which partition we're addressing, so we don't know which triggers
> we're firing.  So the only way to prevent this is to recheck.  Which I
> think is supposed to work already, but apparently doesn't.

That has to do with the assumption written down in the following portion
of a comment in InitResultRelInfo():

    /*
     * If partition_root has been specified, that means we are building the
     * ResultRelInfo for one of its leaf partitions.  In that case, we need
     * *not* initialize the leaf partition's constraint, but rather the
     * partition_root's (if any).

which, as it turns out, is wrong.  It completely disregards the fact that
BR triggers are executed after tuple-routing and can change the row.

Attached patch makes InitResultRelInfo() *always* initialize the
partition's constraint, that is, regardless of whether insert/copy is
through the parent or directly on the partition.  I'm wondering if
ExecInsert() and CopyFrom() should check if it actually needs to execute
the constraint?  I mean it's needed if there exists BR insert triggers
which may change the row, but not otherwise.  The patch currently does not
implement that check.

Thanks,
Amit

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers