On Fri, Sep 29, 2023 at 2:26 PM Bruce Momjian <br...@momjian.us> wrote: > > On Mon, Jan 23, 2023 at 06:42:45PM -0500, James Coleman wrote: > > On Mon, Jan 23, 2023 at 4:07 PM James Coleman <jtc...@gmail.com> wrote: > > > > > > On Mon, Jan 23, 2023 at 3:41 PM Robert Haas <robertmh...@gmail.com> wrote: > > > > > > > > On Mon, Jan 23, 2023 at 3:19 PM James Coleman <jtc...@gmail.com> wrote: > > > > > On Mon, Jan 23, 2023 at 1:26 PM Robert Haas <robertmh...@gmail.com> > > > > > wrote: > > > > > > On Mon, Jan 23, 2023 at 8:31 AM James Coleman <jtc...@gmail.com> > > > > > > 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.
Thanks! Regards, James Coleman
