Amit Kapila <[email protected]> writes:
> On Wed, Apr 28, 2021 at 5:31 AM Tom Lane <[email protected]> wrote:
>> It's griping about this:
>> curtxn->concurrent_abort = true;
>> and I think it's got a point. There is little if any reason to have
>> confidence that curtxn must be non-NULL when this code is reached,
>> because it's in a PG_CATCH segment and there is a lot of code within
>> the PG_TRY that could throw an error before the spot where curtxn
>> is set. Not to mention that curtxn is set only conditionally.
> We can do that to silence this Warning, otherwise, there won't be any
> problem because it is set only when we get a particular error code and
> we can get that error code only when the conditions that lead to
> curtxn being set are met.
To be blunt, that argument is hopelessly naive. You cannot safely
make assumptions about a CATCH block having omniscient knowledge
of where an error was thrown from. At least, not with a check
no stronger than checking the SQLSTATE. All you need is some
extension ... or a user-defined function ... deciding to throw
that same SQLSTATE, and you're hosed.
If this code really does make the assumption that there's
only one possible cause of that SQLSTATE, I wonder whether
it's broken in ways worse than a mere core dump.
regards, tom lane
