executor: use parser instead of preview and support added for special comments

go/vt/sqlparser/parse_test.go

@@ -1132,7 +1132,7 @@ var (
 		output: "alter table a",
 	}, {
 		input:  "analyze table a",
-		output: "alter table a",
+		output: "otherread",
 	}, {
 		input:  "flush tables",
 		output: "flush",

go/vt/sqlparser/sql.y

@@ -1509,7 +1509,7 @@ truncate_statement:
 analyze_statement:
   ANALYZE TABLE table_name
   {
-    $$ = &DDL{Action: AlterStr, Table: $3}
+    $$ = &OtherRead{}
   }
 
 show_statement:

go/vt/vtexplain/vtexplain_flaky_test.go

@@ -138,7 +138,7 @@ func TestErrors(t *testing.T) {
 	}{
 		{
 			SQL: "INVALID SQL",
-			Err: "vtexplain execute error in 'INVALID SQL': unrecognized statement: INVALID SQL",
+			Err: "vtexplain execute error in 'INVALID SQL': syntax error at position 8 near 'INVALID'",
 		},
 
 		{

go/vt/vtgate/executor.go

@@ -185,21 +185,31 @@ func (e *Executor) execute(ctx context.Context, safeSession *SafeSession, sql st
 	stmtType := sqlparser.Preview(sql)
 	logStats.StmtType = stmtType.String()
 
+	stmt, err := sqlparser.Parse(sql)
+	if err != nil {
+		// If the DDL statement failed to be properly parsed, fall through anyway
+		if stmtType == sqlparser.StmtDDL {
+			stmt = &sqlparser.DDL{}
+		} else {
+			return nil, err
+		}
+	}
+
 	// Mysql warnings are scoped to the current session, but are
 	// cleared when a "non-diagnostic statement" is executed:
 	// https://dev.mysql.com/doc/refman/8.0/en/show-warnings.html
 	//
 	// To emulate this behavior, clear warnings from the session
 	// for all statements _except_ SHOW, so that SHOW WARNINGS
 	// can actually return them.
-	if stmtType != sqlparser.StmtShow {
+	if _, isShow := stmt.(*sqlparser.Show); !isShow {
 		safeSession.ClearWarnings()
 	}
 
-	switch stmtType {
-	case sqlparser.StmtSelect:
+	switch specStmt := stmt.(type) {
+	case *sqlparser.Select, *sqlparser.Union:
 		return e.handleExec(ctx, safeSession, sql, bindVars, destKeyspace, destTabletType, dest, logStats, stmtType)
-	case sqlparser.StmtInsert, sqlparser.StmtReplace, sqlparser.StmtUpdate, sqlparser.StmtDelete:
+	case *sqlparser.Insert, *sqlparser.Update, *sqlparser.Delete:
 		safeSession := safeSession
 
 		mustCommit := false
@@ -236,24 +246,22 @@ func (e *Executor) execute(ctx context.Context, safeSession *SafeSession, sql st
 			logStats.CommitTime = time.Since(commitStart)
 		}
 		return qr, nil
-	case sqlparser.StmtDDL:
+	case *sqlparser.DDL:
 		return e.handleDDL(ctx, safeSession, sql, bindVars, dest, destKeyspace, destTabletType, logStats)
-	case sqlparser.StmtBegin:
+	case *sqlparser.Begin:
 		return e.handleBegin(ctx, safeSession, sql, bindVars, destTabletType, logStats)
-	case sqlparser.StmtCommit:
+	case *sqlparser.Commit:
 		return e.handleCommit(ctx, safeSession, sql, bindVars, logStats)
-	case sqlparser.StmtRollback:
+	case *sqlparser.Rollback:
 		return e.handleRollback(ctx, safeSession, sql, bindVars, logStats)
-	case sqlparser.StmtSet:
+	case *sqlparser.Set:
 		return e.handleSet(ctx, safeSession, sql, bindVars, logStats)
-	case sqlparser.StmtShow:
+	case *sqlparser.Show:
 		return e.handleShow(ctx, safeSession, sql, bindVars, dest, destKeyspace, destTabletType, logStats)
-	case sqlparser.StmtUse:
-		return e.handleUse(ctx, safeSession, sql, bindVars)
-	case sqlparser.StmtOther:
+	case *sqlparser.Use:
+		return e.handleUse(ctx, safeSession, sql, bindVars, specStmt)
+	case *sqlparser.OtherAdmin, *sqlparser.OtherRead:
 		return e.handleOther(ctx, safeSession, sql, bindVars, dest, destKeyspace, destTabletType, logStats)
-	case sqlparser.StmtComment:
-		return e.handleComment(sql)
 	}
 	return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unrecognized statement: %s", sql)
 }
@@ -1143,16 +1151,7 @@ func (e *Executor) handleShow(ctx context.Context, safeSession *SafeSession, sql
 	return e.handleOther(ctx, safeSession, sql, bindVars, dest, destKeyspace, destTabletType, logStats)
 }
 
-func (e *Executor) handleUse(ctx context.Context, safeSession *SafeSession, sql string, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) {
-	stmt, err := sqlparser.Parse(sql)
-	if err != nil {
-		return nil, err
-	}
-	use, ok := stmt.(*sqlparser.Use)
-	if !ok {
-		// This code is unreachable.
-		return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unrecognized USE statement: %v", sql)
-	}
+func (e *Executor) handleUse(ctx context.Context, safeSession *SafeSession, sql string, bindVars map[string]*querypb.BindVariable, use *sqlparser.Use) (*sqltypes.Result, error) {
 	destKeyspace, destTabletType, _, err := e.ParseDestinationTarget(use.DBName.String())
 	if err != nil {
 		return nil, err
@@ -1199,12 +1198,6 @@ func (e *Executor) handleOther(ctx context.Context, safeSession *SafeSession, sq
 	return result, err
 }
 
-func (e *Executor) handleComment(sql string) (*sqltypes.Result, error) {
-	_, _ = sqlparser.ExtractMysqlComment(sql)
-	// Not sure if this is a good idea.
-	return &sqltypes.Result{}, nil
-}
-
 // StreamExecute executes a streaming query.
 func (e *Executor) StreamExecute(ctx context.Context, method string, safeSession *SafeSession, sql string, bindVars map[string]*querypb.BindVariable, target querypb.Target, callback func(*sqltypes.Result) error) (err error) {
 	logStats := NewLogStats(ctx, method, sql, bindVars)

go/vt/vtgate/executor_dml_test.go

@@ -1601,7 +1601,7 @@ func TestKeyDestRangeQuery(t *testing.T) {
 	sbc2.Queries = nil
 
 	// it does not work for inserts
-	_, err = executorExec(executor, "INSERT INTO sharded_user_msgs(message) VALUE('test')", nil)
+	_, err = executorExec(executor, "INSERT INTO sharded_user_msgs(message) VALUES('test')", nil)
 
 	want := "range queries not supported for inserts: TestExecutor[-]"
 	if err == nil || err.Error() != want {
@@ -1686,7 +1686,7 @@ func TestKeyShardDestQuery(t *testing.T) {
 
 	// it works for inserts
 
-	sql = "INSERT INTO sharded_user_msgs(message) VALUE('test')"
+	sql = "INSERT INTO sharded_user_msgs(message) VALUES('test')"
 	_, err = executorExec(executor, sql, nil)
 
 	wantQueries = []*querypb.BoundQuery{{