Repository: vitessio/vitess
Pull request #6055 (2020-04-12 11:58:11)

Plan Generation for explain analyze describe optimize repair statements 


Patch:
diff --git a/go/vt/vtgate/engine/send.go b/go/vt/vtgate/engine/send.go
index 1f3db92004..9c5a0c8ef3 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 24623f8a6b..ac6a3a79dc 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 b139135339..9b7fbc0a03 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 33113c3e54..b701efb25c 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 0000000000..ac9654a949
--- /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
+}


Test patch:
diff --git a/go/vt/vtgate/plan_executor_test.go b/go/vt/vtgate/plan_executor_test.go
index 5f60da8d5c..1cb2d64e91 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/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go
index 27c97b23d2..ae445a8ed3 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 c70832caa8..a3b282848f 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 b04353ef12..228017b89e 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 0000000000..79298c22f4
--- /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 0000000000..d92f991f64
--- /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 662604a456..f61672a9c1 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"