Re: REPACK (ANALYZE) within transaction block segfaults

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(dot)cs(at)gmail(dot)com> wrote:

> 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(dot)fujii(at)gmail(dot)com> wrote:
>
>> On Thu, Sep 3, 2026 at 5:27 PM Osama Abdul Qader
>> <osamaabdulqader(dot)cs(at)gmail(dot)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
>>
>