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

On Fri, Aug 23, 2013 at 6:01 PM, Stephen Frost <sfrost@snowman.net> wrote:
> * Amit Kapila (amit.kapila16@gmail.com) wrote:
>> On Thu, Aug 22, 2013 at 6:06 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> > * Amit Kapila (amit.kapila16@gmail.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
addsome 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