Skip to content

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

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.

Sign up for GitHub

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

Merged
systay merged 1 commit into from
Jan 8, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions go/test/endtoend/vtgate/lookup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,3 +513,15 @@ func TestSelectNullLookup(t *testing.T) {

exec(t, conn, "delete from t6")
}

func TestUnicodeLooseMD5CaseInsensitive(t *testing.T) {
ctx := context.Background()
conn, err := mysql.Connect(ctx, &vtParams)
require.NoError(t, err)
defer conn.Close()

exec(t, conn, "insert into t4(id1, id2) values(1, 'test')")
defer exec(t, conn, "delete from t4")

assertMatches(t, conn, "SELECT id1, id2 from t4 where id2 = 'Test'", `[[INT64(1) VARCHAR("test")]]`)
}
4 changes: 2 additions & 2 deletions go/test/endtoend/vtgate/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,14 @@ create table t4(
id1 bigint,
id2 varchar(10),
primary key(id1)
) Engine=InnoDB;
) ENGINE=InnoDB DEFAULT charset=utf8mb4 COLLATE=utf8mb4_general_ci;

create table t4_id2_idx(
id2 varchar(10),
id1 bigint,
keyspace_id varbinary(50),
primary key(id2, id1)
) Engine=InnoDB;
) Engine=InnoDB DEFAULT charset=utf8mb4 COLLATE=utf8mb4_general_ci;

create table t5_null_vindex(
id bigint not null,
Expand Down
6 changes: 3 additions & 3 deletions go/test/utils/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ import (
// In Test*() function:
//
// mustMatch(t, want, got, "something doesn't match")
func MustMatchFn(allowUnexportedTypes []interface{}, ignoredFields []string, extraOpts ...cmp.Option) func(t *testing.T, want, got interface{}, errMsg string) {
func MustMatchFn(allowUnexportedTypes []interface{}, ignoredFields []string, extraOpts ...cmp.Option) func(t *testing.T, want, got interface{}, errMsg ...string) {
diffOpts := append([]cmp.Option{
cmp.AllowUnexported(allowUnexportedTypes...),
cmpIgnoreFields(ignoredFields...),
}, extraOpts...)
// Diffs want/got and fails with errMsg on any failure.
return func(t *testing.T, want, got interface{}, errMsg string) {
return func(t *testing.T, want, got interface{}, errMsg ...string) {
t.Helper()
diff := cmp.Diff(want, got, diffOpts...)
if diff != "" {
t.Fatalf("%s: (-want +got)\n%v", errMsg, diff)
t.Fatalf("%v: (-want +got)\n%v", errMsg, diff)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtexplain/testdata/multi-output/selectsharded-output.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ select count(*) from user where id = 1 /* point aggregate */
select count(*) from user where name in ('alice','bob') /* scatter aggregate */

1 ks_sharded/40-80: select name, user_id from name_user_map where name in ('alice') limit 10001 /* scatter aggregate */
1 ks_sharded/c0-: select name, user_id from name_user_map where name in ('bob') limit 10001 /* scatter aggregate */
2 ks_sharded/-40: select count(*) from user where name in ('alice', 'bob') limit 10001 /* scatter aggregate */
2 ks_sharded/c0-: select name, user_id from name_user_map where name in ('bob') limit 10001 /* scatter aggregate */
3 ks_sharded/-40: select count(*) from user where name in ('alice', 'bob') limit 10001 /* scatter aggregate */

----------------------------------------------------------------------
select name, count(*) from user group by name /* scatter aggregate */
Expand Down
38 changes: 12 additions & 26 deletions go/vt/vtgate/executor_select_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,10 +551,8 @@ func TestSelectBindvars(t *testing.T) {
lookup.SetResults([]*sqltypes.Result{sqltypes.MakeTestResult(
sqltypes.MakeTestFields("b|a", "varbinary|varbinary"),
"foo1|1",
"foo2|1",
), sqltypes.MakeTestResult(
sqltypes.MakeTestFields("b|a", "varbinary|varbinary"),
"foo1|1",
"foo2|1",
)})

Expand All @@ -567,12 +565,8 @@ func TestSelectBindvars(t *testing.T) {
Sql: "select id from user where id = :id",
BindVariables: map[string]*querypb.BindVariable{"id": sqltypes.Int64BindVariable(1)},
}}
if !reflect.DeepEqual(sbc1.Queries, wantQueries) {
t.Errorf("sbc1.Queries: %+v, want %+v\n", sbc1.Queries, wantQueries)
}
if sbc2.Queries != nil {
t.Errorf("sbc2.Queries: %+v, want nil\n", sbc2.Queries)
}
utils.MustMatch(t, sbc1.Queries, wantQueries)
assert.Empty(t, sbc2.Queries)
sbc1.Queries = nil
testQueryLog(t, logChan, "TestExecute", "SELECT", sql, 1)

Expand All @@ -591,11 +585,10 @@ func TestSelectBindvars(t *testing.T) {
"__vals": sqltypes.TestBindVariable([]interface{}{"foo1", "foo2"}),
},
}}
if !reflect.DeepEqual(sbc1.Queries, wantQueries) {
t.Errorf("sbc1.Queries: %+v, want %+v\n", sbc1.Queries, wantQueries)
}
utils.MustMatch(t, wantQueries, sbc1.Queries)
sbc1.Queries = nil
testQueryLog(t, logChan, "VindexLookup", "SELECT", "select name, user_id from name_user_map where name in ::name", 1)
testQueryLog(t, logChan, "VindexLookup", "SELECT", "select name, user_id from name_user_map where name in ::name", 1)
testQueryLog(t, logChan, "TestExecute", "SELECT", sql, 1)

// Test with BytesBindVariable
Expand All @@ -606,17 +599,14 @@ func TestSelectBindvars(t *testing.T) {
})
require.NoError(t, err)
wantQueries = []*querypb.BoundQuery{{
Sql: "select id from user where name in ::__vals",
Sql: "select id from user where 1 != 1",
BindVariables: map[string]*querypb.BindVariable{
"name1": sqltypes.BytesBindVariable([]byte("foo1")),
"name2": sqltypes.BytesBindVariable([]byte("foo2")),
"__vals": sqltypes.TestBindVariable([]interface{}{[]byte("foo1"), []byte("foo2")}),
"name1": sqltypes.BytesBindVariable([]byte("foo1")),
"name2": sqltypes.BytesBindVariable([]byte("foo2")),
},
}}
if !reflect.DeepEqual(sbc1.Queries, wantQueries) {
t.Errorf("sbc1.Queries: %+v, want %+v\n", sbc1.Queries, wantQueries)
}

utils.MustMatch(t, wantQueries, sbc1.Queries)
testQueryLog(t, logChan, "VindexLookup", "SELECT", "select name, user_id from name_user_map where name in ::name", 1)
testQueryLog(t, logChan, "VindexLookup", "SELECT", "select name, user_id from name_user_map where name in ::name", 1)
testQueryLog(t, logChan, "TestExecute", "SELECT", sql, 1)

Expand Down Expand Up @@ -645,9 +635,7 @@ func TestSelectBindvars(t *testing.T) {
"name": sqltypes.StringBindVariable("nonexistent"),
},
}}
if !reflect.DeepEqual(sbc1.Queries, wantQueries) {
t.Errorf("sbc1.Queries: %+v, want %+v\n", sbc1.Queries, wantQueries)
}
utils.MustMatch(t, wantQueries, sbc1.Queries)

vars, err := sqltypes.BuildBindVariable([]interface{}{sqltypes.NewVarBinary("nonexistent")})
require.NoError(t, err)
Expand All @@ -657,13 +645,11 @@ func TestSelectBindvars(t *testing.T) {
"name": vars,
},
}}
if !reflect.DeepEqual(lookup.Queries, wantLookupQueries) {
t.Errorf("lookup.Queries: %+v, want %+v\n", lookup.Queries, wantLookupQueries)
}

utils.MustMatch(t, wantLookupQueries, lookup.Queries)

testQueryLog(t, logChan, "VindexLookup", "SELECT", "select name, user_id from name_user_map where name in ::name", 1)
testQueryLog(t, logChan, "TestExecute", "SELECT", sql, 1)

}

func TestSelectEqual(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/vindexes/lookup_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy Markdown

@bezmax bezmax Jan 8, 2021

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
Copy Markdown
Collaborator Author

@systay systay Jan 8, 2021

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

for _, id := range ids {
vars, err := sqltypes.BuildBindVariable([]interface{}{id})
Expand Down