Re: Fix incorrect comment reference

From: James Coleman <jtc331(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix incorrect comment reference
Date: 2023-01-23 20:19:14
Message-ID: CAAaqYe_mqK1AWkVB7Jp5Q3RBOGDEAmeBKF3xd9sPWyHXvmr=FQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jan 23, 2023 at 1:26 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Jan 23, 2023 at 8:31 AM James Coleman <jtc331(at)gmail(dot)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)?

James Coleman

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-01-23 20:26:19 Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?
Previous Message David G. Johnston 2023-01-23 20:09:36 v16 GRANT role TO role needs a multi-option setting capability