pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Fix typos and inconsistencies for v16
Date: Tue, 18 Apr 2023 15:10:07 -0400
Message-ID: <2808275.1681845007@sss.pgh.pa.us> (raw)
In-Reply-To: <ZD7luYOGJs8DtDcJ@telsasoft.com>
References: <d5f68d19-c0fc-91a9-118d-7c6a5a3f5fad@gmail.com>
	<ZD3D1QxoccnN8A1V@telsasoft.com>
	<CAApHDvpQWpB81E=Vb5zTrJdD_jeZ613ngt4KzQ=TUzOCftaREg@mail.gmail.com>
	<ZD7luYOGJs8DtDcJ@telsasoft.com>

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





view thread (12+ messages)  latest in thread

Message-ID: <2808275.1681845007@sss.pgh.pa.us>
Permalink:  ../2808275.1681845007@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/2808275.1681845007@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pryzby@telsasoft.com, dgrowleyml@gmail.com, exclusion@gmail.com
  Subject: Re: Fix typos and inconsistencies for v16
  In-Reply-To: <2808275.1681845007@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox