From a2a33844a45e17ba2c1ecda288533be60373a1ba Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 3 Feb 2021 18:04:39 +0530 Subject: [PATCH 1/8] added desc parsing test Signed-off-by: Harshit Gangal --- go/vt/sqlparser/parse_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go index ea1ece1d5bf..51a7bd7ed10 100644 --- a/go/vt/sqlparser/parse_test.go +++ b/go/vt/sqlparser/parse_test.go @@ -1534,13 +1534,13 @@ var ( output: "explain select * from t", }, { input: "desc foobar", - output: "otherread", + output: "explain foobar", }, { - input: "explain t1", - output: "otherread", + input: "explain t1", }, { - input: "explain t1 col", - output: "otherread", + input: "explain t1 col", + }, { + input: "explain t1 '%col%'", }, { input: "explain select * from t", }, { From cefc1042a35a80b99956601871d06594c2d928b7 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 4 Feb 2021 00:18:24 +0530 Subject: [PATCH 2/8] add full parsing for describe Signed-off-by: Harshit Gangal --- go/vt/sqlparser/ast.go | 61 +++++++++++++++++++++++++------------ go/vt/sqlparser/rewriter.go | 7 +++++ go/vt/sqlparser/sql.go | 6 ++-- go/vt/sqlparser/sql.y | 8 ++--- 4 files changed, 55 insertions(+), 27 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 284f90578b2..7475dfac189 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -86,6 +86,12 @@ type ( SQLNode } + // Explain is an interface that represents the Explain statements + Explain interface { + iExplain() + Statement + } + // AddConstraintDefinition represents a ADD CONSTRAINT alter option AddConstraintDefinition struct { ConstraintDefinition *ConstraintDefinition @@ -505,32 +511,12 @@ type ( Name ColIdent } - // Explain represents an EXPLAIN statement - Explain struct { - Type ExplainType - Statement Statement - } - - // ExplainType is an enum for Explain.Type - ExplainType int8 - // CallProc represents a CALL statement CallProc struct { Name TableName Params Exprs } - // OtherRead represents a DESCRIBE, or EXPLAIN statement. - // It should be used only as an indicator. It does not contain - // the full AST for the statement. - OtherRead struct{} - - // OtherAdmin represents a misc statement that relies on ADMIN privileges, - // such as REPAIR, OPTIMIZE, or TRUNCATE statement. - // It should be used only as an indicator. It does not contain - // the full AST for the statement. - OtherAdmin struct{} - // LockType is an enum for Lock Types LockType int8 @@ -550,6 +536,32 @@ type ( // UnlockTables represents the unlock statement UnlockTables struct{} + + // ExplainType is an enum for Explain.Type + ExplainType int8 + + // Explain represents an EXPLAIN statement + ExplainStmt struct { + Type ExplainType + Statement Statement + } + + // ExplainTab represents the explain table statement + ExplainTab struct { + Table TableName + Wild string + } + + // OtherRead represents a DESCRIBE, or EXPLAIN statement. + // It should be used only as an indicator. It does not contain + // the full AST for the statement. + OtherRead struct{} + + // OtherAdmin represents a misc statement that relies on ADMIN privileges, + // such as REPAIR, OPTIMIZE, or TRUNCATE statement. + // It should be used only as an indicator. It does not contain + // the full AST for the statement. + OtherAdmin struct{} ) func (*Union) iStatement() {} @@ -592,6 +604,7 @@ func (*DropView) iStatement() {} func (*TruncateTable) iStatement() {} func (*RenameTable) iStatement() {} func (*CallProc) iStatement() {} +func (*Desc) iStatement() {} func (*CreateView) iDDLStatement() {} func (*AlterView) iDDLStatement() {} @@ -2498,6 +2511,14 @@ func (node *Explain) Format(buf *TrackedBuffer) { buf.astPrintf(node, "explain %s%v", format, node.Statement) } +// Format formats the node. +func (node *Desc) Format(buf *TrackedBuffer) { + buf.astPrintf(node, "explain %v", node.Table) + if node.Wild != "" { + buf.astPrintf(node, " %s", node.Wild) + } +} + // Format formats the node. func (node *CallProc) Format(buf *TrackedBuffer) { buf.astPrintf(node, "call %v(%v)", node.Name, node.Params) diff --git a/go/vt/sqlparser/rewriter.go b/go/vt/sqlparser/rewriter.go index 19f1050b06f..c592c630557 100644 --- a/go/vt/sqlparser/rewriter.go +++ b/go/vt/sqlparser/rewriter.go @@ -342,6 +342,10 @@ func replaceDerivedTableSelect(newNode, parent SQLNode) { parent.(*DerivedTable).Select = newNode.(SelectStatement) } +func replaceDescTable(newNode, parent SQLNode) { + parent.(*Desc).Table = newNode.(TableName) +} + func replaceDropColumnName(newNode, parent SQLNode) { parent.(*DropColumn).Name = newNode.(*ColName) } @@ -1263,6 +1267,9 @@ func (a *application) apply(parent, node SQLNode, replacer replacerFunc) { case *DerivedTable: a.apply(node, n.Select, replaceDerivedTableSelect) + case *Desc: + a.apply(node, n.Table, replaceDescTable) + case *DropColumn: a.apply(node, n.Name, replaceDropColumnName) diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index eea7d1bc634..e85eac8f6b1 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -8291,19 +8291,19 @@ yydefault: yyDollar = yyS[yypt-1 : yypt+1] //line sql.y:2730 { - yyVAL.str = "" + yyVAL.str = yyDollar[1].colIdent.val } case 513: yyDollar = yyS[yypt-1 : yypt+1] //line sql.y:2734 { - yyVAL.str = "" + yyVAL.str = "'" + string(yyDollar[1].bytes) + "'" } case 514: yyDollar = yyS[yypt-3 : yypt+1] //line sql.y:2740 { - yyVAL.statement = &OtherRead{} + yyVAL.statement = &Desc{Table: yyDollar[2].tableName, Wild: yyDollar[3].str} } case 515: yyDollar = yyS[yypt-3 : yypt+1] diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index 048a14c17de..0ae0e7072d1 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -2728,17 +2728,17 @@ wild_opt: } | sql_id { - $$ = "" + $$ = $1.val } | STRING { - $$ = "" + $$ = "'" + string($1) + "'" } - + explain_statement: explain_synonyms table_name wild_opt { - $$ = &OtherRead{} + $$ = &Desc{Table: $2, Wild: $3} } | explain_synonyms explain_format_opt explainable_statement { From d11253e0c675e01157c762f40700cb11d3676d89 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 4 Feb 2021 22:24:49 +0530 Subject: [PATCH 3/8] added explain interface and explain statement & explain table as type of it Signed-off-by: Harshit Gangal --- go/vt/sqlparser/analyzer.go | 2 +- go/vt/sqlparser/ast.go | 20 +++++++++++--------- go/vt/sqlparser/rewriter.go | 22 +++++++++++----------- go/vt/sqlparser/sql.go | 4 ++-- go/vt/sqlparser/sql.y | 4 ++-- 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 4d7c9424f6a..b44f5b36b1c 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -84,7 +84,7 @@ func ASTToStatementType(stmt Statement) StatementType { return StmtUse case *OtherRead, *OtherAdmin, *Load: return StmtOther - case *Explain: + case Explain: return StmtExplain case *Begin: return StmtBegin diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 7475dfac189..aec1b9c8507 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -45,14 +45,13 @@ type ( // SelectStatement any SELECT statement. SelectStatement interface { + Statement iSelectStatement() - iStatement() iInsertRows() AddOrder(*Order) SetLimit(*Limit) SetLock(lock Lock) MakeDistinct() - SQLNode } // DDLStatement represents any DDL Statement @@ -537,16 +536,16 @@ type ( // UnlockTables represents the unlock statement UnlockTables struct{} - // ExplainType is an enum for Explain.Type + // ExplainType is an enum for ExplainStmt.Type ExplainType int8 - // Explain represents an EXPLAIN statement + // ExplainStmt represents an Explain statement ExplainStmt struct { Type ExplainType Statement Statement } - // ExplainTab represents the explain table statement + // ExplainTab represents the Explain table ExplainTab struct { Table TableName Wild string @@ -583,7 +582,6 @@ func (*Rollback) iStatement() {} func (*SRollback) iStatement() {} func (*Savepoint) iStatement() {} func (*Release) iStatement() {} -func (*Explain) iStatement() {} func (*OtherRead) iStatement() {} func (*OtherAdmin) iStatement() {} func (*Select) iSelectStatement() {} @@ -604,7 +602,8 @@ func (*DropView) iStatement() {} func (*TruncateTable) iStatement() {} func (*RenameTable) iStatement() {} func (*CallProc) iStatement() {} -func (*Desc) iStatement() {} +func (*ExplainStmt) iStatement() {} +func (*ExplainTab) iStatement() {} func (*CreateView) iDDLStatement() {} func (*AlterView) iDDLStatement() {} @@ -635,6 +634,9 @@ func (*RenameIndex) iAlterOption() {} func (*Validation) iAlterOption() {} func (TableOptions) iAlterOption() {} +func (*ExplainStmt) iExplain() {} +func (*ExplainTab) iExplain() {} + // IsFullyParsed implements the DDLStatement interface func (*TruncateTable) IsFullyParsed() bool { return true @@ -2499,7 +2501,7 @@ func (node *Release) Format(buf *TrackedBuffer) { } // Format formats the node. -func (node *Explain) Format(buf *TrackedBuffer) { +func (node *ExplainStmt) Format(buf *TrackedBuffer) { format := "" switch node.Type { case EmptyType: // do nothing @@ -2512,7 +2514,7 @@ func (node *Explain) Format(buf *TrackedBuffer) { } // Format formats the node. -func (node *Desc) Format(buf *TrackedBuffer) { +func (node *ExplainTab) Format(buf *TrackedBuffer) { buf.astPrintf(node, "explain %v", node.Table) if node.Wild != "" { buf.astPrintf(node, " %s", node.Wild) diff --git a/go/vt/sqlparser/rewriter.go b/go/vt/sqlparser/rewriter.go index c592c630557..d4073481c10 100644 --- a/go/vt/sqlparser/rewriter.go +++ b/go/vt/sqlparser/rewriter.go @@ -342,10 +342,6 @@ func replaceDerivedTableSelect(newNode, parent SQLNode) { parent.(*DerivedTable).Select = newNode.(SelectStatement) } -func replaceDescTable(newNode, parent SQLNode) { - parent.(*Desc).Table = newNode.(TableName) -} - func replaceDropColumnName(newNode, parent SQLNode) { parent.(*DropColumn).Name = newNode.(*ColName) } @@ -362,8 +358,12 @@ func replaceExistsExprSubquery(newNode, parent SQLNode) { parent.(*ExistsExpr).Subquery = newNode.(*Subquery) } -func replaceExplainStatement(newNode, parent SQLNode) { - parent.(*Explain).Statement = newNode.(Statement) +func replaceExplainStmtStatement(newNode, parent SQLNode) { + parent.(*ExplainStmt).Statement = newNode.(Statement) +} + +func replaceExplainTabTable(newNode, parent SQLNode) { + parent.(*ExplainTab).Table = newNode.(TableName) } type replaceExprsItems int @@ -1267,9 +1267,6 @@ func (a *application) apply(parent, node SQLNode, replacer replacerFunc) { case *DerivedTable: a.apply(node, n.Select, replaceDerivedTableSelect) - case *Desc: - a.apply(node, n.Table, replaceDescTable) - case *DropColumn: a.apply(node, n.Name, replaceDropColumnName) @@ -1286,8 +1283,11 @@ func (a *application) apply(parent, node SQLNode, replacer replacerFunc) { case *ExistsExpr: a.apply(node, n.Subquery, replaceExistsExprSubquery) - case *Explain: - a.apply(node, n.Statement, replaceExplainStatement) + case *ExplainStmt: + a.apply(node, n.Statement, replaceExplainStmtStatement) + + case *ExplainTab: + a.apply(node, n.Table, replaceExplainTabTable) case Exprs: replacer := replaceExprsItems(0) diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index e85eac8f6b1..82322dd688f 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -8303,13 +8303,13 @@ yydefault: yyDollar = yyS[yypt-3 : yypt+1] //line sql.y:2740 { - yyVAL.statement = &Desc{Table: yyDollar[2].tableName, Wild: yyDollar[3].str} + yyVAL.statement = &ExplainTab{Table: yyDollar[2].tableName, Wild: yyDollar[3].str} } case 515: yyDollar = yyS[yypt-3 : yypt+1] //line sql.y:2744 { - yyVAL.statement = &Explain{Type: yyDollar[2].explainType, Statement: yyDollar[3].statement} + yyVAL.statement = &ExplainStmt{Type: yyDollar[2].explainType, Statement: yyDollar[3].statement} } case 516: yyDollar = yyS[yypt-2 : yypt+1] diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index 0ae0e7072d1..91945f86166 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -2738,11 +2738,11 @@ wild_opt: explain_statement: explain_synonyms table_name wild_opt { - $$ = &Desc{Table: $2, Wild: $3} + $$ = &ExplainTab{Table: $2, Wild: $3} } | explain_synonyms explain_format_opt explainable_statement { - $$ = &Explain{Type: $2, Statement: $3} + $$ = &ExplainStmt{Type: $2, Statement: $3} } other_statement: From 63329ceb81165f9e1ee24aa86c029bd664462128 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 4 Feb 2021 22:25:46 +0530 Subject: [PATCH 4/8] plan explain table correctly Signed-off-by: Harshit Gangal --- go/vt/vtgate/executor_test.go | 39 ++++++------ go/vt/vtgate/planbuilder/builder.go | 11 +--- go/vt/vtgate/planbuilder/explain.go | 62 +++++++++++++++---- go/vt/vtgate/planbuilder/other_read.go | 1 - .../planbuilder/testdata/other_read_cases.txt | 4 +- 5 files changed, 74 insertions(+), 43 deletions(-) diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go index a92b107b291..628e51618e8 100644 --- a/go/vt/vtgate/executor_test.go +++ b/go/vt/vtgate/executor_test.go @@ -1035,27 +1035,26 @@ func TestExecutorOther(t *testing.T) { for _, stmt := range stmts { for _, tc := range tcs { - sbc1.ExecCount.Set(0) - sbc2.ExecCount.Set(0) - sbclookup.ExecCount.Set(0) - - _, err := executor.Execute(ctx, "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(), + t.Run(fmt.Sprintf("%s-%s", stmt, tc.targetStr), func(t *testing.T) { + sbc1.ExecCount.Set(0) + sbc2.ExecCount.Set(0) + sbclookup.ExecCount.Set(0) + + _, err := executor.Execute(ctx, "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", stmt) + } else { + assert.NoError(t, err) + } + + 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) - } } } } diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 27122528233..2f1463af163 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -158,15 +158,8 @@ func createInstructionFor(query string, stmt sqlparser.Statement, vschema Contex return buildVSchemaDDLPlan(stmt, vschema) case *sqlparser.Use: return buildUsePlan(stmt, vschema) - case *sqlparser.Explain: - if stmt.Type == sqlparser.VitessType { - innerInstruction, err := createInstructionFor(query, stmt.Statement, vschema) - if err != nil { - return nil, err - } - return buildExplainPlan(innerInstruction) - } - return buildOtherReadAndAdmin(query, vschema) + case sqlparser.Explain: + return buildExplainPlan(stmt, vschema) case *sqlparser.OtherRead, *sqlparser.OtherAdmin: return buildOtherReadAndAdmin(query, vschema) case *sqlparser.Set: diff --git a/go/vt/vtgate/planbuilder/explain.go b/go/vt/vtgate/planbuilder/explain.go index 74d912dddbf..e13b423bf19 100644 --- a/go/vt/vtgate/planbuilder/explain.go +++ b/go/vt/vtgate/planbuilder/explain.go @@ -20,26 +20,53 @@ import ( "strings" "vitess.io/vitess/go/sqltypes" + "vitess.io/vitess/go/vt/key" querypb "vitess.io/vitess/go/vt/proto/query" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" ) -func extractQuery(m map[string]interface{}) string { - queryObj, ok := m["Query"] - if !ok { - return "" +// Builds an explain-plan for the given Primitive +func buildExplainPlan(stmt sqlparser.Explain, vschema ContextVSchema) (engine.Primitive, error) { + switch explain := stmt.(type) { + case *sqlparser.ExplainTab: + return explainTabPlan(explain, vschema) + case *sqlparser.ExplainStmt: + if explain.Type == sqlparser.VitessType { + return buildVitessTypePlan(explain, vschema) + } + return buildOtherReadAndAdmin(sqlparser.String(explain), vschema) } - query, ok := queryObj.(string) - if !ok { - return "" + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG unexpected explain type: %T", stmt) +} + +func explainTabPlan(explain *sqlparser.ExplainTab, vschema ContextVSchema) (engine.Primitive, error) { + table, _, _, _, destination, err := vschema.FindTableOrVindex(explain.Table) + if err != nil { + return nil, err } + explain.Table.Qualifier = sqlparser.NewTableIdent("") - return query + if destination == nil { + destination = key.DestinationAnyShard{} + } + + return &engine.Send{ + Keyspace: table.Keyspace, + TargetDestination: destination, + Query: sqlparser.String(explain), + SingleShardOnly: true, + }, nil } -// Builds an explain-plan for the given Primitive -func buildExplainPlan(input engine.Primitive) (engine.Primitive, error) { - descriptions := treeLines(engine.PrimitiveToPlanDescription(input)) +func buildVitessTypePlan(explain *sqlparser.ExplainStmt, vschema ContextVSchema) (engine.Primitive, error) { + innerInstruction, err := createInstructionFor(sqlparser.String(explain.Statement), explain.Statement, vschema) + if err != nil { + return nil, err + } + descriptions := treeLines(engine.PrimitiveToPlanDescription(innerInstruction)) var rows [][]sqltypes.Value for _, line := range descriptions { @@ -74,6 +101,19 @@ func buildExplainPlan(input engine.Primitive) (engine.Primitive, error) { return engine.NewRowsPrimitive(rows, fields), nil } +func extractQuery(m map[string]interface{}) string { + queryObj, ok := m["Query"] + if !ok { + return "" + } + query, ok := queryObj.(string) + if !ok { + return "" + } + + return query +} + type description struct { header string descr engine.PrimitiveDescription diff --git a/go/vt/vtgate/planbuilder/other_read.go b/go/vt/vtgate/planbuilder/other_read.go index ac9654a9498..aaedecc634f 100644 --- a/go/vt/vtgate/planbuilder/other_read.go +++ b/go/vt/vtgate/planbuilder/other_read.go @@ -35,7 +35,6 @@ func buildOtherReadAndAdmin(sql string, vschema ContextVSchema) (engine.Primitiv 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/other_read_cases.txt b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt index d183afb081c..0d52d47f4e8 100644 --- a/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/other_read_cases.txt @@ -57,7 +57,7 @@ }, "TargetDestination": "AnyShard()", "IsDML": false, - "Query": "describe select * from t", + "Query": "explain select * from t", "SingleShardOnly": true } } @@ -75,7 +75,7 @@ }, "TargetDestination": "AnyShard()", "IsDML": false, - "Query": "desc select * from t", + "Query": "explain select * from t", "SingleShardOnly": true } } From 34bff41590a429292f599dd55d6675e6a46df1cf Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 4 Feb 2021 23:35:43 +0530 Subject: [PATCH 5/8] fix explain plan in tabletserver Signed-off-by: Harshit Gangal --- go/vt/vttablet/tabletserver/planbuilder/permission.go | 2 +- go/vt/vttablet/tabletserver/planbuilder/plan.go | 4 ++-- .../tabletserver/planbuilder/testdata/stream_cases.txt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go/vt/vttablet/tabletserver/planbuilder/permission.go b/go/vt/vttablet/tabletserver/planbuilder/permission.go index 1bad105b4e7..80949b76c02 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/permission.go +++ b/go/vt/vttablet/tabletserver/planbuilder/permission.go @@ -57,7 +57,7 @@ func BuildPermissions(stmt sqlparser.Statement) []Permission { } case *sqlparser.OtherAdmin, *sqlparser.CallProc, *sqlparser.Begin, *sqlparser.Commit, *sqlparser.Rollback, *sqlparser.Load, *sqlparser.Savepoint, *sqlparser.Release, *sqlparser.SRollback, *sqlparser.Set, *sqlparser.Show, - *sqlparser.OtherRead, *sqlparser.Explain: + *sqlparser.OtherRead, sqlparser.Explain: // no op default: panic(fmt.Errorf("BUG: unexpected statement type: %T", node)) diff --git a/go/vt/vttablet/tabletserver/planbuilder/plan.go b/go/vt/vttablet/tabletserver/planbuilder/plan.go index 71d9758354b..340e7edb014 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/plan.go +++ b/go/vt/vttablet/tabletserver/planbuilder/plan.go @@ -202,7 +202,7 @@ func Build(statement sqlparser.Statement, tables map[string]*schema.Table, isRes plan = &Plan{PlanID: PlanDDL, FullQuery: fullQuery} case *sqlparser.Show: plan, err = analyzeShow(stmt, dbName) - case *sqlparser.OtherRead, *sqlparser.Explain: + case *sqlparser.OtherRead, sqlparser.Explain: plan, err = &Plan{PlanID: PlanOtherRead}, nil case *sqlparser.OtherAdmin: plan, err = &Plan{PlanID: PlanOtherAdmin}, nil @@ -254,7 +254,7 @@ func BuildStreaming(sql string, tables map[string]*schema.Table, isReservedConn return nil, vterrors.New(vtrpcpb.Code_FAILED_PRECONDITION, "select with lock not allowed for streaming") } plan.Table = lookupTable(stmt.From, tables) - case *sqlparser.OtherRead, *sqlparser.Show, *sqlparser.Union, *sqlparser.CallProc: + case *sqlparser.OtherRead, *sqlparser.Show, *sqlparser.Union, *sqlparser.CallProc, sqlparser.Explain: // pass default: return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "'%v' not allowed for streaming", sqlparser.String(stmt)) diff --git a/go/vt/vttablet/tabletserver/planbuilder/testdata/stream_cases.txt b/go/vt/vttablet/tabletserver/planbuilder/testdata/stream_cases.txt index 4724e019b94..16ca07e1907 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/testdata/stream_cases.txt +++ b/go/vt/vttablet/tabletserver/planbuilder/testdata/stream_cases.txt @@ -50,7 +50,7 @@ { "PlanID": "SelectStream", "TableName": "", - "FullQuery": "otherread" + "FullQuery": "explain foo" } # dml From 4672a0b4c0c0674e9e628e3f3cfa674b0bac8a0e Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 4 Feb 2021 23:58:03 +0530 Subject: [PATCH 6/8] added e2e test Signed-off-by: Harshit Gangal --- go/test/endtoend/vtgate/misc_test.go | 3 +++ go/vt/vttablet/sandboxconn/sandboxconn.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 253b0363490..133e8397d98 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -352,6 +352,9 @@ func TestExplainPassthrough(t *testing.T) { got := fmt.Sprintf("%v", result.Rows) require.Contains(t, got, "SIMPLE") // there is a lot more coming from mysql, // but we are trying to make the test less fragile + + result = exec(t, conn, "explain ks.t1") + require.EqualValues(t, 2, len(result.Rows)) } func TestXXHash(t *testing.T) { diff --git a/go/vt/vttablet/sandboxconn/sandboxconn.go b/go/vt/vttablet/sandboxconn/sandboxconn.go index 4589a749084..b2187b4742c 100644 --- a/go/vt/vttablet/sandboxconn/sandboxconn.go +++ b/go/vt/vttablet/sandboxconn/sandboxconn.go @@ -511,7 +511,7 @@ func (sbc *SandboxConn) getNextResult(stmt sqlparser.Statement) *sqltypes.Result case *sqlparser.Select, *sqlparser.Union, *sqlparser.Show, - *sqlparser.Explain, + sqlparser.Explain, *sqlparser.OtherRead: return SingleRowResult case *sqlparser.Set, From 15495afc3f0a4fc89af01267f29f477351e83b7d Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Fri, 5 Feb 2021 00:25:40 +0530 Subject: [PATCH 7/8] fix wrangler test Signed-off-by: Harshit Gangal --- go/vt/wrangler/vexec_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/wrangler/vexec_test.go b/go/vt/wrangler/vexec_test.go index 1ed3ccdc1d1..43dbde69db4 100644 --- a/go/vt/wrangler/vexec_test.go +++ b/go/vt/wrangler/vexec_test.go @@ -378,7 +378,7 @@ func TestVExecValidations(t *testing.T) { { name: "unsupported query", query: "describe _vt.vreplication", - errorString: "query not supported by vexec: otherread", + errorString: "query not supported by vexec: explain _vt.vreplication", }, } for _, bq := range badQueries { From f8dcedfeaa454c6f476e403d60ca6bd0370cf678 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Sun, 7 Feb 2021 23:00:47 +0530 Subject: [PATCH 8/8] addressed review comments Signed-off-by: Harshit Gangal --- go/vt/sqlparser/ast.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index aec1b9c8507..d2d7adfd951 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -87,8 +87,8 @@ type ( // Explain is an interface that represents the Explain statements Explain interface { - iExplain() Statement + iExplain() } // AddConstraintDefinition represents a ADD CONSTRAINT alter option