Hi Tom, The actual fix do the job for preventing coredump. Effectively, reporting "" instead of (null) would be more consistent with the values found in pg_settings.
Another effect of the patch is that it become possible to start a cluster with external_pid_file='' in postgresql.conf It would have that same behavior as many other parameters. I started a 9.6 beta with the following lines in postgresql.conf and the only one that reported a problem is external_pid_file external_pid_file = '' # write an extra PID file unix_socket_group = '' # (change requires restart) bonjour_name = '' # defaults to the computer name ssl_ca_file = '' # (change requires restart) ssl_crl_file = '' # (change requires restart) krb_server_keyfile = '' shared_preload_libraries = '' # (change requires restart) synchronous_standby_names = '' # standby servers that provide sync rep cluster_name = '' # added to process titles if nonempty default_tablespace = '' # a tablespace name, '' uses the default temp_tablespaces = '' # a list of tablespace names, '' uses local_preload_libraries = '' session_preload_libraries = '' It seems to me that the general behavior it to consider an empty string as an unset parameter in postgresql.conf, why should external_pid_file be an exception ? I would prefer postgresql -C to report an empty string to be consistent with pg_settings in a backport of the fix. I also would prefer external_pid_file to behave as other parameters in the next versions. Regards. Alain On 22 June 2016 at 17:24, Tom Lane <t...@sss.pgh.pa.us> wrote: > alain radix <alain.ra...@gmail.com> writes: > > Here is a new patch with postmaster.c modification so that it check > about a > > empty string instead of a null pointer. > > Why should we adopt this, when there is already a better (more complete) > fix in git? > > I'm not sold on the wisdom of modifying the semantics of > external_pid_file, which is what this patch effectively does. I don't > know if anything outside our core code looks at that variable, but if > anything does, this would create issues for it. > > I'm even less sold on the wisdom of changing every other GUC that > has a null bootstrap default, which is what we would also have to do > in order to avoid the identical misbehavior for other -C cases. > > > The meaning is no more to avoid a core dump as you've done a change for > > that but to have the same result with postgres -C as with a request to > > pg_settings, an empty string when the parameter is not set. > > Well, maybe we should make -C print an empty string not "(nil)" for > such cases. You're right that we don't make a distinction between > NULL and "" in other code paths that display a string GUC, so doing > so here is a bit inconsistent. > > regards, tom lane > -- Alain Radix
