Thread: Revoke execution permission of pg_stat_statements_reset() frompg_read_all_stats role

Revoke execution permission of pg_stat_statements_reset() frompg_read_all_stats role

From
Haribabu Kommi
Date:
[spawning a new thread]

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.
Attachment
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

Attachment
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


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

Attachment
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