rowexec: remove unnecessary joiner member#67326
rowexec: remove unnecessary joiner member#67326craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @andreimatei, @nvanbenschoten, and @sumeerbhola)
pkg/sql/rowexec/joinreader_strategies.go, line 429 at r1 (raw file):
} var err error containerIdx, err = s.lookedUpRows.AddRow(ctx, row)
nit: why not call this lookedUpRowIdx?
|
Thanks for doing this. I should have removed this when switching |
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewable status:
The joiner had a field tracking a confusing lookup row index. Besides being confusing, this member obscured the fact that it was equal to the ordinal returned by inserting rows into a disk container - and thus that the respective ordinal is used. Release note: None
64700a7 to
748c79e
Compare
andreimatei
left a comment
There was a problem hiding this comment.
bors r+
Reviewable status:
pkg/sql/rowexec/joinreader_strategies.go, line 429 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit: why not call this
lookedUpRowIdx?
I went back and forth, but ultimately I left it; I think it's good here to emphasize strongly the link to the cointainer. Since this method deals with a single thing - a looked-up row - emphasizing that the id identifies it is less interesting.
|
Build failed (retrying...): |
|
Build succeeded: |
The joiner had a field tracking a confusing lookup row index. Besides
being confusing, this member obscured the fact that it was equal to the
ordinal returned by inserting rows into a disk container - and thus that
the respective ordinal is used.
Release note: None