|From:||Magnus Hagander <magnus(at)hagander(dot)net>|
|To:||Alvaro Herrera <alvherre(at)commandprompt(dot)com>|
|Cc:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, gsmith(at)gregsmith(dot)com|
|Subject:||Re: [firstname.lastname@example.org: Re: [patch] GUC source file and line number]|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Alvaro Herrera wrote:
> Tom Lane wrote:
>> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>>> Tom Lane wrote:
>>> Hmm, I didn't recheck after Greg's patch, but in mine, it doesn't,
>>> because the location is saved as "reset location" and restored when the
>>> variable is reset. It worked fine in all cases I tested.
>> Hmm. Actually, why is there a need to save and restore at all? There
>> can certainly never be more than one recorded config-file location per
>> variable. What about saying that each variable has one and only one
>> filename/linenumber, but whether these are relevant to the current value
>> is determined by whether the current value's source is S_FILE?
> Hmm, this does make the patch a lot simpler. Attached. (Magnus was
> visionary enough to put the correct test in the pg_settings definition.)
Yeah, it does look at lot simpler. And it certainly takes away the
pieces of code of mine that I was entirely unable to make working :-)
> I also dropped the change to set_config_option, and added a new routine
> to set the source file/line, as you suggested elsewhere. The only
> problem is that we now have two bsearch calls for each option set in a
> config file ... I don't want to change set_config_option just to be
> able to return the struct config_generic for this routine's sake ...
> Better ideas? Is this OK as is?
Well, it's not like it's a performance critical path, is it? I think we
should be ok.
> I noticed some weird things when the config files contain errors, but I
> think it's outside this patch's scope.
> (I dropped the "default" stuff for now, as it doesn't seem that a
> consensus has been reached on that topic.)
This is one of the reasons I suggested keeping that one as a separate
patch in the first place. The other main reason being that once it gets
applied, you really want it to be two different revisions, to clearly
keep them apart ;-) I still think we should eventually get both in
there, but let's treat them as separate entities.
|Next Message||Greg Smith||2008-09-09 08:50:27||Re: Some newbie questions|
|Previous Message||Greg Smith||2008-09-09 08:15:59||Re: Move src/tools/backend/ to wiki|