Revoke execution permission of pg_stat_statements_reset() from pg_read_all_stats role

Started by Haribabu Kommiover 7 years ago5 messageshackers
Jump to latest
#1Haribabu Kommi
kommi.haribabu@gmail.com

Attachments:

0001-Revoke-pg_stat_statements_reset-permissions_v2.patchapplication/octet-stream; name=0001-Revoke-pg_stat_statements_reset-permissions_v2.patchDownload+10-3
#2Michael Paquier
michael@paquier.xyz
In reply to: Haribabu Kommi (#1)
Re: Revoke execution permission of pg_stat_statements_reset() from pg_read_all_stats role

On Mon, Sep 24, 2018 at 11:08:14AM +1000, Haribabu Kommi wrote:

In commit 25fff40798 the execute permission of pg_stat_statements_reset()
is provided to pg_read_all_stats role in [1].

The execute permissions grant to pg_read_all_stats concern is raised in [2]
during the discussion of supporting different methods of reset the stats,
instead of resetting all.

Here I attached the patch that reverts the permission grant as per the
discussion
in [3].

This should be back-patched. Any opinions about bumping up this
extension version in back-branches like what has been done in 53b79ab4?
--
Michael

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: Revoke execution permission of pg_stat_statements_reset() from pg_read_all_stats role

Michael Paquier <michael@paquier.xyz> writes:

This should be back-patched. Any opinions about bumping up this
extension version in back-branches like what has been done in 53b79ab4?

Yes, you need to bump the extension version to change anything in the
extension's script file.

For v10 and up, the method used in 53b79ab4 is overcomplicated: you only
need to add a delta script not a new base script. (If you had to
back-patch before v10, it might be best to add a new base script in all
the branches just to keep the patches consistent; but IIUC this issue only
arises in v10 and up.) I'd consider following, eg, 7f563c09f as a
prototype instead.

regards, tom lane

#4Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#3)
Re: Revoke execution permission of pg_stat_statements_reset() from pg_read_all_stats role

On Mon, Sep 24, 2018 at 12:02:35PM -0400, Tom Lane wrote:

For v10 and up, the method used in 53b79ab4 is overcomplicated: you only
need to add a delta script not a new base script. (If you had to
back-patch before v10, it might be best to add a new base script in all
the branches just to keep the patches consistent; but IIUC this issue only
arises in v10 and up.) I'd consider following, eg, 7f563c09f as a
prototype instead.

Of course, thanks. Sorry for the incorrect reference pointing to a
commit of REL9_6_STABLE. As the patch only needs to be applied down to
v10, there is no need to do anything more complicated than what Hari has
proposed. So, committed after a bit of comment and format tweaks.
--
Michael

#5Haribabu Kommi
kommi.haribabu@gmail.com
In reply to: Michael Paquier (#4)
Re: Revoke execution permission of pg_stat_statements_reset() from pg_read_all_stats role

On Tue, Sep 25, 2018 at 10:58 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Mon, Sep 24, 2018 at 12:02:35PM -0400, Tom Lane wrote:

For v10 and up, the method used in 53b79ab4 is overcomplicated: you only
need to add a delta script not a new base script. (If you had to
back-patch before v10, it might be best to add a new base script in all
the branches just to keep the patches consistent; but IIUC this issue

only

arises in v10 and up.) I'd consider following, eg, 7f563c09f as a
prototype instead.

Of course, thanks. Sorry for the incorrect reference pointing to a
commit of REL9_6_STABLE. As the patch only needs to be applied down to
v10, there is no need to do anything more complicated than what Hari has
proposed. So, committed after a bit of comment and format tweaks.

Thanks for the changes and commit.

Regards,
Haribabu Kommi
Fujitsu Australia