Possible typo/unclear comment in joinpath.c
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
Attachments:
v1-0001-Fix-extra_lateral_rels-typo.patchapplication/octet-stream; name=v1-0001-Fix-extra_lateral_rels-typo.patchDownload+3-4
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
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
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
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
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 -0500Simplify 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