Re: Restore CurrentUserId only if 'prevUser' is valid when abort transaction

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Mon, Nov 12, 2018 at 08:17:29PM -0500, Tom Lane wrote:
>> I think the real bug here is that a bunch of potentially-failable
>> operations have been thrown in before we've finished initializing the
>> TransactionState to minimal sanity. (Inserting code with the aid of a
>> dartboard seems to be a chronic disease around here :-(.)

> When first working on the patch I got to wonder if there were any
> intermediate states which relied on the user ID of the security context
> flags which could have justified its current position. Just checking
> now it looks safe to move up the call. I have checked as well my test
> cases injecting errors. What do you think about the attached?

Looks sane to me.

> Also, I think that we should backpatch something all the way down.

Yes.

>> I'd be strongly inclined to change the elog(WARNING) at line 1815
>> to an assertion, because calling elog exposes us to all kinds of
>> hazards that we don't need here.

> No objections from here. I would do that only on HEAD though.

Well, if it is an issue then it's an issue for back branches too.
It's fine, I think, as long as the warning stays a warning ...
but there are lots of ways in which trying to print a warning
might turn into an error.

regards, tom lane