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

On 8 March 2018 at 12:34, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Mar 8, 2018 at 11:57 AM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>> On 8 March 2018 at 09:15, Pavan Deolasee <pavan.deolasee@gmail.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.
>>
>
> I also think that this is a bug in the original patch and won't be
> directly related to the patch being discussed.

Yes. Will submit a patch for this in a separate thread.





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