Justin Pryzby <pryzby@telsasoft.com> writes:
> On Tue, Apr 18, 2023 at 02:06:43PM +1200, David Rowley wrote:
>> On Tue, 18 Apr 2023 at 10:10, Justin Pryzby <pryzby@telsasoft.com> wrote:
>>> and s/evade/avoid/
>> I didn't touch this. You'll need to provide more justification for why
>> you think it's more correct than what's there.
> I'd noticed that it's a substitution/mistake that's been made in the
> past.
"Evade" doesn't seem like le mot juste there; it's got negative
connotations. But the code around it is just horrible. Some offenses:
* No documentation in the function header comment of what the
usersetArray parameter is or does. Which is bad enough in itself,
but what the parameter actually does is falsify the header comment's
principal claim that the passed context is what is used. So I don't
find that omission acceptable.
* Non-obvious, and quite unnecessary, dependency on the isnull variable
having been left in a particular state by previous code.
* For me, at least, it'd read better if the if/else arms were swapped,
allowing removal of the negation in the if-condition and bringing
the code this comment comments on closer to said comment.
As for the comment text, maybe say
* If the value was USER SET, then apply it at PGC_USERSET context
* rather than the caller-supplied context, to prevent any more-restricted
* GUCs being set. Also pass InvalidOid for the role, to ensure any
* special privileges of the current user aren't applied.
I hesitate to go look at the rest of this commit, but I guess somebody
had better.
regards, tom lane
