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

[8.0] don't try to compare varchars in vtgate #7270

Merged
merged 1 commit into from
Jan 8, 2021

Conversation

systay
Copy link
Collaborator

@systay systay commented Jan 8, 2021

we can't group together multiple varchar values into a IN query,
because that forces use to compare varchar values on the vtgate
level which we can't do correctly yet

This is a backport of #7268 that solves #7254

we can't group together multiple varchar values into a IN query,
because that forces use to compare varchar values on the vtgate
level which we can't do correctly yet

Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay changed the title don't try to compare varchars in vtgate [8.0] don't try to compare varchars in vtgate Jan 8, 2021
@@ -80,7 +80,7 @@ func (lkp *lookupInternal) Lookup(vcursor VCursor, ids []sqltypes.Value, co vtga
if vcursor.InTransactionAndIsDML() {
sel = sel + " for update"
}
if !ids[0].IsIntegral() && !ids[0].IsBinary() {
if !ids[0].IsIntegral() {
// for non integral and binary type, fallback to send query per id
Copy link

Choose a reason for hiding this comment

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

Comment needs to be updated to avoid confusion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 I'll fix this in master

@systay systay merged commit a90d749 into vitessio:release-8.0 Jan 8, 2021
@systay systay deleted the fix-7254-in-8.0 branch January 8, 2021 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants