opt: use paired joins for left semi inverted joins#55986
opt: use paired joins for left semi inverted joins#55986craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @sumeerbhola)
pkg/sql/opt/xform/join_funcs.go, line 467 at r1 (raw file):
continuationCol := opt.ColumnID(0) invertedJoinType := joinType // Anti/semi joins are converted to a pair consisting of a left join and
is there any way we can use an inner join for the first join when the second is a semi join?
pkg/sql/opt/xform/rules/join.opt, line 151 at r1 (raw file):
# custom function for more details. # TODO(rytaft): Add support for SemiJoin. Currently it is supported by first # converting it to InnerJoin using the rule ConvertSemiToInnerJoin.
remove TODO
5863460 to
e12a12a
Compare
sumeerbhola
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/sql/opt/xform/join_funcs.go, line 467 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
is there any way we can use an inner join for the first join when the second is a semi join?
That was silly of me. I thought I'd forgotten to add support for a continuation column for inner joins, but I had actually added that for exactly this reason (and it's even explained in the comment for InvertedJoinerSpec).
Done.
pkg/sql/opt/xform/rules/join.opt, line 151 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
remove TODO
Done
rytaft
left a comment
There was a problem hiding this comment.
Nice!
Reviewed 6 of 6 files at r2.
Reviewable status:
|
pkg/sql/opt/xform/join_funcs.go, line 475 at r2 (raw file):
[nit] maybe rename this function to just be PairedJoin instead of PairedLeftJoin |
e12a12a to
c6dc1db
Compare
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/xform/join_funcs.go, line 475 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
[nit] maybe rename this function to just be PairedJoin instead of PairedLeftJoin
The original intention was for "left join" to refer to the overall join (which is a left semi/anti/outer join). But I see why this can be confusing. I've renamed it as you suggested, and added a sentence to the function comment.
c6dc1db to
d0e2a47
Compare
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:
Semi joins using the inverted index used to be converted to an inner inverted join followed by an inner lookup join and a distinct for de-duplication. They are now converted to paired-joins consisting of an inner inverted join followed by a left semi lookup join, which is more efficient. Release note: None
d0e2a47 to
dd16fc3
Compare
|
Don't forget to update the PR message to change left outer -> inner |
sumeerbhola
left a comment
There was a problem hiding this comment.
Thanks for the reminder. Done. I wish it would sync from the commit message for a single commit PR.
Reviewable status:
|
Yea... that would be nice |
|
bors r+ |
|
Build succeeded: |
Semi joins using the inverted index used to be converted
to an inner inverted join followed by an inner lookup join
and a distinct for de-duplication. They are now converted
to paired-joins consisting of an inner inverted join
followed by a left semi lookup join, which is more
efficient.
Release note: None