On Wed, Aug 28, 2013 at 3:10 PM, Stephen Frost wrote:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.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

Search Discussions

Discussion Posts

Previous

Follow ups

Related Discussions

People

Translate

site design / logo © 2017 Grokbase