Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])

On Fri, Aug 23, 2013 at 6:01 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Amit Kapila (amit(dot)kapila16(at)gmail(dot)com) wrote:
>> On Thu, Aug 22, 2013 at 6:06 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> > * Amit Kapila (amit(dot)kapila16(at)gmail(dot)com) wrote:
>> >> Enable/Disable reading of auto file
>> >> -----------------------------------------------------
>> >> a. Have a new include in postresql.conf
>> >> #include_auto_conf_file postgresql.auto.conf
>> >> as it is a special include, we can read this file relative to data
>> >> directory.
>> >>
>> >> Enable/Disable Alter System command
>> >> -----------------------------------------------------------
>> >> This can be achieved in 3 ways:
>> >> a. Check before executing Alter System if include directive is
>> >> disabled, then just issue a warning to user and proceed with command.
>> >> b. Check before executing Alter System if include directive is
>> >> disabled, then just issue an error and stop.
>> >
>> > It doesn't make sense for it to be a 'warning' with this- the
>> > parameter specifies the file to use. If you don't know what file to
>> > use, how you can possibly do anything but return an error?
>>
>> As the file and location are fixed, we can go-ahead and write to
>> it, but I think now we are deciding
>> if someone disables include dir, then we can just disable Alter
>> System, so it is better to return error in such
>> situation.
>
> It wouldn't be fixed with this approach.
>
>> > Note that I *like* that about this approach.
>> >
>> > There are a few other considerations with this-
>> >
>> > - What should the default be? (Still thinking 'off' myself)
>> default 'off' is a safe option, as it won't allow users to make
>> any change to parameter values until/unless they
>> read from manual, how to use it and what can go wrong, on the
>> other side it will be bit hassle for user to use this
>> command. I think 'on' would be better.
>
> Yeah, no, I still think 'off' would be best for this particular option.
>
>> > - What happens if the user specifies 'postgresql.conf'? I'm thinking we
>> > would disallow such insanity (as that's what it is, imv..) by having
>> > an identifier in the file that this is the PG "auto conf" file.
>> I think we can detect by name and give error.
>> > - Should we have such an identifier in auto.conf to indicate that we
>> > created it, to prevent the user from setting it to something they
>> > shouldn't?
>> I think if user plays with this file manually, it can lead to
>> problems, that's why earlier we have
>> decided to keep a note on top of file which will indicate, do not
>> edit this file manually.
>> I believe that should be sufficient.
>
> I agree that having such a disclaimer at the top of the file is a good
> idea. I'm not completely convinced that's sufficient but it's certainly
> better than nothing.
>
>> > - What's the "bootstrap" mode; iow, if a user enables the option but the
>> > file doesn't exist, what do we do? With this approach, I'd be
>> > inclined to say we simply create it and put the marker to indicate
>> > it's "our" file.
>>
>> Alter System will create the file if doesn't exist.
>
> ... Only if it's enabled though.

Yes.
>
>> > - Should we allow it to be outside of the data dir? We could simply log
>> > an error and ignore the parameter if it's more than a simple filename.
>>
>> This should be an error, the file location and name will be fixed.
>
> Not with this proposal... If it's fixed then it makes no sense to make it
> look like it can be modified.

This proposal is to have a special include which user can only use
for enable/disable
which means he can remove symbol '#' or add '#'.
We cannot stop user from changing file name or add some different
location, but that can lead to problems.
We can have a note on top of this include indicating it is only for
enable/disable.

I think if we add more meaning to it, like allow user to change it,
handling and defining of that will be bit complex.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com