Mmm.. At Sat, 28 Jan 2017 11:52:20 +0800, Craig Ringer <craig.rin...@2ndquadrant.com> wrote in <CAMsr+YGX0uU5Rw0fOKFwxtg_5WxWOBQ=kithwikrb65h67i...@mail.gmail.com> > On 28 Jan. 2017 02:08, "Tom Lane" <t...@sss.pgh.pa.us> wrote: > > Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.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, though:p 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 comment. > 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.. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
