From 3e97109b769028b580f12d910ec77cad85890c2e Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Sun, 9 May 2021 21:44:16 +0530 Subject: [PATCH] System schema query predicate fix Backport of #8087 This is a combination of 4 commits. * add e2e test for system schema * add unit test for system schema issue * redirect to default route if not able to resolve the keyspace in case of information_schema query * update test with new error message Signed-off-by: Harshit Gangal Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/system_schema_test.go | 2 + go/vt/vtgate/engine/route.go | 5 +++ go/vt/vtgate/executor_select_test.go | 29 +++++++++++- go/vt/vtgate/vindexes/vschema.go | 7 ++- go/vt/vtgate/vindexes/vschema_test.go | 45 +++++++------------ go/vt/vtgate/vtgate_test.go | 6 +-- 6 files changed, 58 insertions(+), 36 deletions(-) diff --git a/go/test/endtoend/vtgate/system_schema_test.go b/go/test/endtoend/vtgate/system_schema_test.go index 71bb809142f..8e5c29f3b5f 100644 --- a/go/test/endtoend/vtgate/system_schema_test.go +++ b/go/test/endtoend/vtgate/system_schema_test.go @@ -78,6 +78,8 @@ func TestInformationSchemaQuery(t *testing.T) { assertResultIsEmpty(t, conn, "table_schema = 'PERFORMANCE_SCHEMA'") assertSingleRowIsReturned(t, conn, "table_schema = 'performance_schema' and table_name = 'users'", "performance_schema") assertResultIsEmpty(t, conn, "table_schema = 'performance_schema' and table_name = 'foo'") + assertSingleRowIsReturned(t, conn, "table_schema = 'vt_ks' and table_name = 't1'", "vt_ks") + assertSingleRowIsReturned(t, conn, "table_schema = 'ks' and table_name = 't1'", "vt_ks") } func assertResultIsEmpty(t *testing.T, conn *mysql.Conn, pre string) { diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go index 362bbd15c25..598d3e3af47 100644 --- a/go/vt/vtgate/engine/route.go +++ b/go/vt/vtgate/engine/route.go @@ -462,6 +462,11 @@ func (route *Route) routeInfoSchemaQuery(vcursor VCursor, bindVars map[string]*q if tableName != "" { rss, err := route.paramsRoutedTable(vcursor, bindVars, specifiedKS, tableName) if err != nil { + // Only if keyspace is not found in vschema, we try with default keyspace. + // As the in the table_schema predicates for a keyspace 'ks' it can contain 'vt_ks'. + if vterrors.Code(err) == vtrpcpb.Code_NOT_FOUND { + return defaultRoute() + } return nil, err } if rss != nil { diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go index 5d7b2f8d340..25554126e87 100644 --- a/go/vt/vtgate/executor_select_test.go +++ b/go/vt/vtgate/executor_select_test.go @@ -67,8 +67,8 @@ func TestSelectDBA(t *testing.T) { require.NoError(t, err) wantQueries := []*querypb.BoundQuery{{Sql: query, BindVariables: map[string]*querypb.BindVariable{}}} utils.MustMatch(t, wantQueries, sbc1.Queries) - sbc1.Queries = nil + sbc1.Queries = nil query = "SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES WHERE table_schema = 'performance_schema' AND table_name = 'foo'" _, err = executor.Execute(context.Background(), "TestSelectDBA", NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"}), @@ -82,6 +82,33 @@ func TestSelectDBA(t *testing.T) { }}} utils.MustMatch(t, wantQueries, sbc1.Queries) + sbc1.Queries = nil + query = "select 1 from information_schema.table_constraints where constraint_schema = 'vt_ks' and table_name = 'user'" + _, err = executor.Execute(context.Background(), "TestSelectDBA", + NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"}), + query, map[string]*querypb.BindVariable{}, + ) + require.NoError(t, err) + wantQueries = []*querypb.BoundQuery{{Sql: "select 1 from information_schema.table_constraints where constraint_schema = :__vtschemaname and table_name = :__vttablename", + BindVariables: map[string]*querypb.BindVariable{ + "__vtschemaname": sqltypes.StringBindVariable("vt_ks"), + "__vttablename": sqltypes.StringBindVariable("user"), + }}} + utils.MustMatch(t, wantQueries, sbc1.Queries) + + sbc1.Queries = nil + query = "select 1 from information_schema.table_constraints where constraint_schema = 'vt_ks'" + _, err = executor.Execute(context.Background(), "TestSelectDBA", + NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"}), + query, map[string]*querypb.BindVariable{}, + ) + require.NoError(t, err) + wantQueries = []*querypb.BoundQuery{{Sql: "select 1 from information_schema.table_constraints where constraint_schema = :__vtschemaname", + BindVariables: map[string]*querypb.BindVariable{ + "__vtschemaname": sqltypes.StringBindVariable("vt_ks"), + }}} + utils.MustMatch(t, wantQueries, sbc1.Queries) + } func TestUnsharded(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/vschema.go b/go/vt/vtgate/vindexes/vschema.go index 3054b611dde..7e5c4d70623 100644 --- a/go/vt/vtgate/vindexes/vschema.go +++ b/go/vt/vtgate/vindexes/vschema.go @@ -23,6 +23,9 @@ import ( "io/ioutil" "sort" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/json2" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/sqlparser" @@ -481,7 +484,7 @@ func (vschema *VSchema) findTable(keyspace, tablename string) (*Table, error) { } ks, ok := vschema.Keyspaces[keyspace] if !ok { - return nil, fmt.Errorf("keyspace %s not found in vschema", keyspace) + return nil, vterrors.Errorf(vtrpcpb.Code_NOT_FOUND, "keyspace %s not found in vschema", keyspace) } table := ks.Tables[tablename] if table == nil { @@ -559,7 +562,7 @@ func (vschema *VSchema) FindVindex(keyspace, name string) (Vindex, error) { } ks, ok := vschema.Keyspaces[keyspace] if !ok { - return nil, fmt.Errorf("keyspace %s not found in vschema", keyspace) + return nil, vterrors.Errorf(vtrpcpb.Code_NOT_FOUND, "keyspace %s not found in vschema", keyspace) } return ks.Vindexes[name], nil } diff --git a/go/vt/vtgate/vindexes/vschema_test.go b/go/vt/vtgate/vindexes/vschema_test.go index 823b46f4e8c..0dc1782d47b 100644 --- a/go/vt/vtgate/vindexes/vschema_test.go +++ b/go/vt/vtgate/vindexes/vschema_test.go @@ -2156,33 +2156,24 @@ func TestFindTable(t *testing.T) { } vschema, _ := BuildVSchema(&input) _, err := vschema.FindTable("", "t1") - wantErr := "ambiguous table reference: t1" - if err == nil || err.Error() != wantErr { - t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr) - } + require.EqualError(t, err, "ambiguous table reference: t1") + _, err = vschema.FindTable("", "none") - wantErr = "table none not found" - if err == nil || err.Error() != wantErr { - t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr) - } - got, err := vschema.FindTable("", "ta") - if err != nil { - t.Error(err) - return - } + require.EqualError(t, err, "table none not found") + ta := &Table{ Name: sqlparser.NewTableIdent("ta"), Keyspace: &Keyspace{ Name: "ksa", }, } - if !reflect.DeepEqual(got, ta) { - t.Errorf("FindTable(\"t1a\"): %+v, want %+v", got, ta) - } + got, err := vschema.FindTable("", "ta") + require.NoError(t, err) + require.Equal(t, ta, got) + got, _ = vschema.FindTable("ksa", "ta") - if !reflect.DeepEqual(got, ta) { - t.Errorf("FindTable(\"t1a\"): %+v, want %+v", got, ta) - } + require.Equal(t, ta, got) + none := &Table{ Name: sqlparser.NewTableIdent("none"), Keyspace: &Keyspace{ @@ -2190,19 +2181,13 @@ func TestFindTable(t *testing.T) { }, } got, _ = vschema.FindTable("ksa", "none") - if !reflect.DeepEqual(got, none) { - t.Errorf("FindTable(\"t1a\"): %+v, want %+v", got, none) - } + require.Equal(t, none, got) + _, err = vschema.FindTable("ksb", "none") - wantErr = "table none not found" - if err == nil || err.Error() != wantErr { - t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr) - } + require.EqualError(t, err, "table none not found") + _, err = vschema.FindTable("none", "aa") - wantErr = "keyspace none not found in vschema" - if err == nil || err.Error() != wantErr { - t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr) - } + require.EqualError(t, err, "keyspace none not found in vschema") } func TestFindTableOrVindex(t *testing.T) { diff --git a/go/vt/vtgate/vtgate_test.go b/go/vt/vtgate/vtgate_test.go index ff465ff4931..01ddb499c0a 100644 --- a/go/vt/vtgate/vtgate_test.go +++ b/go/vt/vtgate/vtgate_test.go @@ -21,6 +21,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "context" "github.com/golang/protobuf/proto" @@ -140,9 +142,7 @@ func TestVTGateExecuteWithKeyspaceShard(t *testing.T) { nil, ) want := "vtgate: : keyspace invalid_keyspace not found in vschema" - if err == nil || err.Error() != want { - t.Errorf("Execute: %v, want %s", err, want) - } + assert.EqualError(t, err, want) // Valid keyspace/shard. _, qr, err = rpcVTGate.Execute(