Re: BEFORE trigger can cause undetected partition constraint violation

On 2017/06/02 10:36, Robert Haas wrote:
> On Thu, Jun 1, 2017 at 6:05 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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