public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bruce Momjian <[email protected]>
To: James Coleman <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Fix incorrect comment reference
Date: Fri, 29 Sep 2023 14:26:19 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAaqYe9F6uoMhAr+8rMLwvGzaKaSknPA0Wi3Ehtv8pbSYmJq-Q@mail.gmail.com>
References: <CAAaqYe8YVimoZz3u7W8u5J=DdSi1DPJtPSQ9r=wLRDBQXA2gHw@mail.gmail.com>
<CA+TgmoYw=7p5D1uwtrOmzVZ=JjG4gyiCLLVj0e2xQ+m026VaTw@mail.gmail.com>
<CAAaqYe_mqK1AWkVB7Jp5Q3RBOGDEAmeBKF3xd9sPWyHXvmr=FQ@mail.gmail.com>
<CA+TgmoYkocpPWe12MSLPnUDsFTxQMVUf+a3g2bnrPvbG7LcYfw@mail.gmail.com>
<CAAaqYe_u8ELCr1raCNbFLd=cUo69cS7kUmrchDGtSKYM2OJtXA@mail.gmail.com>
<CAAaqYe9F6uoMhAr+8rMLwvGzaKaSknPA0Wi3Ehtv8pbSYmJq-Q@mail.gmail.com>
On Mon, Jan 23, 2023 at 06:42:45PM -0500, James Coleman wrote:
> On Mon, Jan 23, 2023 at 4:07 PM James Coleman <[email protected]> wrote:
> >
> > On Mon, Jan 23, 2023 at 3:41 PM Robert Haas <[email protected]> wrote:
> > >
> > > On Mon, Jan 23, 2023 at 3:19 PM James Coleman <[email protected]> wrote:
> > > > On Mon, Jan 23, 2023 at 1:26 PM Robert Haas <[email protected]> wrote:
> > > > > On Mon, Jan 23, 2023 at 8:31 AM James Coleman <[email protected]> wrote:
> > > > > > See the attached for a simple comment fix -- the referenced
> > > > > > generate_useful_gather_paths call isn't in grouping_planner it's in
> > > > > > apply_scanjoin_target_to_paths.
> > > > >
> > > > > The intended reading of the comment is not clear. Is it telling you to
> > > > > look at grouping_planner because that's where we
> > > > > generate_useful_gather_paths, or is it telling you to look there to
> > > > > see how we get the final target list together? If it's the former,
> > > > > then your fix is correct. If the latter, it's fine as it is.
> > > > >
> > > > > The real answer is probably that some years ago both things happened
> > > > > in that function. We've moved on from there, but I'm still not sure
> > > > > what the most useful phrasing of the comment is.
> > > >
> > > > Yeah, almost certainly, and the comments just didn't keep up.
> > > >
> > > > Would you prefer something that notes both that the broader concern is
> > > > happening via the grouping_planner() stage but still points to the
> > > > proper callsite (so that people don't go looking for that confused)?
> > >
> > > I don't really have a strong view on what the best thing to do is. I
> > > was just pointing out that the comment might not be quite so obviously
> > > wrong as you were supposing.
> >
> > "Wrong" is certainly too strong; my apologies.
> >
> > I'm really just hoping to improve it for future readers to save them
> > some confusion I had initially reading it.
>
> Updated patch attached.
Patch applied.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Fix incorrect comment reference
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox