Remove redundant MultiXactIdIsRunning() check in HeapTupleSatisfiesUpdate()
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254111psql -h localhost -U postgresBuilt from patchset v4 (message #4), October 24, 2026 at 01:55 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t254111_4 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t254111_4 && git checkout t254111_4Patchset v4 (message #4) is on t254111_4
Hi,
The patch didn't come through as an attachment in my previous mail, attaching it again.
Regards,
Zhao Song
On Oct 8, 2026, at 23:54, Zhao Song <songzhao.asm@icloud.com> wrote:
Hi,
I noticed a redundant MultiXactIdIsRunning() check in HeapTupleSatisfiesUpdate(), when xmax is a multixact containing an update (the HEAP_XMAX_IS_MULTI branch in heapam_visibility.c).
The relevant code is:
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
return TM_BeingModified;if (TransactionIdDidCommit(xmax))
{ ... return TM_Updated / TM_Deleted; }/*
* By here, the update in the Xmax is either aborted or crashed, but
* what about the other members?
*/if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
{
SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId);
return TM_Ok;
}
else
{
/* There are lockers running */
return TM_BeingModified;
}The else branch can't be reached. The first MultiXactIdIsRunning() call already returns if any member, updater or locker, is still running. Between the two calls, we only check TransactionIdDidCommit(xmax). Since the members of a MultiXactId never change (as the comment in MultiXactIdIsRunning() says, "it is not legal to add members to an existing MultiXactId"), and a member that wasn't running at the first check can't become running again, the second call must always return false.
I went through the commit history to understand why this check exists:
* 1ce150b7bb1 added the second check and changed the first one to TransactionIdIsInProgress(xmax), which only checked the updater.
* 07aeb1fec57 added the else branch to handle the case where lockers were still running.
* 05315498012 changed the first check back to MultiXactIdIsRunning(..., false), covering all members again.So since 05315498012, the second check is redundant and its else branch unreachable.
The attached patch removes the second MultiXactIdIsRunning() call and the unreachable branch. This should not change any behavior, but avoids an unnecessary multixact member lookup and proc array scan when the updater has aborted, and makes the code easier to understand.
Regards,
Zhao Song
<v1-0001-Remove-redundant-MultiXactIdIsRunning-check-in-He.patch>
I think your analysis is correct. The "if (TransactionIdDidCommit(xmax))" branch cannot change the result of MultiXactIdIsRunning(), so the later check is redundant.
But I think we should delete the “By here …” comment and retain the “There's no member …” comment because it explains why marking xmax invalid is safe.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
I think your analysis is correct. The "if (TransactionIdDidCommit(xmax))" branch cannot change the result of MultiXactIdIsRunning(), so the later check is redundant.
But I think we should delete the “By here …” comment and retain the “There's no member …” comment because it explains why marking xmax invalid is safe.
Thanks for the review.
Agreed, the "There's no member ..." comment now answers the question the old one asked. v2 attached.
Regards,
Zhao Song