Re: REPACK (ANALYZE) within transaction block segfaults

>
> In [1] 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.)

I removed that part because I interpreted the earlier request to make the
comment more precise as a request to remove the explanation and retain only
the current transaction-block restriction.

I understand the distinction now. Allowing REPACK (ANALYZE) in a
transaction block in the future would not necessarily mean that it is safe
to execute it from a function, procedure, or DO block, since ANALYZE may
start a new transaction in process_single_relation() while an SPI session
is active.

I've restored this explanation in the comment and kept the
transaction-block restriction explicitly as a current restriction for now.

In [2] 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?

The removal was due to my misunderstanding of that feedback. I should have
made the explanation more precise rather than removing it. Sorry about that.

BTW, "top posting" is not the preferred style in this mailing list [3].

Understood. I'll use inline replies going forward.

Also, the regenerated patch has been attached.

On Fri, Sep 4, 2026 at 7:00 PM Osama Abdul Qader <
osamaabdulqader(dot)cs(at)gmail(dot)com> wrote:

> I removed that path because I interpreted the earlier discussion as asking
> me to avoid claiming that REPACK (ANALYZE) inherently performs transaction
> management, and I replaced it with a shorter comment explaining the current
> restriction.
>
> I now understand your point that the comment should also explain the
> separate restriction on execution from a function/procedure/DO block. In
> particular, even if running REPACK (ANALYZE) inside a transaction block is
> reconsidered in the future, the restriction on execution from a function
> may still need to remain.
>
> I'll update the comment to make that distinction explicit and will also
> follow the mailing-list preferred inline-posting style in future replies.
>
> Thanks for pointing this out.
>
> With Regards,
> Osama Abdul Qader
>
> On Fri, 4 Sept, 2026, 6:24 pm Antonin Houska, <ah(at)cybertec(dot)at> wrote:
>
>> Osama Abdul Qader <osamaabdulqader(dot)cs(at)gmail(dot)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] 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] 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].
>>
>> [1] https://www.postgresql.org/message-id/49398.1787944525%40localhost
>> [2]
>> https://www.postgresql.org/message-id/CAHGQGwEezdMUixhJ-N0YO0OFUmh0uPaXRDkds5FS-5dmdwz4Bg%40mail.gmail.com
>> [3] https://wiki.postgresql.org/wiki/Mailing_Lists
>>
>> --
>> Antonin Houska
>> Web: https://www.cybertec-postgresql.com
>>
>