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/plan_executor_test.go b/go/vt/vtgate/plan_executor_test.go index 5f60da8d5c1..1cb2d64e91f 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,78 @@ 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.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) + } + + utils.MustMatch(t, tc.wantCnts, cnts{ + Sbc1Cnt: sbc1.ExecCount.Get(), + Sbc2Cnt: sbc2.ExecCount.Get(), + SbcLookupCnt: sbclookup.ExecCount.Get(), + }, "count did not match") + } + } +} + +func TestPlanExecutorOtherAdmin(t *testing.T) { + 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", } diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 24623f8a6b2..ac6a3a79dc6 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, *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/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..ac9654a9498 --- /dev/null +++ b/go/vt/vtgate/planbuilder/other_read.go @@ -0,0 +1,41 @@ +/* +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 ( + "vitess.io/vitess/go/vt/key" + "vitess.io/vitess/go/vt/vtgate/engine" +) + +func buildOtherReadAndAdmin(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/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 27c97b23d25..ae445a8ed3d 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -211,6 +211,23 @@ func TestDDLPlanningFromFile(t *testing.T) { testFile(t, "ddl_cases.txt", testOutputTempDir, vschema) } +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) + 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) + testFile(t, "other_admin_cases.txt", testOutputTempDir, vschema) +} + func loadSchema(t *testing.T, filename string) *vindexes.VSchema { formal, err := vindexes.LoadFormal(locateFile(filename)) if err != nil { @@ -238,7 +255,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/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_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 new file mode 100644 index 00000000000..d92f991f644 --- /dev/null +++ b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt @@ -0,0 +1,76 @@ +# 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..f61672a9c14 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"