Re: BUG #17792: MERGE uses uninitialized pointer and crashes when target tuple is updated concurrently

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: exclusion(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #17792: MERGE uses uninitialized pointer and crashes when target tuple is updated concurrently
Date: 2023-02-14 19:19:37
Message-ID: 20230214191937.sri5zsqqogpidmxp@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On 2023-Feb-14, Dean Rasheed wrote:

> After trying to induce that, I realised that it doesn't appear to be
> possible, because a delete after a failed update will always succeed,
> because it has the target row locked by that point. So I think that it
> will never need to retry more than once.

Hmm, yeah, that makes sense. Thanks for checking.

> That said, it seems wrong to be checking cpUpdateRetrySlot after an
> attempted delete anyway.

True.

> Perhaps a better fix would be to just change
> the check in ExecMergeMatched() to
>
> if (commandType == CMD_UPDATE && !TupIsNull(context->cpUpdateRetrySlot))
> goto lmerge_matched;
>
> and update the preceding comment, since only an update should set
> cpUpdateRetrySlot.

I agree, this looks to be a good fix. However, I couldn't in a quick
try reproduce the problem, so I haven't been able to verify it. I'll
try to do that early tomorrow.

(I also delete the XXX comment there.)

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end." (2nd Commandment for C programmers)

Attachment Content-Type Size
merge-uninit-2.patch text/x-diff 2.0 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Andres Freund 2023-02-14 21:04:59 Re: BUG #17791: Assert on procarray.c
Previous Message Francisco Olarte 2023-02-14 16:35:03 Re: BUG #17793: Query with large number of joins crashes PostgreSQL