Re: Add default role 'pg_access_server_files'

On Mon, Apr 02, 2018 at 05:09:21PM -0400, Stephen Frost wrote:
> * Michael Paquier (michael@paquier.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.
--
Michael