On Wed, Jul 9, 2014 at 11:05 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Wed, Jul 9, 2014 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Amit Kapila <amit.kapila16@gmail.com> writes:
>> > On Wed, Jul 9, 2014 at 6:40 AM, Mark Kirkwood
>> > <mark.kirkwood@catalyst.net.nz>
>> > wrote:
>> >> Yes, but even well behaved users will see this type of error, because
>> >> initdb uncomments certain values (ones that are dead certs for being
>> >> changed via ALTER SYSTEM subsequently like shared_buffers), and then -
>> >> bang! your next reload gets that "your postgresql.conf contains errors"
>> >> message.
>>
>> > That is the reason, why I have suggested up-thread that uncommented
>> > values should go to postgresql.auto.conf, that will avoid any such
>> > observations for a well-behaved user.
>>
>> Uh, what? That sounds like you are proposing that postgresql.conf itself
>> is a dead letter. Which is not going to fly. We had that conversation
>> already.
>
> It might sound like that but honestly my intention was to just ease the use
> for users who just want to rely on Alter System.
>
>
>> The right way to fix this is just to avoid processing entries that get
>> overridden later in the configuration file scan. That won't cause anyone
>> to get upset about how their old habits no longer work.
>
> Okay. As mentioned upthread, I have fixed by ensuring that for duplicate
> config params, retain only which comes later during parsing.
> I think it might have been bit simpler to fix, if we try to fix after
> parsing
> is complete, but I think for that we might need to replicate the logic
> at multiple places.
ISTM that the patch works fine. Only concern is that the logic needs
O(n^2) comparison, which may cause performance problem. But
"n" in O(n^2) is the number of uncommented parameters and I don't
think it's so big, ISTM I can live with the logic...
Regards,
--
Fujii Masao