exec: fix output batches of LEFT SEMI for hash and merge joiners #39294
Conversation
|
This change is |
|
The issue presented itself in the distributed case during serialization: |
|
I added DNM label until #39299 is fixed (the current reproduction relies on this bug being present). |
|
I understand the cause and fixed in #39331 so removed the DNM label. |
|
It is now RFAL. |
|
Friendly ping. |
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis, @rafiss, and @yuzefovich)
pkg/sql/exec/hashjoiner.go, line 52 at r1 (raw file):
type hashJoinerSpec struct { // TODO(yuzefovich): update this when LEFT ANTI is supported. isLeftSemi bool
Would it make sense to just have this be a sqlbase.JoinType? I'm fine with this too
pkg/sql/exec/mergejoiner.go, line 310 at r1 (raw file):
right mergeJoinInput isLeftSemiAnti bool
Same here.
yuzefovich
force-pushed
the
fix-left-mj
branch
2 times, most recently
from
f530354
to
3f7b8eb
Compare
Previously, the merge joiner's output batch would always have the columns corresponding to both the left and the right sides (even with LEFT SEMI and LEFT ANTI join types although the right side output would not be used). This is incorrect, and now the merge joiner outputs batches with the correct number of columns. A similar issue was present with LEFT SEMI hash joiner and is now fixed. Release note: None
yuzefovich
force-pushed
the
fix-left-mj
branch
from
3f7b8eb
to
5a7955f
Compare
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/sql/exec/hashjoiner.go, line 52 at r1 (raw file):
Previously, asubiotto (Alfonso Subiotto Marqués) wrote…
Would it make sense to just have this be a
sqlbase.JoinType? I'm fine with this too
I didn't want to introduce the join type into the "base" joiner. I refactored the code, so I think your concern has been resolved.
pkg/sql/exec/mergejoiner.go, line 310 at r1 (raw file):
Previously, asubiotto (Alfonso Subiotto Marqués) wrote…
Same here.
Done.
39294: exec: fix output batches of LEFT SEMI for hash and merge joiners r=yuzefovich a=yuzefovich Previously, the merge joiner's output batch would always have the columns corresponding to both the left and the right sides (even with LEFT SEMI and LEFT ANTI join types although the right side output would not be used). This is incorrect, and now the merge joiner outputs batches with the correct number of columns. A similar issue was present with LEFT SEMI hash joiner and is now fixed. Release note: None Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Build succeeded |
Previously, the merge joiner's output batch would always have the
columns corresponding to both the left and the right sides (even
with LEFT SEMI and LEFT ANTI join types although the right side
output would not be used). This is incorrect, and now the merge
joiner outputs batches with the correct number of columns.
A similar issue was present with LEFT SEMI hash joiner and is now
fixed.
Release note: None