Re: Add default role 'pg_access_server_files'

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add default role 'pg_access_server_files'
Date: 2018-04-04 12:18:55
Message-ID: 20180404121855.GM24540@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Michael,

* Michael Paquier (michael(at)paquier(dot)xyz) wrote:
> On Mon, Apr 02, 2018 at 05:09:21PM -0400, Stephen Frost wrote:
> > * Michael Paquier (michael(at)paquier(dot)xyz) wrote:
> >> No refactoring for pg_file_unlink and its v1.1?
> >
> > I considered each function and thought about if it'd make sense to
> > refactor them or if they were simple enough that the additional function
> > wouldn't really be all that useful. I'm open to revisiting that, but
> > just want to make it clear that it was something I thought about and
> > considered. Since pg_file_unlink is basically two function calls, I
> > didn't think it worthwhile to refactor those into their own function.
>
> I don't mind if this is done your way.
>
> >> The argument checks are exactly the same for pg_file_rename and
> >> pg_file_rename_v1_1. Why about just passing fcinfo around and simplify
> >> the patch?
> >
> > In general, I prefer to keep the PG_FUNCTION_ARGS abstraction when we
> > can. Unfortunately, that does fall apart when wrapping an SRF as in
> > pg_logdir_ls(), but with pg_file_rename we can maintain it and it's
> > really not that much code to do so. As with the refactoring of
> > pg_file_unlink, this is something which could really go either way.
>
> Okay...
>
> > I'm not sure how useful it is to REVOKE the rights on the simple SQL
> > function; that would just mean that an admin has to go GRANT the rights
> > on that function as well as the three-argument version.
>
> Indeed, I had a brain fade here.
>
> > The more I think about it, the more I like the approach of just dropping
> > these deprecated "alternative names for things in core" with the new
> > adminpack version. In terms of applications, as I understand it, they
> > aren't used in the latest version of pgAdmin3 and they also aren't used
> > with pgAdmin4, so I don't think we need to be worrying about supporting
> > them in v11.
>
> +1 to simplify the code a bit.

Great, thanks. I'll be doing more review of it myself and see about
pushing it later this afternoon.

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Banck 2018-04-04 12:19:30 Re: pgsql: Validate page level checksums in base backups
Previous Message Thomas Munro 2018-04-04 12:00:14 Re: Optimize Arm64 crc32c implementation in Postgresql