REPACK (ANALYZE) within transaction block segfaults

Hello Nathan,

Thanks for reporting this issue.

I haven't investigated this case yet, but I'll try to reproduce it as soon
as possible and look into the transaction/SPI handling around 'REPACK
(ANALYZE)'.

I'll follow up, once I got something concrete.

With best regards
Osama Abdul Qader

On Thu, 27 Aug, 2026, 7:26 pm Nathan Bossart, <nathandbossart@gmail.com>
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.

Regards,

--
Fujii Masao

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?

--
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

On Fri, Aug 28, 2026 at 09:15:25PM +0200, Antonin Houska wrote:

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:

This is probably the way to go for v19. As you note, the analogous VACUUM
command has long ERROR'd, and we could always look into removing this
restriction in the future. I'd rather do it that way than ship an
incorrect fix in v19 that will be tougher to back out.

--
nathan

Hi,

Just a quick update on the replication slot invalidation durability issue.

I've moved past the initial reproduction and have been investigating the
underlying behavior. I now have a candidate patch which changes the
invalidation flow so that the invalidated slot state is persisted before
the invalidation is published in shared memory. The slot synchronization
path has been updated accordingly, and I've also added TAP coverage for the
durability scenarios, including injected failures during slot persistence.

I was able to get the relevant regression coverage passing. While running
the broader recovery test suite, I encountered a few failures in existing
TAP tests, particularly around 001_stream_rep.pl and 006_logical_decoding.pl.
I'm currently investigating whether these are related to my changes or are
test-environment/intermittent issues.

I'll continue working through these failures and validating the patch. Once
the remaining test issues are understood and the patch is cleaned up, I
expect to have a revised patch ready soon.

Best regards,
Osama Abdul Qader

On Sat, Aug 29, 2026 at 1:22 AM Nathan Bossart <nathandbossart@gmail.com>
wrote:

On Sun, Aug 30, 2026 at 09:52:46PM +0530, Osama Abdul Qader wrote:

Just a quick update on the replication slot invalidation durability issue.

I think you may have replied to the wrong thread...

--
nathan

Greetings of the day everyone,

You were right, my intention was to update you about my reproduction of the
bug and the work I was doing yesterday.

I regret for the confusion I caused by replying to the wrong thread. I'll
sure to double check the thread before sending future updates.

With regards,
Osama Abdul Qader.

On Sun, 30 Aug, 2026, 10:31 pm Nathan Bossart, <nathandbossart@gmail.com>
wrote:

On Sat, Aug 29, 2026 at 4:52 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

This is probably the way to go for v19. As you note, the analogous VACUUM
command has long ERROR'd, and we could always look into removing this
restriction in the future. I'd rather do it that way than ship an
incorrect fix in v19 that will be tougher to back out.

I'm ok with this.

I have a few review comments on Antonin's patch.

As with my patch, I think it would be better to document the restriction
and add tests covering the following cases:

- plain REPACK is allowed in a transaction block
- REPACK (ANALYZE) is not allowed in a transaction block
- REPACK (ANALYZE) is not allowed from a function

+ * Technically, transaction block is not a problem for REPACK
+ * (ANALYZE), but if it's called from a pl/pgsql function,

Since it can also be called from procedures, functions and DO blocks,
mentioning only a PL/pgSQL function seems too narrow.

+ * cluster_rel() might start a new transaction while SPI session is in

Is this correct? It seems that the new transaction for ANALYZE is
started in process_single_relation(), not in cluster_rel().

+ * that's just consistent with VACUUM (FULL, ANALYZE), which is a
+ * synonym for REPACK (ANALYZE).

Is VACUUM (FULL, ANALYZE) really a synonym for REPACK (ANALYZE)?

Regards,

--
Fujii Masao

On 2026-Sep-02, Fujii Masao wrote:

+ * that's just consistent with VACUUM (FULL, ANALYZE), which is a
+ * synonym for REPACK (ANALYZE).

Is VACUUM (FULL, ANALYZE) really a synonym for REPACK (ANALYZE)?

That's the intent, at least. If there are things that work differently,
I would strive to change them so that they do work the same. However,
some such changes might be too invasive for pg19, but I would still see
about changing those in pg20.

Now, maybe there are things about VACUUM FULL ANALYZE that we don't like
(perhaps, for instance, they exist solely because of even older
backwards compatibility concerns) that we would prefer not to have in
REPACK. I don't know if anything of that sort exists, but if so, I
would propose to seek decisions for each thing individually.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Essentially, you're proposing Kevlar shoes as a solution for the problem
that you want to walk around carrying a loaded gun aimed at your foot.
(Tom Lane)

Alvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-Sep-02, Fujii Masao wrote:

+ * that's just consistent with VACUUM (FULL, ANALYZE), which is a
+ * synonym for REPACK (ANALYZE).

Is VACUUM (FULL, ANALYZE) really a synonym for REPACK (ANALYZE)?

That's the intent, at least. If there are things that work differently,
I would strive to change them so that they do work the same. However,
some such changes might be too invasive for pg19, but I would still see
about changing those in pg20.

Now, maybe there are things about VACUUM FULL ANALYZE that we don't like
(perhaps, for instance, they exist solely because of even older
backwards compatibility concerns) that we would prefer not to have in
REPACK. I don't know if anything of that sort exists, but if so, I
would propose to seek decisions for each thing individually.

Maybe the question was about the wording - "synonym" might indicate that both
commands execute the same code. Perhaps the comment should rather say that
REPACK (ANALYZE) is (intended to be) a replacement of VACUUM (FULL, ANALYZE).

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

Hi everyone,

I believe I was replying to the wrong thread earlier.

The issue I was looking into is the crash caused by executing REPACK
(ANALYZE) inside a transaction block.

REPACK (ANALYZE) performs transaction management internally, including
committing and starting a new transaction while processing the relation. It
therefore cannot safely be executed from an existing transaction block.

I have prepared a patch that rejects REPACK (ANALYZE) with
PreventInTransactionBlock(), consistent with the existing restriction
for REPACK
(CONCURRENTLY). I also added a regression test covering execution inside a
transaction block.

The patch applies cleanly to the current tree and passes git diff --check.
Patch attached.

Regards,
Osama Abdul Qader

On Wed, Sep 2, 2026 at 2:17 PM Antonin Houska <ah@cybertec.at> wrote:

Osama Abdul Qader <osamaabdulqader.cs@gmail.com> wrote:

I have prepared a patch that rejects REPACK (ANALYZE) with PreventInTransactionBlock(), consistent with the existing restriction for
REPACK (CONCURRENTLY). I also added a regression test covering execution inside a transaction block.

The patch applies cleanly to the current tree and passes git diff --check.

Is this a new version of [1]/messages/by-id/49398.1787944525@localhost? If so, I'm not sure it addresses all the
problems mentioned in [2]/messages/by-id/CAHGQGwEezdMUixhJ-N0YO0OFUmh0uPaXRDkds5FS-5dmdwz4Bg@mail.gmail.com. And regarding regression tests, it misses the
changes in expected/cluster.out.

[1]: /messages/by-id/49398.1787944525@localhost
[2]: /messages/by-id/CAHGQGwEezdMUixhJ-N0YO0OFUmh0uPaXRDkds5FS-5dmdwz4Bg@mail.gmail.com

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

Hi Antonin and Everyone,

Greetings of the day,

I'll look into the issues mentioned in [1] and [2], including the missing
regression test changes in 'expected/cluster.out', and prepare an updated
patch.

With best regards,
Osama Abdul Qader

On Thu, Sep 3, 2026 at 10:12 AM Antonin Houska <ah@cybertec.at> wrote:

Hi everybody,

Thanks for pointing that out.

Yes, this is an updated version of the patch. I've addressed the missing
regression test changes by updating 'expected/cluster.out' as well.

The patch now includes:

- the 'PreventInTransactionBlock()' check for 'REPACK (ANALYZE)';
- the regression test in 'cluster.sql'; and
- the corresponding expected output in 'expected/cluster.out'

I also verified that the regression tests 'test_setup' and cluster pass,
and that the patch applies cleanly to a clean worktree.

The updated patch is attached.

With Regards,
Osama Abdul Qader

On Thu, Sep 3, 2026 at 12:43 PM Osama Abdul Qader <
osamaabdulqader.cs@gmail.com> wrote:

On Thu, Sep 3, 2026 at 5:27 PM Osama Abdul Qader
<osamaabdulqader.cs@gmail.com> wrote:

The updated patch is attached.

Thanks for updating the patch!

I have a few review comments.

As I told upthread, I think the restriction that REPACK (ANALYZE) cannot
be executed inside a transaction block should be documented. For example,
how about adding something like the following to the description of
the ANALYZE option in the REPACK docs?

This option cannot be used inside a transaction block, or from a
function, procedure, or <command>DO</command> block.

+ * It therefore cannot be executed inside a transaction block or

Is this really true? As discussed upthread, I was thinking that it can
be executed even inside a transaction block, but that we decided to
intentionally prevent it from doing so to match the behavior of
VACUUM (FULL, ANALYZE) as the safe behavior for v19. No?

Regarding the tests, as I told upthread, I think it's better to also
cover the following cases:

- plain REPACK is allowed in a transaction block
- REPACK (ANALYZE) is not allowed from a function

For example:

-------------------------
--- Verify partial analyze works
+-- Verify REPACK (ANALYZE) works, including partial analyze.
 REPACK (ANALYZE) clstr_tst (a);
 REPACK (ANALYZE) clstr_tst;
+-- Plain REPACK is allowed in a transaction block.
+BEGIN;
+REPACK clstr_tst;
+ROLLBACK;
+-- REPACK (ANALYZE) is not allowed in a transaction block.
+BEGIN;
+REPACK (ANALYZE) clstr_tst;
+ROLLBACK;
+-- REPACK (ANALYZE) is not allowed from a function.
+DO $$ BEGIN EXECUTE 'REPACK (ANALYZE) clstr_tst'; END $$;
-------------------------

Regards,

--
Fujii Masao

Good Evening Masao San

Thanks for the detailed review.

I understood and I'll update the patch to:

- document the transaction-block restriction in repack documentation.
- revise the comment in 'repack.c' to reflect that this is an
intentional restriction for v19, rather than an inherent requirement; and
- expand the regression tests to cover plain REPACK inside a transaction
block, REPACK (ANALYZE) inside a transaction block, and REPACK (ANALYZE)
from a DO block.

I'll send an updated patch once these changes are made.

With regards,
Osama Abdul Qader

On Thu, 3 Sept, 2026, 7:21 pm Fujii Masao, <masao.fujii@gmail.com> wrote:

Thanks for the review,

I've updated the patch to address your comments:

- Documented that 'REPACK (ANALYZE)' cannot be used inside a transaction
block, or from a function, procedure or 'DO' block.
- Updated the comment in repack.c to clarify that this restriction is
intentional for now, consistently with VACUUM (FULL, ANALYZE).
- Added regression tests covering plain REPACK inside a transaction
block; REPACK (ANALYZE) inside a transaction block; REPACK (ANALYZE)
from a DO block.
- Regenerated 'expected/cluster.out.

The focused 'test_setup' and 'cluster' regression tests pass, and the patch
applies cleanly to the current tree.

The updated patch is attached.

With regards,
Osama Abdul Qader

On Thu, Sep 3, 2026 at 9:59 PM Osama Abdul Qader <
osamaabdulqader.cs@gmail.com> wrote:

Osama Abdul Qader <osamaabdulqader.cs@gmail.com> wrote:

I've updated the patch to address your comments:

* Documented that 'REPACK (ANALYZE)' cannot be used inside a transaction block, or from a function, procedure or 'DO' block.
* Updated the comment in repack.c to clarify that this restriction is intentional for now, consistently with VACUUM (FULL, ANALYZE).

In [1]/messages/by-id/49398.1787944525@localhost I added a comment explaining why it's a problem to run REPACK (ANALYZE)
from function. I thought it's important so that, when we conclude (in the
future) that running in block is fine, we still keep checking for execution
from a function. (PreventInTransactionBlock() checks both at the moment.)

In [2]/messages/by-id/CAHGQGwEezdMUixhJ-N0YO0OFUmh0uPaXRDkds5FS-5dmdwz4Bg@mail.gmail.com I was advised to make the comment more precise, but as you appear to
have taken the patch over, I expected that you'll do that. However, you simply
removed that part of the comment. Can you please explain why?

BTW, "top posting" is not the preferred style in this mailing list [3]https://wiki.postgresql.org/wiki/Mailing_Lists.

[1]: /messages/by-id/49398.1787944525@localhost
[2]: /messages/by-id/CAHGQGwEezdMUixhJ-N0YO0OFUmh0uPaXRDkds5FS-5dmdwz4Bg@mail.gmail.com
[3]: https://wiki.postgresql.org/wiki/Mailing_Lists

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