-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
indexrec: improve recommendations for queries with inequalities #104208
indexrec: improve recommendations for queries with inequalities #104208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Just left some optional nits.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach and @qiyanghe1998)
-- commits
line 2 at r1:
nit: It's hard to make titles fit in ~72 characters — I don't always manage it — but you can try to make them less specific and put the details in the commit message body. For example, "indexrec: improve recommendations for queries with inequalities" is an option.
-- commits
line 7 at r1:
nit: "changed" makes it sound like you altered existing tests. maybe so I copied similar tests with "R": ...
…index recommendations Fixes cockroachdb#102206 As for the tests, since !=, IS DISTINT FROM and IS NOT are categorized as range index, so I copied similar tests with "R": R, EQ + R, J + R, EQ + J + R. Release note: None
6e7fdde
to
5f50b7a
Compare
Previously, mgartner (Marcus Gartner) wrote…
fixed |
Previously, mgartner (Marcus Gartner) wrote…
Thanks. Great advice, fixed. |
bors r+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @cucaroach)
Build failed (retrying...): |
Build failed (retrying...): |
Build succeeded: |
Fixes #102206
As for the tests, since !=, IS DISTINT FROM and IS NOT are categorized as range
index, so I copied similar tests with "R": R, EQ + R, J + R, EQ + J + R.
Release note: None
Epic: None