From e63860a1be6bc1f6e8cfb3fed65dc62a94b8213c Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Sun, 12 Apr 2020 17:26:02 +0530 Subject: [PATCH 1/5] Plan generation for explain analyze describe statements Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/send.go | 36 ++++++------------- go/vt/vtgate/planbuilder/builder.go | 4 ++- go/vt/vtgate/planbuilder/bypass.go | 1 + go/vt/vtgate/planbuilder/ddl.go | 1 + go/vt/vtgate/planbuilder/other_read.go | 25 +++++++++++++ .../planbuilder/testdata/bypass_cases.txt | 12 ++++--- .../vtgate/planbuilder/testdata/ddl_cases.txt | 15 +++++--- .../planbuilder/testdata/other_read_cases.txt | 0 8 files changed, 59 insertions(+), 35 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/other_read.go create mode 100644 go/vt/vtgate/planbuilder/testdata/other_read_cases.txt diff --git a/go/vt/vtgate/engine/send.go b/go/vt/vtgate/engine/send.go index 1f3db920045..9c5a0c8ef30 100644 --- a/go/vt/vtgate/engine/send.go +++ b/go/vt/vtgate/engine/send.go @@ -17,8 +17,6 @@ limitations under the License. package engine import ( - "encoding/json" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" "vitess.io/vitess/go/vt/proto/query" @@ -45,6 +43,9 @@ type Send struct { // IsDML specifies how to deal with autocommit behaviour IsDML bool + // SingleShardOnly specifies that the query must be send to only single shard + SingleShardOnly bool + noInputs } @@ -53,26 +54,6 @@ func (s *Send) NeedsTransaction() bool { return s.IsDML } -// MarshalJSON serializes the Send into a JSON representation. -// It's used for testing and diagnostics. -func (s *Send) MarshalJSON() ([]byte, error) { - marshalSend := struct { - Opcode string - Keyspace *vindexes.Keyspace - TargetDestination key.Destination - Query string - IsDML bool - }{ - Opcode: "Send", - Keyspace: s.Keyspace, - TargetDestination: s.TargetDestination, - IsDML: s.IsDML, - Query: s.Query, - } - - return json.Marshal(marshalSend) -} - // RouteType implements Primitive interface func (s *Send) RouteType() string { if s.IsDML { @@ -103,6 +84,10 @@ func (s *Send) Execute(vcursor VCursor, bindVars map[string]*query.BindVariable, return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "Keyspace does not have exactly one shard: %v", rss) } + if s.SingleShardOnly && len(rss) != 1 { + return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Unexpected error, DestinationKeyspaceID mapping to multiple shards: %s, got: %v", s.Query, s.TargetDestination) + } + queries := make([]*querypb.BoundQuery, len(rss)) for i := range rss { queries[i] = &querypb.BoundQuery{ @@ -137,9 +122,10 @@ func (s *Send) GetFields(vcursor VCursor, bindVars map[string]*query.BindVariabl func (s *Send) description() PrimitiveDescription { other := map[string]interface{}{ - "Query": s.Query, - "Table": s.GetTableName(), - "IsDML": s.IsDML, + "Query": s.Query, + "Table": s.GetTableName(), + "IsDML": s.IsDML, + "SingleShardOnly": s.SingleShardOnly, } return PrimitiveDescription{ OperatorType: "Send", diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 24623f8a6b2..98918361324 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -301,7 +301,9 @@ func BuildFromStmt(query string, stmt sqlparser.Statement, vschema ContextVSchem } case *sqlparser.Use: instruction, err = buildUsePlan(stmt, vschema) - case *sqlparser.Set, *sqlparser.Show, *sqlparser.DBDDL, *sqlparser.OtherRead, *sqlparser.OtherAdmin: + case *sqlparser.OtherRead: + instruction, err = buildOtherRead(query, vschema) + case *sqlparser.Set, *sqlparser.Show, *sqlparser.DBDDL, *sqlparser.OtherAdmin: return nil, ErrPlanNotSupported case *sqlparser.Begin, *sqlparser.Commit, *sqlparser.Rollback: // Empty by design. Not executed by a plan diff --git a/go/vt/vtgate/planbuilder/bypass.go b/go/vt/vtgate/planbuilder/bypass.go index b139135339f..9b7fbc0a039 100644 --- a/go/vt/vtgate/planbuilder/bypass.go +++ b/go/vt/vtgate/planbuilder/bypass.go @@ -41,5 +41,6 @@ func buildPlanForBypass(stmt sqlparser.Statement, vschema ContextVSchema) (engin TargetDestination: vschema.Destination(), Query: sqlparser.String(stmt), IsDML: sqlparser.IsDMLStatement(stmt), + SingleShardOnly: false, }, nil } diff --git a/go/vt/vtgate/planbuilder/ddl.go b/go/vt/vtgate/planbuilder/ddl.go index 33113c3e54f..b701efb25c4 100644 --- a/go/vt/vtgate/planbuilder/ddl.go +++ b/go/vt/vtgate/planbuilder/ddl.go @@ -23,6 +23,7 @@ func buildDDLPlan(sql string, in sqlparser.Statement, vschema ContextVSchema) (e TargetDestination: destination, Query: sql, //This is original sql query to be passed as the parser can provide partial ddl AST. IsDML: false, + SingleShardOnly: false, }, nil } diff --git a/go/vt/vtgate/planbuilder/other_read.go b/go/vt/vtgate/planbuilder/other_read.go new file mode 100644 index 00000000000..0c9781d7ffd --- /dev/null +++ b/go/vt/vtgate/planbuilder/other_read.go @@ -0,0 +1,25 @@ +package planbuilder + +import ( + "vitess.io/vitess/go/vt/key" + "vitess.io/vitess/go/vt/vtgate/engine" +) + +func buildOtherRead(sql string, vschema ContextVSchema) (engine.Primitive, error) { + destination, keyspace, _, err := vschema.TargetDestination("") + if err != nil { + return nil, err + } + + if destination == nil { + destination = key.DestinationAnyShard{} + } + + return &engine.Send{ + Keyspace: keyspace, + TargetDestination: destination, + Query: sql, //This is original sql query to be passed as the parser can provide partial ddl AST. + IsDML: false, + SingleShardOnly: true, + }, nil +} diff --git a/go/vt/vtgate/planbuilder/testdata/bypass_cases.txt b/go/vt/vtgate/planbuilder/testdata/bypass_cases.txt index c70832caa87..a3b282848fb 100644 --- a/go/vt/vtgate/planbuilder/testdata/bypass_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/bypass_cases.txt @@ -12,7 +12,8 @@ }, "TargetDestination": "Shard(-80)", "IsDML": false, - "Query": "select count(*), col from unsharded" + "Query": "select count(*), col from unsharded", + "SingleShardOnly": false } } @@ -30,7 +31,8 @@ }, "TargetDestination": "Shard(-80)", "IsDML": true, - "Query": "update user set val = 1 where id = 18446744073709551616 and id = 1" + "Query": "update user set val = 1 where id = 18446744073709551616 and id = 1", + "SingleShardOnly": false } } @@ -48,7 +50,8 @@ }, "TargetDestination": "Shard(-80)", "IsDML": true, - "Query": "delete from USER where ID = 42" + "Query": "delete from USER where ID = 42", + "SingleShardOnly": false } } @@ -66,6 +69,7 @@ }, "TargetDestination": "Shard(-80)", "IsDML": true, - "Query": "insert into USER(ID, NAME) values (42, 'ms X')" + "Query": "insert into USER(ID, NAME) values (42, 'ms X')", + "SingleShardOnly": false } } diff --git a/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt b/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt index b04353ef12c..228017b89e2 100644 --- a/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt @@ -12,7 +12,8 @@ }, "TargetDestination": "AllShards()", "IsDML": false, - "Query": "create table t1(id bigint, primary key(id))" + "Query": "create table t1(id bigint, primary key(id))", + "SingleShardOnly": false } } @@ -30,7 +31,8 @@ }, "TargetDestination": "AllShards()", "IsDML": false, - "Query": "create table user.t1(id bigint, primary key(id))" + "Query": "create table user.t1(id bigint, primary key(id))", + "SingleShardOnly": false } } @@ -48,7 +50,8 @@ }, "TargetDestination": "AllShards()", "IsDML": false, - "Query": "create table a(id int)" + "Query": "create table a(id int)", + "SingleShardOnly": false } } @@ -70,7 +73,8 @@ }, "TargetDestination": "AllShards()", "IsDML": false, - "Query": "alter table a ADD id int" + "Query": "alter table a ADD id int", + "SingleShardOnly": false } } @@ -88,7 +92,8 @@ }, "TargetDestination": "AllShards()", "IsDML": false, - "Query": "alter table user.b ADD id int" + "Query": "alter table user.b ADD id int", + "SingleShardOnly": false } } diff --git a/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt new file mode 100644 index 00000000000..e69de29bb2d From f06c589a8ac2887f1c01791f43ab344fad89fa6b Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Sun, 12 Apr 2020 17:26:49 +0530 Subject: [PATCH 2/5] plan test for explain analyze describe statements Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/plan_test.go | 21 +++++- .../planbuilder/testdata/other_read_cases.txt | 75 +++++++++++++++++++ .../testdata/unsupported_cases.txt | 12 ++- 3 files changed, 103 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 27c97b23d25..6b6d5a38878 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -211,6 +211,22 @@ func TestDDLPlanningFromFile(t *testing.T) { testFile(t, "ddl_cases.txt", testOutputTempDir, vschema) } +func TestOtherReadPlanningFromFile(t *testing.T) { + // We are testing this separately so we can set a default keyspace + testOutputTempDir, err := ioutil.TempDir("", "plan_test") + require.NoError(t, err) + vschema := &vschemaWrapper{ + v: loadSchema(t, "schema_test.json"), + keyspace: &vindexes.Keyspace{ + Name: "main", + Sharded: false, + }, + tabletType: topodatapb.TabletType_MASTER, + } + + testFile(t, "other_read_cases.txt", testOutputTempDir, vschema) +} + func loadSchema(t *testing.T, filename string) *vindexes.VSchema { formal, err := vindexes.LoadFormal(locateFile(filename)) if err != nil { @@ -238,7 +254,10 @@ type vschemaWrapper struct { } func (vw *vschemaWrapper) TargetDestination(qualifier string) (key.Destination, *vindexes.Keyspace, topodatapb.TabletType, error) { - keyspaceName := vw.keyspace.Name + var keyspaceName string + if vw.keyspace != nil { + keyspaceName = vw.keyspace.Name + } if vw.dest == nil && qualifier != "" { keyspaceName = qualifier } diff --git a/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt index e69de29bb2d..db35529eca9 100644 --- a/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt @@ -0,0 +1,75 @@ +# Explain statement +"explain select * from user" +{ + "QueryType": "OTHER", + "Original": "explain select * from user", + "Instructions": { + "OperatorType": "Send", + "Variant": "", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "AnyShard()", + "IsDML": false, + "Query": "explain select * from user", + "SingleShardOnly": true + } +} + +# Analyze statement +"analyze table t1" +{ + "QueryType": "OTHER", + "Original": "analyze table t1", + "Instructions": { + "OperatorType": "Send", + "Variant": "", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "AnyShard()", + "IsDML": false, + "Query": "analyze table t1", + "SingleShardOnly": true + } +} + +# Describe statement +"describe t1" +{ + "QueryType": "OTHER", + "Original": "describe t1", + "Instructions": { + "OperatorType": "Send", + "Variant": "", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "AnyShard()", + "IsDML": false, + "Query": "describe t1", + "SingleShardOnly": true + } +} + +# Desc statement +"desc t1" +{ + "QueryType": "OTHER", + "Original": "desc t1", + "Instructions": { + "OperatorType": "Send", + "Variant": "", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "AnyShard()", + "IsDML": false, + "Query": "desc t1", + "SingleShardOnly": true + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt index 662604a4569..14f55b70290 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt @@ -10,10 +10,6 @@ "show create database" "plan building not supported" -# DBA statements -"explain select * from user" -"plan building not supported" - # union operations in subqueries (FROM) "select * from (select * from user union all select * from user_extra) as t" "unsupported: UNION cannot be executed as a single route" @@ -407,3 +403,11 @@ # delete with multi-table targets "delete music,user from music inner join user where music.id = user.id" "unsupported: multi-shard or vindex write statement" + +# Repair statement +"repair table t1,t2 quick" +"plan building not supported" + +# Optimize statement +"optimize table t1" +"plan building not supported" From 006e218db6bb657b469249f36136af14e31f65f4 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Sun, 12 Apr 2020 19:20:56 +0530 Subject: [PATCH 3/5] add other read test in plan_execute Signed-off-by: Harshit Gangal --- go/vt/vtgate/plan_executor_test.go | 77 ++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/go/vt/vtgate/plan_executor_test.go b/go/vt/vtgate/plan_executor_test.go index 5f60da8d5c1..4fb4c2a1979 100644 --- a/go/vt/vtgate/plan_executor_test.go +++ b/go/vt/vtgate/plan_executor_test.go @@ -1045,8 +1045,7 @@ func TestPlanExecutorComment(t *testing.T) { } } -func TestPlanExecutorOther(t *testing.T) { - t.Skip("not support yet") +func TestPlanExecutorOtherRead(t *testing.T) { executor, sbc1, sbc2, sbclookup := createExecutorEnvUsing(planAllTheThings) type cnts struct { @@ -1089,10 +1088,82 @@ func TestPlanExecutorOther(t *testing.T) { } stmts := []string{ - "show tables", "analyze table t1", "describe t1", "explain t1", + } + + for _, stmt := range stmts { + for _, tc := range tcs { + sbc1.ExecCount.Set(0) + sbc2.ExecCount.Set(0) + sbclookup.ExecCount.Set(0) + + _, err := executor.Execute(context.Background(), "TestExecute", NewSafeSession(&vtgatepb.Session{TargetString: tc.targetStr}), stmt, nil) + if tc.hasNoKeyspaceErr { + assert.Error(t, err, errNoKeyspace) + } else if tc.hasDestinationShardErr { + assert.Errorf(t, err, "Destination can only be a single shard for statement: %s, got: DestinationExactKeyRange(-)", stmt) + } else { + assert.NoError(t, err) + } + + diff := cmp.Diff(tc.wantCnts, cnts{ + Sbc1Cnt: sbc1.ExecCount.Get(), + Sbc2Cnt: sbc2.ExecCount.Get(), + SbcLookupCnt: sbclookup.ExecCount.Get(), + }) + if diff != "" { + t.Errorf("stmt: %s\ntc: %+v\n-want,+got:\n%s", stmt, tc, diff) + } + } + } +} + +func TestPlanExecutorOtherAdmin(t *testing.T) { + t.Skip("not support yet") + executor, sbc1, sbc2, sbclookup := createExecutorEnvUsing(planAllTheThings) + + type cnts struct { + Sbc1Cnt int64 + Sbc2Cnt int64 + SbcLookupCnt int64 + } + + tcs := []struct { + targetStr string + + hasNoKeyspaceErr bool + hasDestinationShardErr bool + wantCnts cnts + }{ + { + targetStr: "", + hasNoKeyspaceErr: true, + }, + { + targetStr: "TestExecutor[-]", + hasDestinationShardErr: true, + }, + { + targetStr: KsTestUnsharded, + wantCnts: cnts{ + Sbc1Cnt: 0, + Sbc2Cnt: 0, + SbcLookupCnt: 1, + }, + }, + { + targetStr: "TestExecutor", + wantCnts: cnts{ + Sbc1Cnt: 1, + Sbc2Cnt: 0, + SbcLookupCnt: 0, + }, + }, + } + + stmts := []string{ "repair table t1", "optimize table t1", } From 2a71ca31d0ad579db20f4c8aa12b613a404b9967 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Sun, 12 Apr 2020 19:57:16 +0530 Subject: [PATCH 4/5] Plan generation for optimize repair statement Signed-off-by: Harshit Gangal --- go/vt/vtgate/plan_executor_test.go | 1 - go/vt/vtgate/planbuilder/builder.go | 6 +-- go/vt/vtgate/planbuilder/other_read.go | 2 +- go/vt/vtgate/planbuilder/plan_test.go | 3 +- .../testdata/other_admin_cases.txt | 37 +++++++++++++++++++ .../planbuilder/testdata/other_read_cases.txt | 1 + .../testdata/unsupported_cases.txt | 8 ---- 7 files changed, 44 insertions(+), 14 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/testdata/other_admin_cases.txt diff --git a/go/vt/vtgate/plan_executor_test.go b/go/vt/vtgate/plan_executor_test.go index 4fb4c2a1979..91e80937d4b 100644 --- a/go/vt/vtgate/plan_executor_test.go +++ b/go/vt/vtgate/plan_executor_test.go @@ -1121,7 +1121,6 @@ func TestPlanExecutorOtherRead(t *testing.T) { } func TestPlanExecutorOtherAdmin(t *testing.T) { - t.Skip("not support yet") executor, sbc1, sbc2, sbclookup := createExecutorEnvUsing(planAllTheThings) type cnts struct { diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 98918361324..ac6a3a79dc6 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -301,9 +301,9 @@ func BuildFromStmt(query string, stmt sqlparser.Statement, vschema ContextVSchem } case *sqlparser.Use: instruction, err = buildUsePlan(stmt, vschema) - case *sqlparser.OtherRead: - instruction, err = buildOtherRead(query, vschema) - case *sqlparser.Set, *sqlparser.Show, *sqlparser.DBDDL, *sqlparser.OtherAdmin: + case *sqlparser.OtherRead, *sqlparser.OtherAdmin: + instruction, err = buildOtherReadAndAdmin(query, vschema) + case *sqlparser.Set, *sqlparser.Show, *sqlparser.DBDDL: return nil, ErrPlanNotSupported case *sqlparser.Begin, *sqlparser.Commit, *sqlparser.Rollback: // Empty by design. Not executed by a plan diff --git a/go/vt/vtgate/planbuilder/other_read.go b/go/vt/vtgate/planbuilder/other_read.go index 0c9781d7ffd..535c498551b 100644 --- a/go/vt/vtgate/planbuilder/other_read.go +++ b/go/vt/vtgate/planbuilder/other_read.go @@ -5,7 +5,7 @@ import ( "vitess.io/vitess/go/vt/vtgate/engine" ) -func buildOtherRead(sql string, vschema ContextVSchema) (engine.Primitive, error) { +func buildOtherReadAndAdmin(sql string, vschema ContextVSchema) (engine.Primitive, error) { destination, keyspace, _, err := vschema.TargetDestination("") if err != nil { return nil, err diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 6b6d5a38878..ae445a8ed3d 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -211,7 +211,7 @@ func TestDDLPlanningFromFile(t *testing.T) { testFile(t, "ddl_cases.txt", testOutputTempDir, vschema) } -func TestOtherReadPlanningFromFile(t *testing.T) { +func TestOtherPlanningFromFile(t *testing.T) { // We are testing this separately so we can set a default keyspace testOutputTempDir, err := ioutil.TempDir("", "plan_test") require.NoError(t, err) @@ -225,6 +225,7 @@ func TestOtherReadPlanningFromFile(t *testing.T) { } testFile(t, "other_read_cases.txt", testOutputTempDir, vschema) + testFile(t, "other_admin_cases.txt", testOutputTempDir, vschema) } func loadSchema(t *testing.T, filename string) *vindexes.VSchema { diff --git a/go/vt/vtgate/planbuilder/testdata/other_admin_cases.txt b/go/vt/vtgate/planbuilder/testdata/other_admin_cases.txt new file mode 100644 index 00000000000..79298c22f4d --- /dev/null +++ b/go/vt/vtgate/planbuilder/testdata/other_admin_cases.txt @@ -0,0 +1,37 @@ +# Repair statement +"repair table t1,t2 quick" +{ + "QueryType": "OTHER", + "Original": "repair table t1,t2 quick", + "Instructions": { + "OperatorType": "Send", + "Variant": "", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "AnyShard()", + "IsDML": false, + "Query": "repair table t1,t2 quick", + "SingleShardOnly": true + } +} + +# Optimize statement +"optimize table t1" +{ + "QueryType": "OTHER", + "Original": "optimize table t1", + "Instructions": { + "OperatorType": "Send", + "Variant": "", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "AnyShard()", + "IsDML": false, + "Query": "optimize table t1", + "SingleShardOnly": true + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt index db35529eca9..d92f991f644 100644 --- a/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt @@ -73,3 +73,4 @@ "SingleShardOnly": true } } + diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt index 14f55b70290..f61672a9c14 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt @@ -403,11 +403,3 @@ # delete with multi-table targets "delete music,user from music inner join user where music.id = user.id" "unsupported: multi-shard or vindex write statement" - -# Repair statement -"repair table t1,t2 quick" -"plan building not supported" - -# Optimize statement -"optimize table t1" -"plan building not supported" From e83186acc07cccd64a00a1d908a735b25c681755 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 13 Apr 2020 22:04:19 +0530 Subject: [PATCH 5/5] addressed review comments Signed-off-by: Harshit Gangal --- go/vt/vtgate/plan_executor_test.go | 9 +++------ go/vt/vtgate/planbuilder/other_read.go | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/plan_executor_test.go b/go/vt/vtgate/plan_executor_test.go index 91e80937d4b..1cb2d64e91f 100644 --- a/go/vt/vtgate/plan_executor_test.go +++ b/go/vt/vtgate/plan_executor_test.go @@ -1101,21 +1101,18 @@ func TestPlanExecutorOtherRead(t *testing.T) { _, err := executor.Execute(context.Background(), "TestExecute", NewSafeSession(&vtgatepb.Session{TargetString: tc.targetStr}), stmt, nil) if tc.hasNoKeyspaceErr { - assert.Error(t, err, errNoKeyspace) + assert.EqualError(t, err, "keyspace not specified") } else if tc.hasDestinationShardErr { assert.Errorf(t, err, "Destination can only be a single shard for statement: %s, got: DestinationExactKeyRange(-)", stmt) } else { assert.NoError(t, err) } - diff := cmp.Diff(tc.wantCnts, cnts{ + utils.MustMatch(t, tc.wantCnts, cnts{ Sbc1Cnt: sbc1.ExecCount.Get(), Sbc2Cnt: sbc2.ExecCount.Get(), SbcLookupCnt: sbclookup.ExecCount.Get(), - }) - if diff != "" { - t.Errorf("stmt: %s\ntc: %+v\n-want,+got:\n%s", stmt, tc, diff) - } + }, "count did not match") } } } diff --git a/go/vt/vtgate/planbuilder/other_read.go b/go/vt/vtgate/planbuilder/other_read.go index 535c498551b..ac9654a9498 100644 --- a/go/vt/vtgate/planbuilder/other_read.go +++ b/go/vt/vtgate/planbuilder/other_read.go @@ -1,3 +1,19 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package planbuilder import (