Re: [HACKERS] Restrict concurrent update/delete with UPDATE of partition key

On 8 March 2018 at 09:15, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> wrote:
> For example, with your patches applied:
>
> CREATE TABLE pa_target (key integer, val text)
> PARTITION BY LIST (key);
> CREATE TABLE part1 PARTITION OF pa_target FOR VALUES IN (1);
> CREATE TABLE part2 PARTITION OF pa_target FOR VALUES IN (2);
> INSERT INTO pa_target VALUES (1, 'initial1');
>
> session1:
> BEGIN;
> UPDATE pa_target SET val = val || ' updated by update1' WHERE key = 1;
> UPDATE 1
> postgres=# SELECT * FROM pa_target ;
> key | val
> -----+-----------------------------
> 1 | initial1 updated by update1
> (1 row)
>
> session2:
> UPDATE pa_target SET val = val || ' updated by update2', key = key + 1 WHERE
> key = 1
> <blocks>
>
> session1:
> postgres=# COMMIT;
> COMMIT
>
> <session1 unblocks and completes its UPDATE>
>
> postgres=# SELECT * FROM pa_target ;
> key | val
> -----+-----------------------------
> 2 | initial1 updated by update2
> (1 row)
>
> Ouch. The committed updates by session1 are overwritten by session2. This
> clearly violates the rules that rest of the system obeys and is not
> acceptable IMHO.
>
> Clearly, ExecUpdate() while moving rows between partitions is missing out on
> re-constructing the to-be-updated tuple, based on the latest tuple in the
> update chain. Instead, it's simply deleting the latest tuple and inserting a
> new tuple in the new partition based on the old tuple. That's simply wrong.

You are right. This need to be fixed. This is a different issue than
the particular one that is being worked upon in this thread, and both
these issues have different fixes.

Like you said, the tuple needs to be reconstructed when ExecDelete()
finds that the row has been updated by another transaction. We should
send back this information from ExecDelete() (I think tupleid
parameter gets updated in this case), and then in ExecUpdate() we
should goto lreplace, so that the the row is fetched back similar to
how it happens when heap_update() knows that the tuple was updated.

--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company