Horton

Thread

Commits

  1. Fix obsolete comments referencing JoinPathExtraData.extra_lateral_rels.

  2. Simplify LATERAL-related calculations within add_paths_to_joinrel().

  1. Possible typo/unclear comment in joinpath.c

    James Coleman <jtc331@gmail.com> — 2021-04-14T15:36:38Z

    In joinpath.c three times we reference "extra_lateral_rels" (with
underscores like it's a field), but as far as I can tell that's not a
field anywhere in the source code, and looking at the code that
follows it seems like it should be referencing "lateral_relids" (and
the "extra" is really "extra [in relation to relids]").

Assuming that interpretation is correct, I'd attached a patch to
change all three occurrences to "extra lateral_relids" to reduce
confusion.

Thanks,
James

  2. Re: Possible typo/unclear comment in joinpath.c

    Justin Pryzby <pryzby@telsasoft.com> — 2021-04-14T16:42:53Z

    On Wed, Apr 14, 2021 at 11:36:38AM -0400, James Coleman wrote:
> In joinpath.c three times we reference "extra_lateral_rels" (with
> underscores like it's a field), but as far as I can tell that's not a
> field anywhere in the source code, and looking at the code that
> follows it seems like it should be referencing "lateral_relids" (and
> the "extra" is really "extra [in relation to relids]").

It looks like a loose end from 

commit edca44b1525b3d591263d032dc4fe500ea771e0e
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Mon Dec 7 18:56:14 2015 -0500

    Simplify LATERAL-related calculations within add_paths_to_joinrel().

-- 
Justin

  3. Re: Possible typo/unclear comment in joinpath.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-04-14T17:27:22Z

    Justin Pryzby <pryzby@telsasoft.com> writes:
> On Wed, Apr 14, 2021 at 11:36:38AM -0400, James Coleman wrote:
>> In joinpath.c three times we reference "extra_lateral_rels" (with
>> underscores like it's a field), but as far as I can tell that's not a
>> field anywhere in the source code, and looking at the code that
>> follows it seems like it should be referencing "lateral_relids" (and
>> the "extra" is really "extra [in relation to relids]").

> It looks like a loose end from 

> commit edca44b1525b3d591263d032dc4fe500ea771e0e
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Date:   Mon Dec 7 18:56:14 2015 -0500

>     Simplify LATERAL-related calculations within add_paths_to_joinrel().

Yeah :-(.  I'm usually pretty careful about grepping for comment
references as well as code references to a field when I do something
like that, but obviously I missed that step that time.

Will fix, thanks James!

			regards, tom lane

  4. Re: Possible typo/unclear comment in joinpath.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-04-14T18:32:40Z

    I wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
>> It looks like a loose end from 
>> commit edca44b1525b3d591263d032dc4fe500ea771e0e

> Yeah :-(.  I'm usually pretty careful about grepping for comment
> references as well as code references to a field when I do something
> like that, but obviously I missed that step that time.

No, I take that back.  There were no references to extra_lateral_rels
after that commit; these comments were added by 45be99f8c, about
six weeks later.  The latter was a pretty large patch and had
presumably been under development for quite some time, so the comments
were probably accurate when written but didn't get updated.

			regards, tom lane

  5. Re: Possible typo/unclear comment in joinpath.c

    Robert Haas <robertmhaas@gmail.com> — 2021-04-14T19:46:11Z

    On Wed, Apr 14, 2021 at 2:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> No, I take that back.  There were no references to extra_lateral_rels
> after that commit; these comments were added by 45be99f8c, about
> six weeks later.  The latter was a pretty large patch and had
> presumably been under development for quite some time, so the comments
> were probably accurate when written but didn't get updated.

Woops.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  6. Re: Possible typo/unclear comment in joinpath.c

    James Coleman <jtc331@gmail.com> — 2021-04-14T20:36:22Z

    On Wed, Apr 14, 2021 at 1:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > On Wed, Apr 14, 2021 at 11:36:38AM -0400, James Coleman wrote:
> >> In joinpath.c three times we reference "extra_lateral_rels" (with
> >> underscores like it's a field), but as far as I can tell that's not a
> >> field anywhere in the source code, and looking at the code that
> >> follows it seems like it should be referencing "lateral_relids" (and
> >> the "extra" is really "extra [in relation to relids]").
>
> > It looks like a loose end from
>
> > commit edca44b1525b3d591263d032dc4fe500ea771e0e
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > Date:   Mon Dec 7 18:56:14 2015 -0500
>
> >     Simplify LATERAL-related calculations within add_paths_to_joinrel().
>
> Yeah :-(.  I'm usually pretty careful about grepping for comment
> references as well as code references to a field when I do something
> like that, but obviously I missed that step that time.
>
> Will fix, thanks James!
>
>                         regards, tom lane

Thanks for fixing, Tom!

James