Re: exposing pg_controldata and pg_config as functions

On 01/18/2016 08:51 PM, Michael Paquier wrote:
> On Tue, Jan 19, 2016 at 1:49 PM, Michael Paquier
>> + }
>> +
>> + /*
>> + * no longer need the tuple descriptor reference created by
>> The patch has some whitespaces.

I take it you mean a line with only whitespace? Fixed.

>> +REVOKE ALL on pg_config FROM PUBLIC;
>> +REVOKE EXECUTE ON FUNCTION pg_config() FROM PUBLIC;
>> I guess that this portion is still under debate :)

Left in for the moment.

>> make[1]: Nothing to be done for `all'.
>> make -C ../backend submake-errcodes
>> make[2]: *** No rule to make target `config_info.o', needed by
>> `libpgcommon.a'. Stop.
>> make[2]: *** Waiting for unfinished jobs....
>> The patch is visibly forgetting to include config_info.c, which should
>> be part of src/common.

Confounded git! ;-)
Fixed.

>> /*
>> + * This function cleans up the paths for use with either cmd.exe or Msys
>> + * on Windows. We need them to use filenames without spaces, for which a
>> + * short filename is the safest equivalent, eg:
>> + * C:/Progra~1/
>> + */
>> +void
>> +cleanup_path(char *path)
>> +{
>> Perhaps this refactoring would be useful as a separate patch?

It doesn't seem worth doing on its own, and it is required by the rest
of this patch. I'd say keep it here, but I'll break it out if you think
it important enough.

> You need as well to update @pgcommonallfiles in Mkvcbuild.pm or the
> compilation with MSVC is going to fail.

Good catch -- done.

The only things I know of still lacking is:
1) Documentation
2) Decision on REVOKE ... FROM PUBLIC

I'm assuming by the lack of complainants that there is enough support
for the feature (conceptually at least) that it is worthwhile for me to
write the docs. Will do that over the next couple of days or so.

Thanks for the code reviews!

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development