Thanks for your reply. These patches look good to me!

> The only readily-reachable error case in BeginInternalSubTransaction
> is this specific one about IsInParallelMode, which was added later
> than the original design and evidently with not a lot of thought or
> testing.  The comment for it speculates about whether we could get
> rid of it, so I wonder if our thoughts about this ought to go in that
> direction.

IMHO, there are other error reports in the function
BeginInternalSubTransaction(), like
```
ereport(ERROR,
                (errcode(ERRCODE_OUT_OF_MEMORY),
                 errmsg("out of memory"),
                 errdetail("Failed on request of size %zu in memory context
\"%s\".",
                           size, context->name)));
```
we cannot avoid this crash by just getting rid of IsInParallelMode().

And in my test, the server won't crash in the plperl test.

With regards,
Hao Zhang

Tom Lane <t...@sss.pgh.pa.us> 于2023年12月2日周六 09:51写道:

> I wrote:
> > The only readily-reachable error case in BeginInternalSubTransaction
> > is this specific one about IsInParallelMode, which was added later
> > than the original design and evidently with not a lot of thought or
> > testing.  The comment for it speculates about whether we could get
> > rid of it, so I wonder if our thoughts about this ought to go in that
> > direction.
>
> After thinking a bit more I wonder why we need that error check at all.
> Why isn't it sufficient to rely on GetNewTransactionId()'s check that
> throws an error if a parallelized subtransaction tries to obtain an XID?
> I don't see why we'd need to "synchronize transaction state" about
> anything that never acquires an XID.
>
>                         regards, tom lane
>

Reply via email to