Re: [HACKERS] Restrict concurrent update/delete with UPDATE ofpartition key - Mailing list pgsql-hackers

From Pavan Deolasee
Subject Re: [HACKERS] Restrict concurrent update/delete with UPDATE ofpartition key
Date
Msg-id CABOikdNQpZPjL7Hp8pYfRBwkr7ojuENUagFquCm7sV=uz-DNjA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Restrict concurrent update/delete with UPDATE ofpartition key  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: [HACKERS] Restrict concurrent update/delete with UPDATE ofpartition key  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers


On Thu, Mar 8, 2018 at 12:31 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Mar 8, 2018 at 11:04 AM, Pavan Deolasee
<pavan.deolasee@gmail.com> wrote:
>
> On Tue, Feb 13, 2018 at 12:41 PM, amul sul <sulamul@gmail.com> wrote:
>>
>> Thanks for the confirmation, updated patch attached.
>>
>
> I am actually very surprised that 0001-Invalidate-ip_blkid-v5.patch does not
> do anything to deal with the fact that t_ctid may no longer point to itself
> to mark end of the chain. I just can't see how that would work.
>

I think it is not that patch doesn't care about the end of the chain.
 For example, ctid pointing to itself is used to ensure that for
deleted rows, nothing more needs to be done like below check in the
ExecUpdate/ExecDelete code path.

Yeah, but it only looks for places where it needs to detect deleted tuples and thus wants to throw an error. I am worried about other places where it is assumed that the ctid points to a valid looking tid, self or otherwise. I see no such places being either updated or commented.

Now may be there is no danger because of other protections in place, but it looks hazardous.
 

I have not tested, but it seems this could be problematic, but I feel
we could deal with such cases by checking invalid block id in the
above if check.  Another one such case is in EvalPlanQualFetch


Right.
 

> What happens if a partition key update deletes a row, but the operation is
> aborted? Do we need any special handling for that case?
>

If the transaction is aborted than future updates would update the
ctid to a new row, do you see any problem with it?

I don't know. May be there is none. But it needs to explained why it's not a problem.
 

> I am actually worried that we're tinkering with ip_blkid to handle one
> corner case of detecting partition key update. This is going to change
> on-disk format and probably need more careful attention. Are we certain that
> we would never require update-chain following when partition keys are
> updated?
>

I think we should never need update-chain following when the row is
moved from one partition to another partition, otherwise, we don't
change anything on the tuple.

I am not sure I follow. I understand that it's probably a tough problem to follow update chain from one partition to another. But why do you think we would never need that? What if someone wants to improve on the restriction this patch is imposing and actually implement partition key UPDATEs the way we do for regular tables i.e. instead of throwing error, we actually update/delete the row in the new partition?
 

> If so, can we think about some other mechanism which actually even
> leaves behind <new_partition, new_ctid>? I am not saying we should do that,
> but it warrants a thought.
>

Oh, this would much bigger disk-format change and need much more
thoughts, where will we store new partition information.

Yeah, but the disk format will change probably change just once. Or may be this can be done local to a partition table without requiring any disk format changes? Like adding a nullable hidden column in each partition to store the forward pointer?
 
Thanks,
Pavan

pgsql-hackers by date:

Previous
From: Amit Khandekar
Date:
Subject: Re: [HACKERS] Restrict concurrent update/delete with UPDATE ofpartition key
Next
From: Pavel Stehule
Date:
Subject: Re: INOUT parameters in procedures