Michael, * Michael Paquier (mich...@paquier.xyz) wrote: > On Mon, Apr 02, 2018 at 05:09:21PM -0400, Stephen Frost wrote: > > * Michael Paquier (mich...@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.
Great, thanks. I'll be doing more review of it myself and see about pushing it later this afternoon. Stephen
signature.asc
Description: PGP signature
