Re: Should we get rid of custom_variable_classes altogether?

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> On Sun, Oct 2, 2011 at 10:05 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> So at this point I'd vote for just dropping it and always allowing
>> custom (that is, qualified) GUC names to be set, whether the prefix
>> corresponds to any loaded module or not.

> Sounds sensible. One less thing to configure is a good thing.

Attached is a draft patch for that.

While working on this I got annoyed at our cheesy handling of the
situation where a "placeholder" value has to be turned into a real
setting, which happens when the corresponding extension gets loaded.
There are several issues:

1. If it's a SUSET variable, a preceding attempt to set the value via
SET will fail even if you're a superuser, for example

regression=# set plpgsql.variable_conflict = use_variable;
SET
regression=# load 'plpgsql';
ERROR: permission denied to set parameter "plpgsql.variable_conflict"

The reason for that is that define_custom_variable doesn't know whether
the pre-existing value was set by a superuser, so it must assume the
worst. Seems like we could easily fix that by having set_config_option
set a flag in the GUC variable noting whether a SET was done by a
superuser or not.

2. If you do get an error while re-assigning the pre-existing value of
the variable, it's thrown as an ERROR. This is really pretty nasty
because it'll abort execution of the extension module's init function;
for example, a likely consequence is that other custom variables of
the module don't set set up correctly, and it could easily be a lot
worse if there are other things the init function hasn't done yet.

I think we need to arrange that set_config_option only reports failures
to apply such values as WARNING, not ERROR. There isn't anything in its
present API that could be used for that, but perhaps we could add a new
enum variant for "action" that commands it.

3. We aren't very careful about preserving the reset value of the
variable, in case it's different from the active value (which could
happen if the user did a SET and there's also a value from the
postgresql.conf file).

This seems like it just requires working a bit harder in
define_custom_variable, to reapply the reset value as well as the
current value of the variable.

Any objections to fixing that stuff, while I'm looking at it?

regards, tom lane