Re: BUG #16794: BEFORE UPDATE FOR EACH ROW triggers on partitioned tables can break tuple moving UPDATEs

On 2021-Jan-26, Alvaro Herrera wrote:

> Looked at this today, and it's not nearly as easy to fix as I hoped.
> The misbehavior in corner cases seems weird enough that we should keep
> the restriction ... but the way the restriction is implemented currently
> is completely broken.

Actually, I was misled by the fact that I had both INSERT triggers and
UPDATE triggers, and they were both changing the partition keys.  What I
now think we should do is just remove the restrictions for UPDATE, and
let the trigger do whatever; and keep what we have for INSERT, because
it is useful.  Things work out okay.  As in the attached patch.  A quick
write-up:

In the UPDATE case, this is the timeline:

u0. The user-specified changes are carried out in the tuple.
u1. BEFORE UPDATE FOR EACH ROW triggers run, *in the original partition*.
    We don't know yet if the tuple is okay in this partition or not.
u2. The partition constraint (of the original partition) is tested.
u3. If the partition constraint returns OK, we're done.
... since the partition constraint failed, we have to route the tuple:
u4. a DELETE is executed in the original partition. Appropriate triggers run.
u5. ExecFindPartition determines the target partition
u6. an INSERT is executed in the new partition. Appropriate triggers run.

The originally claimed reason not to allow the trigger from moving the
row to another partition was that if we allowed that, then we would not
run the correct triggers. But that's wrong: because the UPDATE at step
u1 runs triggers in the original partition, not in the new one (which
would be determined only at u5) then that holds true for the original
update too, not just the changes in the before-row triggers.  So if the
triggers change the partkey columns ... it doesn't have any semantic
difference from the user changing the partkey columns.  We don't care.


As for INSERT -- it does not know how to handle rerouting.  So we're
only producing a better-quality error message by having the
same-partition check.  If we did not have the check, the only difference
is that ExecPartitionCheck would fail a bit later, with a mysterious
error that the tuple doesn't meet the partition constraint.

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
Tom: There seems to be something broken here.
Teodor: I'm in sackcloth and ashes...  Fixed.
        http://archives.postgresql.org/message-id/482D1632.8010507@sigaev.ru