|From:||Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>|
|Cc:||tgl(at)sss(dot)pgh(dot)pa(dot)us, pgsql-hackers(at)postgresql(dot)org, coelho(at)cri(dot)ensmp(dot)fr, robertmhaas(at)gmail(dot)com|
|Subject:||Re: BUG: pg_stat_statements query normalization issues with combined queries|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
At Sat, 28 Jan 2017 11:52:20 +0800, Craig Ringer <craig(dot)ringer(at)2ndquadrant(dot)com> wrote in <CAMsr+YGX0uU5Rw0fOKFwxtg_5WxWOBQ=KiTHWiKrb65H67inwg(at)mail(dot)gmail(dot)com>
> On 28 Jan. 2017 02:08, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
> > By the way the existing comment for the hook,
> >> *
> >> * We provide a function hook variable that lets loadable plugins get
> >> * control when ProcessUtility is called. Such a plugin would normally
> >> * call standard_ProcessUtility().
> >> */
> > This is quite a matter of course for developers. planner_hook and
> > other ones don't have such a comment or have a one-liner at
> > most. Since this hook gets a good amount of commnet, it seems
> > better to just remove the existing one..
> Hm? I see pretty much the same wording for planner_hook:
Sorry, my eyes were very short-ranged. Surely most of them have
commnents with very similar phrase. ExplainOneQuery seems to be
one exception but I'll call ExplainOneQuery in the hook function,
Anyway I don't want to remove the comment in ProcessUtility since
now I know that is rather the common case.
> * To support loadable plugins that monitor or modify planner behavior,
> * we provide a hook variable that lets a plugin get control before and
> * after the standard planning process. The plugin would normally call
> * standard_planner().
> I think other places have copied-and-pasted this as well.
> The real problem with this is it isn't a correct specification of the
> contract: in most cases, the right thing to do is more like "call the
> previous occupant of the ProcessUtility_hook, or standard_ProcessUtility
> if there was none."
> Not sure if it's worth trying to be more precise in these comments,
> but if we do something about it, we need to hit all the places with
> this wording.
That seems bad. I'll prefer rather leaving them alone. Modifying
them wouldn't be so much gain if many of them already have such
> I'd rather leave it for the hooks documentation work that's being done
> elsewhere and have a README.hooks or something that discusses patterns
> common across hooks. Then we can just reference it.
> Otherwise we'll have a lot of repetitive comments. Especially once we
> mention that you can also ERROR out or (potentially) take no action at all.
Sorry for the noise..
NTT Open Source Software Center
|Next Message||Andrew Borodin||2017-01-30 07:28:11||Re: Review: GIN non-intrusive vacuum of posting tree|
|Previous Message||Ashutosh Bapat||2017-01-30 06:42:27||Re: Create a separate test file for exercising system views|