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

Alvaro Herrera <alvherre@alvh.no-ip.org>

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2023-02-14T19:19:37Z
Lists: pgsql-bugs

Attachments

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)

Commits

  1. Don't rely on uninitialized value in MERGE / DELETE