On Jul 16, 2011, at 9:55 PM, Tom Lane wrote:
> I wrote:
>> I think that it might be sensible to have the following behavior:
>> 1. Parse the file, where "parse" means collect all the name = value
>> pairs. Bail out if we find any syntax errors at that level of detail.
>> (With this patch, we could report some or all of the syntax errors
>> 2. Tentatively apply the new custom_variable_classes setting if any.
>> 3. Check to see whether all the "name"s are valid. If not, report
>> the ones that aren't and bail out.
>> 4. Apply each "value". If some of them aren't valid, report that,
>> but continue, and apply all the ones that are valid.
>> We can expect that the postmaster and all backends will agree on the
>> results of steps 1 through 3. They might differ as to the validity
>> of individual values in step 4 (as per my example of a setting that
>> depends on database_encoding), but we should never end up with a
>> situation where a globally correct value is not globally applied.
Attached is my first attempt to implement your plan. Basically, I've
reshuffled pieces of the ProcessConfigFile on top of my previous patch,
dropped verification calls of set_config_option and moved the check for
custom_variable_class existence right inside the loop that assigns new values
to GUC variables.
I'd think that removal of custom_variable_classes or setting it from the
extensions could be a separate patch.
I appreciate your comments and suggestions.
> I thought some more about this, and it occurred to me that it's not that
> hard to foresee a situation where different backends might have
> different opinions about the results of step 3, ie, different ideas
> about the set of valid GUC names. This could arise as a result of some
> of them having a particular extension module loaded and others not.
> Right now, whether or not you have say plpgsql loaded will not affect
> your ability to do "SET plpgsql.junk = foobar" --- as long as "plpgsql"
> is listed in custom_variable_classes, we'll accept the command and
> create a placeholder variable for plpgsql.junk. But it seems perfectly
> plausible that we might someday try to tighten that up so that once a
> module has done EmitWarningsOnPlaceholders("plpgsql"), we'll no longer
> allow creation of new placeholders named plpgsql.something. If we did
> that, we could no longer assume that all backends agree on the set of
> legal GUC variable names.
> So that seems like an argument --- not terribly strong, but still an
> argument --- for doing what I suggested next:
>> The original argument for the current behavior was to avoid applying
>> settings from a thoroughly munged config file, but I think that the
>> checks involved in steps 1-3 would be sufficient to reject files that
>> had major problems. It's possible that step 1 is really sufficient to
>> cover the issue, in which case we could drop the separate step-3 pass
>> and just treat invalid GUC names as a reason to ignore the particular
>> line rather than the whole file. That would make things simpler and
>> faster, and maybe less surprising too.
> IOW, I'm now pretty well convinced that so long as the configuration
> file is syntactically valid, we should go ahead and attempt to apply
> each name = value setting individually, without allowing the invalidity
> of any one name or value to prevent others from being applied.
Command Prompt, Inc. http://www.CommandPrompt.com
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
In response to
pgsql-hackers by date
|Next:||From: pasman pasmański||Date: 2011-07-25 18:56:52|
|Subject: Re: problem with compiling beta3 on mingw32+WinXP|
|Previous:||From: Heikki Linnakangas||Date: 2011-07-25 18:52:16|
|Subject: Re: WIP: Fast GiST index build|