Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opt: fix incorrect results from zigzag join with different index directions #97151

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

rytaft
Copy link
Collaborator

@rytaft rytaft commented Feb 14, 2023

Prior to this commit, the optimizer could plan a zigzag join between two indexes where the matching column(s) had ascending values in one index and descending values in the other. This could cause incorrect results, because it broke an assumption of the zigzagJoiner.

For example, consider this table and query:

CREATE TABLE t (
  c INT NOT NULL,
  l INT NOT NULL,
  r INT NOT NULL,
  INDEX (l ASC, c DESC),
  INDEX (r ASC, c ASC)
);
INSERT INTO t VALUES (1, 1, -1), (2, 1, -2);
SELECT * FROM t@{FORCE_ZIGZAG} WHERE l = 1 AND r = -1;

This query should produce 1 row, but prior to this commit it produced 0 rows. This was due to the fact that the direction of c was different between the two indexes.

This commit fixes the issue by preventing the optimizer from planning a zigzag join in such cases.

Fixes #97090

Release note (bug fix): Fixed a bug in the query engine that could cause incorrect results in some cases when a zigzag join was planned. The bug could occur when the two indexes used for the zigzag join had a suffix of matching columns but with different directions. For example, planning a zigzag join with INDEX(a ASC, b ASC) and INDEX(c ASC, b DESC) could cause incorrect results. This bug has existed since at least v19.1. It is now fixed, because the optimizer will no longer plan a zigzag join in such cases.

…ctions

Prior to this commit, the optimizer could plan a zigzag join between two
indexes where the matching column(s) had ascending values in one index and
descending values in the other. This could cause incorrect results, because
it broke an assumption of the zigzagJoiner.

For example, consider this table and query:

CREATE TABLE t (
  c INT NOT NULL,
  l INT NOT NULL,
  r INT NOT NULL,
  INDEX (l ASC, c DESC),
  INDEX (r ASC, c ASC)
);
INSERT INTO t VALUES (1, 1, -1), (2, 1, -2);
SELECT * FROM t@{FORCE_ZIGZAG} WHERE l = 1 AND r = -1;

This query should produce 1 row, but prior to this commit it produced 0
rows. This was due to the fact that the direction of c was different between
the two indexes.

This commit fixes the issue by preventing the optimizer from planning a
zigzag join in such cases.

Fixes cockroachdb#97090

Release note (bug fix): Fixed a bug in the query engine that could cause
incorrect results in some cases when a zigzag join was planned. The bug could
occur when the two indexes used for the zigzag join had a suffix of matching
columns but with different directions. For example, planning a zigzag join with
INDEX(a ASC, b ASC) and INDEX(c ASC, b DESC) could cause incorrect results.
This bug has existed since at least v19.1. It is now fixed, because the
optimizer will no longer plan a zigzag join in such cases.
@rytaft rytaft requested a review from a team as a code owner February 14, 2023 23:16
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix! :lgtm:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)

Copy link
Collaborator Author

@rytaft rytaft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR! Friendly ping, @mgartner.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)

Copy link
Collaborator Author

@rytaft rytaft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to go ahead and merge this since it's a low-risk change and I want to get the fix backported.

bors r+

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)

@craig
Copy link
Contributor

craig bot commented Feb 22, 2023

Build failed:

Copy link
Collaborator Author

@rytaft rytaft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated flake (I will open an issue shortly)

bors r+

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)

@yuzefovich
Copy link
Member

The flake is me from a recently merged #96695, I'll take a look, no need for an issue.

@rytaft
Copy link
Collaborator Author

rytaft commented Feb 22, 2023

Thanks!

@craig
Copy link
Contributor

craig bot commented Feb 22, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sql: zigzag join can produce incorrect results when the index direction differs on the equality columns
3 participants