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

> > - 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.



Attachment: signature.asc
Description: Digital signature

Reply via email to