Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]

On Tuesday, January 22, 2013 7:10 PM Zoltán Böszörményi wrote:
> 2013-01-22 13:32 keltezéssel, Amit kapila írta:
> > On Saturday, January 19, 2013 2:37 AM Boszormenyi Zoltan wrote:
> > 2013-01-18 21:48 keltezéssel, Boszormenyi Zoltan írta:
> >> 2013-01-18 21:32 keltezéssel, Tom Lane írta:
> >>> Boszormenyi Zoltan <zb(at)cybertec(dot)at> writes:
> >>>> 2013-01-18 11:05 keltezéssel, Amit kapila írta:
> >>>>>> On using mktemp, linux compilation gives below warning
> >>>>>> warning: the use of `mktemp' is dangerous, better use `mkstemp'
> >
> >>>> Everywhere else that we need to do something like this, we just
> use our
> >>>> own PID to disambiguate, ie
> >>>> sprintf(tempfilename, "/path/to/file.%d", (int) getpid());
> >>>> There is no need to deviate from that pattern or introduce
> portability
> >>>> issues, since we can reasonably assume that no non-Postgres
> programs are
> >>>> creating files in this directory.
> >>> Thanks for the enlightenment, I will post a new version soon.
> >> Here it is.
> > The patch sent by you works fine.
> > It needs small modification as below:
> >
> > The "auto.conf.d" directory should follow the postgresql.conf file
> directory not the data_directory.
> > The same is validated while parsing the postgresql.conf configuration
> file.
> >
> > Patch is changed to use the postgresql.conf file directory as below.
> >
> > StrNCpy(ConfigFileDir, ConfigFileName, sizeof(ConfigFileDir));
> > get_parent_directory(ConfigFileDir);
> > /* Frame auto conf name and auto conf sample temp file name */
> > snprintf(AutoConfFileName, sizeof(AutoConfFileName), "%s/%s/%s",
> > ConfigFileDir,
> > PG_AUTOCONF_DIR,
> > PG_AUTOCONF_FILENAME);
>
> Maybe it's just me but I prefer to have identical
> settings across all replicated servers. But I agree
> that there can be use cases with different setups.
>
> All in all, this change makes it necessary to run the
> same SET PERSISTENT statements on all slave servers,
> too, to make them identical again if the configuration
> is separated from the data directory (like on Debian
> or Ubuntu using the default packages). This needs to be
> documented explicitly.

SET PERSISTENT command needs to run on all slave servers even if the path we
take w.r.t data directory
unless user takes backup after command.

> >
> > This closes all comments raised till now for this patch.
> > Kindly let me know if you feel something is missing?
>
> I can't think of anything else.

In that case can you mark it as "Ready For Committer"

With Regards,
Amit Kapila.