From: Tom Lane <tgl@sss.pgh.pa.us>
To: Hao Zhang <zhrt1446384557@gmail.com>
cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] plpython function causes server panic
In-reply-to: <CALY6Dr8fPasw5DP3FFD2YjRZMKeX9_Ank14tz_bmPjjqLB1U7g@mail.gmail.com>
References: <CALY6Dr-2yLVeVPhNMhuBnRgOZo1UjoTETgtKBx1B2gUi8yy+3g@mail.gmail.com> <301509.1701479055@sss.pgh.pa.us> <307441.1701481871@sss.pgh.pa.us> <CALY6Dr8fPasw5DP3FFD2YjRZMKeX9_Ank14tz_bmPjjqLB1U7g@mail.gmail.com>
Comments: In-reply-to Hao Zhang <zhrt1446384557@gmail.com>
	message dated "Mon, 04 Dec 2023 17:21:29 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <976743.1701813393.1@sss.pgh.pa.us>
Date: Tue, 05 Dec 2023 16:56:33 -0500
Message-ID: <976744.1701813393@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/976744.1701813393%40sss.pgh.pa.us>
Precedence: bulk

Hao Zhang <zhrt1446384557@gmail.com> writes:
>> 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

Sure, but all the other ones are extremely hard to hit, which is why
we didn't bother to worry about them to begin with.  If we want to
make this more formally bulletproof, my inclination would be to
(a) get rid of the IsInParallelMode restriction and then (b) turn
the function into a critical section, so that any other error gets
treated as a PANIC.  Maybe at some point we'd be willing to make a
variant of BeginInternalSubTransaction that has a different API and
can manage such cases without a PANIC, but that seems far down the
road to me, and certainly not something to be back-patched.

The main reason for my caution here is that, by catching an error
and allowing Python (or Perl, or something else) code to decide
what to do next, we are very dependent on that code doing the right
thing.  This is already a bit of a leap of faith for run-of-the-mill
errors.  For errors in transaction startup or shutdown, I think it's
a bigger leap than I care to make.  We're pretty well hosed if we
can't make the transaction machinery work, so imagining that we can
clean up after such an error and march merrily onwards seems mighty
optimistic.

			regards, tom lane