Re: Bug with pg_ctl -w/wait and config-only directories

On Mon, Oct 3, 2011 at 11:04 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> OK, here is a patch that adds a -C option to the postmaster so any
> config variable can be dumped, even while the server is running (there
> is no security check because we don't have a user name at this point),
> e.g.:
>
>        postgres -D /pg_upgrade/tmp -C data_directory
>        /u/pg/data

It seems both ugly and unnecessary to declare dump_config_variable as
char[MAXPGPATH]. MAXPGPATH doesn't seem like the right length for a
buffer intended to hold a GUC name, but in fact I don't think you need
a buffer at all. I think you can just declare it as char * and say
dump_config_variable = optarg. getopt() doesn't overwrite the input
argument vector, does it?

Also, I think you should remove this comment:

+ /* This allows anyone to read super-user config values. */

It allows anyone to read super-user config values *who can already
read postgresql.conf*. Duh.

> It also modifies pg_ctl to use this feature.  It works fine for pg_ctl
> -w start/stop with a config-only directory, so this is certainly in the
> right direction.  You can also use pg_ctl -o '--data_directory=/abc' and
> it will be understood:
>
>        pg_ctl -o '--data_directory=/u/pg/data' -D tmp start
>
> If you used --data_directory to start the server, you will need to use
> --data_directory to stop it, which seems reasonable.

Yep. I think that when adjust_data_dir() changes pg_data it probably
needs to call canonicalize_path() on the new value.

> Patch attached.  This was much simpler than I thought.  :-)

Yes, this looks pretty simple.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company