opt: fix bug with incorrect results produced by paired left lookup join#82054
opt: fix bug with incorrect results produced by paired left lookup join#82054craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
msirek
left a comment
There was a problem hiding this comment.
The column remapping looks complete and I could find no other variables based on scanPrivate, lookupJoin, or lookupConstraint that need an update (such as was done with pkCols).
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @sumeerbhola, and @yuzefovich)
yuzefovich
left a comment
There was a problem hiding this comment.
Thanks for the quick fix! LGTM too.
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/lookup_join line 790 at r1 (raw file):
chat_id INT NOT NULL, author_id INT NOT NULL, INDEX chat_id_idx (chat_id) -- without this index everything works fine
nit: not sure if we want to keep this comment, up to you.
Prior to this patch, it was possible for a paired join to produce incorrect results for a left lookup join. In particular, some output rows had non-NULL values for right-side columns when the right-side columns should have been NULL. This commit fixes the issue by updating the optimizer to ensure that only columns from the second join in the paired join (the index join) are projected, not columns from the first (the lookup join). Fixes cockroachdb#81968 Release note (bug fix): Fixed an issue where a left lookup join could have incorrect results. In particular, some output rows could have non-NULL values for right-side columns when the right-side columns should have been NULL. This issue only exists in 22.1.0 and prior development releases of 22.1.
rytaft
left a comment
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/sql/logictest/testdata/logic_test/lookup_join line 790 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: not sure if we want to keep this comment, up to you.
Done.
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 0cb0dca to blathers/backport-release-22.1-82054: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
|
Great find and fix! |
Prior to this patch, it was possible for a paired join to produce incorrect
results for a left lookup join. In particular, some output rows had
non-NULL values for right-side columns when the right-side columns should
have been NULL.
This commit fixes the issue by updating the optimizer to ensure that
only columns from the second join in the paired join (the index join)
are projected, not columns from the first (the lookup join).
Fixes #81968
Release note (bug fix): Fixed an issue where a left lookup join could
have incorrect results. In particular, some output rows could have non-NULL
values for right-side columns when the right-side columns should
have been NULL. This issue only exists in 22.1.0 and prior development
releases of 22.1.