From: Antonin Houska <ah@cybertec.at>
To: "Euler Taveira" <euler@eulerto.com>
cc: =?us-ascii?Q?=3D=3FUTF-8=3FQ=3F=3DC3=3D81lvaro=5FHerrera=3F=3D?=
 <alvherre@kurilemu.de>,
    pgsql-hackers@lists.postgresql.org
Subject: Re: Unexpected changes of CurrentResourceOwner and
 CurrentMemoryContext
In-reply-to: <267a39fd-8b8a-48b0-8f99-b4257482ac95@app.fastmail.com>
References: <202509111800.kp6xfq6pnke7@alvherre.pgsql>
 <267a39fd-8b8a-48b0-8f99-b4257482ac95@app.fastmail.com>
Comments: In-reply-to "Euler Taveira" <euler@eulerto.com>
   message dated "Thu, 11 Sep 2025 17:38:23 -0300."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Fri, 12 Sep 2025 08:46:18 +0200
Message-ID: <6242.1757659578@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/6242.1757659578%40localhost>
Precedence: bulk

Euler Taveira <euler@eulerto.com> wrote:

> On Thu, Sep 11, 2025, at 3:05 PM, =C3=81lvaro Herrera wrote:
> > On 2025-Sep-03, Antonin Houska wrote:
> >
> >> When working on the REPACK command, we see an ERROR caused by unexpect=
ed
> >> change of CurrentResourceOwner [1]. I think the problem is that
> >> reorderbuffer.c does not restore the original value after calling
> >> RollbackAndReleaseCurrentSubTransaction(). The attached patch tries to=
 handle
> >> the call like other callers throughout the tree do.
> >
>=20
> Interesting. I'm wondering that if this patch is applied we could remove =
the
> following code
>=20
> 		/*
> 		 * Logical decoding could have clobbered CurrentResourceOwner during
> 		 * transaction management, so restore the executor's value.  (This is
> 		 * a kluge, but it's not worth cleaning up right now.)
> 		 */
> 		CurrentResourceOwner =3D old_resowner;
>=20
> from pg_logical_slot_get_changes_guts and LogicalSlotAdvanceAndCheckSnapS=
tate
> functions too. IIUC the referred code is a band-aid that will be improved
> someday.

Even though we're fixing the likely reason of this problem, we cannot be 10=
0%
sure that no other problem like this still exists. So I'd not remove this
assignment. Maybe add Assert(CurrentResourceOwner =3D=3D old_resowner) in f=
ront of
that, and adjust the comment?

> > I have registered this as
> > https://commitfest.postgresql.org/patch/6051/
> >
> > I've been wondering whether this should be backpatched.  In principle
> > this is a bugfix, so it should, but I don't offhand recall any cases
> > where failure to set the current context/resowner in the other
> > reorderbuffer.c users causes a live bug, so ... maybe master only?  I'm
> > wondering if it's possible where anybody _depends_ on the current
> > behavior, but I suppose that's quite unlikely.
> >
>=20
> I would say apply it to master only. If/when we have a bug report we can
> backpatch it.

+1

> Per the crash description, I'm not sure we can create a
> reproducible test case with the current supported commands. Am I wrong?

It seems so, at least with he "CurrentResourceOwner =3D old_resowner" assig=
nment
in place. REPACK CONCURRENTLY exposes the problem a bit more because it has=
 at
least one kind of resource open during logical decoding: relation.

--=20
Antonin Houska
Web: https://www.cybertec-postgresql.com