From: Antonin Houska <ah@cybertec.at>
To: Nathan Bossart <nathandbossart@gmail.com>
cc: Fujii Masao <masao.fujii@gmail.com>, pgsql-hackers@postgresql.org,
    alvherre@kurilemu.de
Subject: Re: REPACK (ANALYZE) within transaction block segfaults
In-reply-to: <apCSd2NvvEJfBmVu@nathan>
References: <apBCEl4hBHlq0axm@nathan>
 <CAHGQGwEqN80ka_PY__DiPCu+S=h_7KgujnMEU38OU9C1zsGQpA@mail.gmail.com>
 <apCSd2NvvEJfBmVu@nathan>
Comments: In-reply-to Nathan Bossart <nathandbossart@gmail.com>
   message dated "Thu, 27 Aug 2026 14:39:35 -0500."
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Date: Fri, 28 Aug 2026 21:15:25 +0200
Message-ID: <49398.1787944525@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/49398.1787944525%40localhost>
Precedence: bulk

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Nathan Bossart <nathandbossart@gmail.com> wrote:

> On Fri, Aug 28, 2026 at 01:09:59AM +0900, Fujii Masao wrote:
> > On Thu, Aug 27, 2026 at 10:56=E2=80=AFPM Nathan Bossart
> > <nathandbossart@gmail.com> wrote:
> >> Presumably we need to handle transaction blocks a bit like how vacuum()
> >> does.  Or maybe even prevent REPACK (ANALYZE) within a transaction blo=
ck.
> >=20
> > I looked into this a bit. I think the problem is not ordinary
> > transaction blocks themselves, but non-top-level execution, such as the
> > DO block in the reproducer.
> >=20
> > So the attached patch rejects only non-top-level REPACK (ANALYZE)
> > commands.
>=20
> Hm.  Couldn't we do something like the in_outer_xact/use_own_xacts stuff =
in
> vacuum() to get it working instead?

I think there are just two different concepts (for historical reasons?):
vacuum_rel() expects no active transaction on entry, while cluster_rel()
handles transaction boundaries on its own.

Since REPACK (ANALYZE) is effectively VACUUM (FULL, ANALYZE), I'd prefer the
same behavior, i.e. prohibiting execution both in a transaction block and i=
n a
function:

postgres=3D# BEGIN; VACUUM (FULL, ANALYZE) t; END;
BEGIN
ERROR:  VACUUM cannot run inside a transaction block
ROLLBACK

postgres=3D# DO $$ BEGIN EXECUTE 'VACUUM (FULL, ANALYZE) t'; END $$;
ERROR:  VACUUM cannot be executed from a function or procedure
CONTEXT:  SQL statement "VACUUM (FULL, ANALYZE) t"
PL/pgSQL function inline_code_block line 1 at EXECUTE

postgres=3D# BEGIN; REPACK (ANALYZE) t; END;
BEGIN
ERROR:  REPACK (ANALYZE) cannot run inside a transaction block
ROLLBACK

postgres=3D# DO $$ BEGIN EXECUTE 'REPACK (ANALYZE) t'; END $$;
ERROR:  REPACK (ANALYZE) cannot be executed from a function or procedure
CONTEXT:  SQL statement "REPACK (ANALYZE) t"
PL/pgSQL function inline_code_block line 1 at EXECUTE

The attached patch does that.

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


--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=0001-Do-not-allow-REPACK-ANALYZE-in-function-and-in-trans.patch

From b577bc4b61474b65a80b5ab8b5333c6ccd751e81 Mon Sep 17 00:00:00 2001
From: Antonin Houska <ah@cybertec.at>
Date: Fri, 28 Aug 2026 19:53:26 +0200
Subject: [PATCH] Do not allow REPACK (ANALYZE) in function and in transaction
 block.

If REPACK (ANALYZE) is called from a pl/pgsql function, cluster_rel() might
start a new transaction while SPI session is in progress. Use
PreventInTransactionBlock() to avoid that.

That function also raises error if REPACK (ANALYZE) is called in a transaction
block, but that's fine: VACUUM (FULL, ANALYZE) - a synonym of REPACK (ANALYZE)
- also raises ERROR in that case.
---
 src/backend/commands/repack.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 477c86b2ba6..81877029199 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -314,6 +314,20 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
 		 */
 		PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
 	}
+	else if ((params.options & CLUOPT_ANALYZE) != 0)
+	{
+		/*
+		 * Technically, transaction block is not a problem for REPACK
+		 * (ANALYZE), but if it's called from a pl/pgsql function,
+		 * cluster_rel() might start a new transaction while SPI session is in
+		 * progress. Make sure ERROR is raised instead.
+		 *
+		 * This way we also prohibit execution in a transaction block, but
+		 * that's just consistent with VACUUM (FULL, ANALYZE), which is a
+		 * synonym for REPACK (ANALYZE).
+		 */
+		PreventInTransactionBlock(isTopLevel, "REPACK (ANALYZE)");
+	}
 
 	/*
 	 * If a single relation is specified, process it and we're done ... unless
-- 
2.52.0


--=-=-=--