Re: [ Re: [patch] GUC source file and line number]

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: [ Re: [patch] GUC source file and line number]
Date: 2008-09-09 08:18:33
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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.


In response to


Browse pgsql-hackers by date

  From Date Subject
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