bindinfo: report error for completely invalid hint of bind_sql (pingc…

…ap#16059)

bindinfo/bind_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/pingcap/parser"
 	"github.com/pingcap/parser/auth"
 	"github.com/pingcap/parser/model"
+	"github.com/pingcap/parser/terror"
 	"github.com/pingcap/tidb/bindinfo"
 	"github.com/pingcap/tidb/domain"
 	"github.com/pingcap/tidb/kv"
@@ -947,6 +948,17 @@ func (s *testSuite) TestHintsSetID(c *C) {
 	c.Assert(len(bindData.Bindings), Equals, 1)
 	bind = bindData.Bindings[0]
 	c.Assert(bind.ID, Equals, "use_index(@`sel_1` `test`.`t` `idx_a`)")
+
+	s.cleanBindingEnv(tk)
+	err := tk.ExecToErr("create global binding for select * from t using select /*+ non_exist_hint() */ * from t")
+	c.Assert(terror.ErrorEqual(err, parser.ErrWarnOptimizerHintParseError), IsTrue)
+	tk.MustExec("create global binding for select * from t where a > 10 using select * from t where a > 10")
+	bindData = bindHandle.GetBindRecord(hash, sql, "test")
+	c.Check(bindData, NotNil)
+	c.Check(bindData.OriginalSQL, Equals, "select * from t where a > ?")
+	c.Assert(len(bindData.Bindings), Equals, 1)
+	bind = bindData.Bindings[0]
+	c.Assert(bind.ID, Equals, "")
 }
 
 func (s *testSuite) TestCapturePlanBaselineIgnoreTiFlash(c *C) {

bindinfo/cache.go

@@ -117,14 +117,20 @@ func (br *BindRecord) prepareHints(sctx sessionctx.Context) error {
 				return err
 			}
 		}
-		hintsSet, err := hint.ParseHintsSet(p, bind.BindSQL, bind.Charset, bind.Collation, br.Db)
+		hintsSet, warns, err := hint.ParseHintsSet(p, bind.BindSQL, bind.Charset, bind.Collation, br.Db)
 		if err != nil {
 			return err
 		}
 		hintsStr, err := hintsSet.Restore()
 		if err != nil {
 			return err
 		}
+		// For `create global binding for select * from t using select * from t`, we allow it though hintsStr is empty.
+		// For `create global binding for select * from t using select /*+ non_exist_hint() */ * from t`,
+		// the hint is totally invaild, we escalate warning to error.
+		if hintsStr == "" && len(warns) > 0 {
+			return warns[0]
+		}
 		br.Bindings[i].Hint = hintsSet
 		br.Bindings[i].ID = hintsStr
 	}

util/hint/hint_processor.go

@@ -185,14 +185,17 @@ func BindHint(stmt ast.StmtNode, hintsSet *HintsSet) ast.StmtNode {
 }
 
 // ParseHintsSet parses a SQL string, then collects and normalizes the HintsSet.
-func ParseHintsSet(p *parser.Parser, sql, charset, collation, db string) (*HintsSet, error) {
-	stmtNode, err := p.ParseOneStmt(sql, charset, collation)
+func ParseHintsSet(p *parser.Parser, sql, charset, collation, db string) (*HintsSet, []error, error) {
+	stmtNodes, warns, err := p.Parse(sql, charset, collation)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	hs := CollectHint(stmtNode)
+	if len(stmtNodes) != 1 {
+		return nil, nil, errors.New(fmt.Sprintf("bind_sql must be a single statement: %s", sql))
+	}
+	hs := CollectHint(stmtNodes[0])
 	processor := &BlockHintProcessor{}
-	stmtNode.Accept(processor)
+	stmtNodes[0].Accept(processor)
 	for i, tblHints := range hs.tableHints {
 		newHints := make([]*ast.TableOptimizerHint, 0, len(tblHints))
 		for _, tblHint := range tblHints {
@@ -202,7 +205,7 @@ func ParseHintsSet(p *parser.Parser, sql, charset, collation, db string) (*Hints
 			offset := processor.GetHintOffset(tblHint.QBName, TypeSelect, i+1)
 			if offset < 0 || !processor.checkTableQBName(tblHint.Tables, TypeSelect) {
 				hintStr := RestoreTableOptimizerHint(tblHint)
-				return nil, errors.New(fmt.Sprintf("Unknown query block name in hint %s", hintStr))
+				return nil, nil, errors.New(fmt.Sprintf("Unknown query block name in hint %s", hintStr))
 			}
 			tblHint.QBName = GenerateQBName(TypeSelect, offset)
 			for i, tbl := range tblHint.Tables {
@@ -214,7 +217,22 @@ func ParseHintsSet(p *parser.Parser, sql, charset, collation, db string) (*Hints
 		}
 		hs.tableHints[i] = newHints
 	}
-	return hs, nil
+	return hs, extractHintWarns(warns), nil
+}
+
+func extractHintWarns(warns []error) []error {
+	for _, w := range warns {
+		if parser.ErrWarnOptimizerHintUnsupportedHint.Equal(w) ||
+			parser.ErrWarnOptimizerHintInvalidToken.Equal(w) ||
+			parser.ErrWarnMemoryQuotaOverflow.Equal(w) ||
+			parser.ErrWarnOptimizerHintParseError.Equal(w) ||
+			parser.ErrWarnOptimizerHintInvalidInteger.Equal(w) {
+			// Just one warning is enough, however we use a slice here to stop golint complaining
+			// "error should be the last type when returning multiple items" for `ParseHintsSet`.
+			return []error{w}
+		}
+	}
+	return nil
 }
 
 // BlockHintProcessor processes hints at different level of sql statement.