agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Fujii Masao <masao.fujii@gmail.com>
Cc: pgsql-hackers@postgresql.org
Cc: alvherre@kurilemu.de
Subject: Re: REPACK (ANALYZE) within transaction block segfaults
Date: Fri, 28 Aug 2026 21:15:25 +0200
Message-ID: <49398.1787944525@localhost> (raw)
In-Reply-To: <apCSd2NvvEJfBmVu@nathan>
References: <apBCEl4hBHlq0axm@nathan>
<CAHGQGwEqN80ka_PY__DiPCu+S=h_7KgujnMEU38OU9C1zsGQpA@mail.gmail.com>
<apCSd2NvvEJfBmVu@nathan>
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 PM 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 block.
> >
> > 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.
> >
> > So the attached patch rejects only non-top-level REPACK (ANALYZE)
> > commands.
>
> 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 in a
function:
postgres=# BEGIN; VACUUM (FULL, ANALYZE) t; END;
BEGIN
ERROR: VACUUM cannot run inside a transaction block
ROLLBACK
postgres=# 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=# BEGIN; REPACK (ANALYZE) t; END;
BEGIN
ERROR: REPACK (ANALYZE) cannot run inside a transaction block
ROLLBACK
postgres=# 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.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
[text/x-diff] 0001-Do-not-allow-REPACK-ANALYZE-in-function-and-in-trans.patch (1.7K, ../49398.1787944525@localhost/2-0001-Do-not-allow-REPACK-ANALYZE-in-function-and-in-trans.patch)
download | inline diff:
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
view thread (25+ messages) latest in thread
Message-ID: <49398.1787944525@localhost>
Permalink: ../49398.1787944525@localhost/
Also on: postgresql.org/message-id/49398.1787944525@localhost
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, nathandbossart@gmail.com, masao.fujii@gmail.com, alvherre@kurilemu.de
Subject: Re: REPACK (ANALYZE) within transaction block segfaults
In-Reply-To: <49398.1787944525@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox