Final(?) proposal on GUC hook extensions - Mailing list pgsql-hackers

I've been working on merging the last few variable.c variables into GUC.
Here are some notes on what I'm planning to modify in GUC to make that
possible.

1. The variable.c routines have quite a few specialized error reports
(eg, rejecting intervals larger than months in SET TIME ZONE).  While
we could drop all of 'em and just fall back to GUC's generic "invalid
value for this variable" message, that seems fairly user-unfriendly.
I am also dissatisfied with the separation between parse_hook and
assign_hook for a GUC variable: in every case, that is resulting in
having to do the parsing of the variable twice, leading to cluttered
or duplicate code.  What I'm thinking of doing is getting rid of the
separate parse_hook and instead defining assign_hooks to have the
signature
bool assign_hook (const char *newvalue, bool interactive)

If interactive is TRUE it's okay to throw an elog rather than returning;
if interactive is FALSE (ie, we're reading postgresql.conf) then do not
throw an elog, just return false on bad input.  (Possibly the routines
could emit helpful messages via elog(LOG) instead of elog(ERROR) when
interactive is false.)  Returning TRUE indicates successful assignment,
returning FALSE indicates unsuccessful; in the interactive case GUC will
throw a generic elog message on FALSE return.

AFAICT the main reason for separating parse_hook and assign_hook in
the current GUC code is to allow failure to be detected before storing
the new value into the state array, rather than after --- but given
the changes we intend to make to allow rollback of SET after error,
this is no longer essential.

2. As previously suggested, I will add a show_hook (signature
char *show_hook(void) seems sufficient) to allow variable-specific
code to override computation of what is to be displayed for SHOW.

3. We also appear to need a reset_hook to override the default
action of just calling the assign_hook with the stored default value.
Here I'm envisioning void reset_hook(const char *newvalue, bool isAll)
where the stored default value is passed (in case needed) and isAll
distinguishes resetting the individual variable vs RESET ALL.  Per
previous discussion, some variables may want to ignore RESET ALL
and this gives them an opportunity to do it.  An alternative possibility
that may prove simpler is to still use assign_hook for resetting, but to
pass it an additional "context" parameter to let it know whether it's
being called for SET, RESET, or RESET ALL.

4. Per previous discussion, I have written code that "flattens" the
node-list output of gram.y into a string.  Experimenting with this,
I find that those variables that actually want to read their input
value as a list (eg, search_path, datestyle) would like the list
elements to be quoted when not simple identifiers.  But if the
flattening code always does this, then lots of other variable-parsing
code has to be prepared to cope with quotes around its input.  I am
thinking of adding a flag value to the GUC arrays that tells the
flattening code whether to quote list elements or not --- we'd need
to look up the target variable before flattening the input, but that
seems no big problem.  When the flag is not set, we could probably
reject multiple elements in the input list out-of-hand.  (The current
CVS tip has this behavior hard-wired for all GUC variables, but that
will not do for search_path.)


Thomas had suggested that we think about further extensibility of GUC,
such as letting loadable modules add variables to the set of known
variables.  That seems like a good idea to me but I've not got time to
pursue it right now.  We'd have to settle some interesting questions
first, anyway (like do we want loadable modules to get loaded into the
postmaster, and if not how do we deal with postgresql.conf entries
intended for loadable modules).

Comments?
        regards, tom lane

PS: once I get done with this, I will get started on the SET SESSION/
SET LOCAL rollback behavior that I think we agreed to in the last thread.