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

On Wed, Aug 28, 2013 at 3:10 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Stephen Frost <sfrost(at)snowman(dot)net> writes:
>> > While I appreciate that there are bootstrap-type issues with this, I
>> > really don't like this idea of "later stuff can just override earlier
>> > stuff".
>>
>> > include files and conf.d-style options are for breaking the config up,
>> > not to allow you to override options because a file came later than an
>> > earlier file. Our particular implementation of config-file reading
>> > happens to lend itself to later-definition-wins, but that's really
>> > counter-intuitive for anyone unfamiliar with PG, imv.
>>
>> I don't follow this argument at all. Do you know any software with text
>> config files that will act differently from this if the same setting is
>> listed twice? "Last one wins" is certainly what I'd expect.
>
> Have you tried having the same mount specified in multiple files in
> fstab.d..? Also, aiui (not a big exim fan personally), duplicate
> definitions in an exim4/conf.d would result in an error. Playing around
> in apache2/sites-enabled, it appears to be "first wins" wrt virtual
> hosts.
>
> There's a number of cases where only a single value is being set and
> subseqeunt files are 'additive' (eg: ld.so.conf.d), so they don't have
> this issue. Similar to that are script directories, which simply run a
> set of scripts in the $DIR.d and, as it's the scripts themselves which
> are the 'parameters', and being files in a directory, you can't
> duplicate them. Others (eg: pam.d) define the file name to be an
> enclosing context, again preventing duplication by using the filename
> itself.
>
> There are counter-examples also; sysctl.d will replace what's already
> been set, so perhaps it simply depends on an individual's experience.
> For my part, I'd much prefer an error or warning saying "you've got
> duplicate definitions of X" than a last-wins approach, though perhaps
> that's just because I like things to be very explicit and clear.
> Allowing multiple definitions of the same parameter to be valid isn't
> 'clear' to me.

This may be true, but I think it's got little if anything to do with
the current patch. There are lots of things that I don't like about
our GUC machinery, too, but the goal of this thread shouldn't be to
fix them all, but to get a useful piece of functionality added.

The thing that we should keep in mind here is that it's *already*
possible - TODAY - for users to overwrite postgresql.conf with a new
set of settings. pgAdmin has this functionality built-in; it only
requires that you install the "adminpack" contrib module, which we
ship. According to Dave Page, pgAdmin will even offer to run CREATE
EXTENSION adminpack for you, if you haven't done that already. Also
according to Dave Page, if two users try to use it concurrently, you
will get exactly the sort of awful results that you might expect. If
it gets killed halfway through rewriting the file, too bad for you.

The sorts of watered-down half-features being proposed here are not
going to do anything to address that situation. If there are
restrictions on what GUCs can be changed with this feature, or if the
feature is disabled by default, or if you can only use it when the
moon is full and you hop on your left foot while spinning around
sideways, then pgAdmin (and any other, similar tools) are just going
to keep doing what they do today in the same crappy, unsafe way they
currently do it. Meanwhile, people who use psql are going to continue
to find themselves without a reasonable API for doing the same thing
that can be done easily using pgAdmin.

I think the goals of this patch should be to (1) let users of other
clients manipulate the startup configuration just as easily as we can
already do using pgAdmin and (2) remove some of the concurrency
hazards that pgAdmin introduces, for example by using locking and
atomic renames. Restricting the functionality to something less than
what pgAdmin provides will just cause people to ignore the new
mechanism and use the one that already exists and, by and large,
works. And trying to revise other aspects of how GUCs and config
files work as part of this effort is not reasonably related to this
patch, and should be kept out of the discussion.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company