SHOW ALL does not honor pg_read_all_settings membership

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

SHOW ALL does not honor pg_read_all_settings membership

Laurenz Albe
I noticed that commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 forgot
to teach SHOW ALL to show all GUCs when the user belongs to pg_read_all_settings.

Patch attached; I think this should be backpatched.

Yours,
Laurenz Albe

0001-Teach-SHOW-ALL-to-honor-pg_read_all_settings-members.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: SHOW ALL does not honor pg_read_all_settings membership

Laurenz Albe
On Thu, 2018-03-01 at 16:22 +0100, I wrote:
> I noticed that commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 forgot
> to teach SHOW ALL to show all GUCs when the user belongs to pg_read_all_settings.
>
> Patch attached; I think this should be backpatched.

Now that the dust from the last commitfest is settling, I'll make a second
attempt to attract attention for this small bug fix.

The original commit was Simon's.

Yours,
Laurenz Albe

Reply | Threaded
Open this post in threaded view
|

Re: SHOW ALL does not honor pg_read_all_settings membership

Michael Paquier-2
On Mon, Apr 16, 2018 at 02:32:10PM +0200, Laurenz Albe wrote:
> Now that the dust from the last commitfest is settling, I'll make a second
> attempt to attract attention for this small bug fix.
>
> The original commit was Simon's.

Thanks for the ping.

This was new as of v10, so this cannot be listed as an open item still I
have added that under the section for older bugs, because you are right
as far as I can see.

GetConfigOption is wrong by the way, as restrict_superuser means that
all members of the group pg_read_all_settings can read
GUC_SUPERUSER_ONLY params, and not only superusers, so the comment at
least needs a fix, the variable ought to be renamed as well.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: SHOW ALL does not honor pg_read_all_settings membership

Laurenz Albe
Michael Paquier wrote:

> On Mon, Apr 16, 2018 at 02:32:10PM +0200, Laurenz Albe wrote:
> > Now that the dust from the last commitfest is settling, I'll make a second
> > attempt to attract attention for this small bug fix.
> >
> > The original commit was Simon's.
>
> Thanks for the ping.
>
> This was new as of v10, so this cannot be listed as an open item still I
> have added that under the section for older bugs, because you are right
> as far as I can see.
>
> GetConfigOption is wrong by the way, as restrict_superuser means that
> all members of the group pg_read_all_settings can read
> GUC_SUPERUSER_ONLY params, and not only superusers, so the comment at
> least needs a fix, the variable ought to be renamed as well.
Thanks for the review!

I agree; here is a patch for that.

Yours,
Laurenz Albe

0002-Fix-comments-and-a-parameter-name.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: SHOW ALL does not honor pg_read_all_settings membership

Michael Paquier-2
On Fri, Apr 20, 2018 at 01:21:46PM +0200, Laurenz Albe wrote:
> I agree; here is a patch for that.

Thanks for taking care of that as well this looks fine to me.  Note to
committer: this needs to be merged with the first patch actually fixing
the SHOW ALL issue.

All the four core callers of GetConfigOption() actually do not use
restrict_superuser set to true...  Modules may use this option, so of
course let's not remove it.
--
Michael

signature.asc (849 bytes) Download Attachment
Previous Thread Next Thread