Feedback-ID: ie3de48e3:Fastmail
Date: Fri, 25 Sep 2026 16:56:58 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Thom Brown <thom@linux.com>
Cc: Antonin Houska <ah@cybertec.at>, shihao zhong <zhong950419@gmail.com>,
	Robert Treat <rob@xzilla.net>, Masahiko Sawada <sawada.mshk@gmail.com>,
	Manu <manuelreyesbravo@gmail.com>, pgsql-hackers@lists.postgresql.org
Subject: Re: REPACK (CONCURRENTLY) can silently lose updates when the toast
 table is rewritten
Message-ID: <araJcIXmBySfiXzc@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="nyjzce72b5mmvfal"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAA-aLv6SMuRVomk6yqh8i9hHF_iHYMZBYhR8dH6M-=x219KOSw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/araJcIXmBySfiXzc%40alvherre.pgsql>
Precedence: bulk


--nyjzce72b5mmvfal
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2026-Sep-25, Thom Brown wrote:

> Moving it into cluster_rel() makes sense, but does it need to be the
> full ShareUpdateExclusiveLock there?

Yeah, it's better to acquire the lock you want upfront, because
otherwise you introduce more risk of deadlock caused by lock upgrades
(admittedly the user would have to be doing something really stupid in
order for this to be a real problem, but still.)  It's only AEL that we
don't want to hold for long.

Maybe the patch could be somewhat like this, then?  I didn't review the
test carefully other than running without the code fix to verify that it
fails, and then passes with the fix; and I didn't read the commit
messages either, which I think are LLM-written and not really correct.
(Also, I would push both things as a single commit.)

I think changing the lock as obtained by copy_table_data is not very
nice, because that one is unconditional, and here we only want it in
concurrent mode.  BTW I noticed that the comment for copy_table_data
mentions decoding_ctx as an argument, which doesn't exist.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)

--nyjzce72b5mmvfal
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename=v4-0001-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch