Re: [gsmith@gregsmith.com: Re: [patch] GUC source file and line number]

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Greg just sent me this patch, augmenting the one I sent to add source
> file and line to GUC vars; Greg's patch adds a column with the default
> value of each var.

I haven't tested, but doesn't this lose the source-location information
if a setting acquired from the config file is temporarily overridden via
SET (consider SET LOCAL, or a SET in a rolled-back xact)?  It'll go to
NULL and not come back.

Since there is no possibility that any module outside GUC should ever
supply a config file location, I don't think that changing the API for
set_config_option is a good idea.  Instead have ProcessConfigFile()
call some internal function that's not used by anyone else, and let
set_config_option assume that it's setting a non-file-sourced value.
That'd reduce the footprint of the patch quite a bit.

I dislike using the terminology "default" so cavalierly, because that
is a fairly slippery concept in GUC.  Default for whom, with respect
to what?  It looks like the patch actually means it to be the boot_val,
but I think a lot of users would think that "default" refers to the
reset value, ie, what their setting will be if they haven't said SET.
The fact that the session default didn't necessarily come from the
config file (see ALTER USER SET, ALTER DATABASE SET) complicates matters
still more.  *Please* use some other word than "default" to title that
column.  Also, I think that a reasonable case could be made for exposing
both boot_val and reset_val in the view --- if there is a use for one,
there is likely to be a use for the other.
        regards, tom lane