Re: pgsql: Implement streaming mode in ReorderBuffer.

Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> On Wed, Apr 28, 2021 at 5:31 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> 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