Re: postgresql.auto.conf and reload

On Tue, Aug 12, 2014 at 1:23 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Aug 11, 2014 at 11:08 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>>
>> While updating the patch, I found that the ConfigVariable which
>> is removed from list has the fields that the memory has been already
>> allocated but we forgot to free them. So I included the code to free
>> them in the patch.
>
> Yes, that is right.
>
> ! /*
> ! * Pick up only the data_directory if DataDir is not set, which
> ! * means that the configuration file is read for the first time and
> ! * PG_AUTOCONF_FILENAME file cannot be read yet. In this case,
> ! * we shouldn't pick any settings except the data_directory
> ! * from postgresql.conf because they might be overwritten
> ! * with the settings in PG_AUTOCONF_FILENAME file which will be
> ! * read later. OTOH, since it's ensured that data_directory doesn't
> ! * exist in PG_AUTOCONF_FILENAME file, it will never be overwritten
> ! * later.
> ! */
> ! else
>
> It is bit incovinient to read this part of code, some other places in
> same file use comment inside else construct which seems to be
> better. one example is as below:

Yep, updated that way.

>
> else
> {
> /*
> * ordinary variable, append to list. For multiple items of
> * same parameter, retain only which comes later.
> */
>
>
> I have marked this as Ready For Committer.

Thanks for the review! Committed.

Regards,

--
Fujii Masao