On 11/01/2024 19:37, Robert Haas wrote:
> On Wed, Jan 10, 2024 at 4:25 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> The problem with CommitTransactionCommand (or rather
>> AbortCurrentTransaction() which has the same problem)
>> and ShowTransactionStateRec is that they get called in a state where
>> aborting can lead to a panic. If you add a "check_stack_depth()" to them
>> and try to reproducer scripts that Egor posted, you still get a panic.
>
> Hmm, that's unfortunate. I'm not sure what to do about that. But I'd
> still suggest looking for a goto-free approach.
Here's one goto-free attempt. It adds a local loop to where the
recursion was, so that if you have a chain of subtransactions that need
to be aborted in CommitTransactionCommand, they are aborted iteratively.
The TBLOCK_SUBCOMMIT case already had such a loop.
I added a couple of comments in the patch marked with "REVIEWER NOTE",
to explain why I changed some things. They are to be removed before
committing.
I'm not sure if this is better than a goto. In fact, even if we commit
this, I think I'd still prefer to replace the remaining recursive calls
with a goto. Recursion feels a weird to me, when we're unwinding the
states from the stack as we go.
Of course we could use a "for (;;) { ... continue }" construct around
the whole function, instead of a goto, but I don't think that's better
than a goto in this case.
--
Heikki Linnakangas
Neon (https://neon.tech)