Merge pull request #7445 from planetscale/desc-fix

Describe table to route based on table name and qualifier

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) {

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

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
@@ -86,6 +85,12 @@ type (
  SQLNode
  }
 
+ // Explain is an interface that represents the Explain statements
+ Explain interface {
+ Statement
+ iExplain()
+ }
+
  // AddConstraintDefinition represents a ADD CONSTRAINT alter option
  AddConstraintDefinition struct {
  ConstraintDefinition *ConstraintDefinition
@@ -505,32 +510,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 +535,32 @@ type (
 
  // UnlockTables represents the unlock statement
  UnlockTables struct{}
+
+ // ExplainType is an enum for ExplainStmt.Type
+ ExplainType int8
+
+ // ExplainStmt represents an Explain statement
+ ExplainStmt struct {
+ Type ExplainType
+ Statement Statement
+ }
+
+ // ExplainTab represents the Explain table
+ 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() {}
@@ -571,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() {}
@@ -592,6 +602,8 @@ func (*DropView) iStatement() {}
 func (*TruncateTable) iStatement() {}
 func (*RenameTable) iStatement() {}
 func (*CallProc) iStatement() {}
+func (*ExplainStmt) iStatement() {}
+func (*ExplainTab) iStatement() {}
 
 func (*CreateView) iDDLStatement() {}
 func (*AlterView) iDDLStatement() {}
@@ -622,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
@@ -2486,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
@@ -2498,6 +2513,14 @@ func (node *Explain) Format(buf *TrackedBuffer) {
  buf.astPrintf(node, "explain %s%v", format, node.Statement)
 }
 
+// Format formats the node.
+func (node *ExplainTab) 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)

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",
  }, {

go/vt/sqlparser/sql.y

@@ -2728,21 +2728,21 @@ wild_opt:
  }
 | sql_id
  {
- $$ = "" 
+ $$ = $1.val
  }
 | STRING
  {
- $$ = "" 
+ $$ = "'" + string($1) + "'"
  }
- 
+
 explain_statement:
  explain_synonyms table_name wild_opt
  {
- $$ = &OtherRead{}
+ $$ = &ExplainTab{Table: $2, Wild: $3}
  }
 | explain_synonyms explain_format_opt explainable_statement
  {
- $$ = &Explain{Type: $2, Statement: $3}
+ $$ = &ExplainStmt{Type: $2, Statement: $3}
  }
 
 other_statement:

go/vt/vtgate/executor_test.go

@@ -1037,27 +1037,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)
- }
  }
  }
 }

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: