parser: improved handling of SET

sougou · bramp · commit 9d74ad24ce78 · 2018-06-05T07:41:09.000-07:00
This change improves the handling of constructs like
SET NAMES and SET CHARSET. It allows intermixing of
these with other assignments.

VTGate and mysqlproxy have been correspondingly updated.

Signed-off-by: Sugu Sougoumarane &lt;ssougou@gmail.com&gt;
diff --git a/analyzer.go b/analyzer.go
@@ -267,21 +267,18 @@ func StringIn(str string, values ...string) bool {
 // if the query is a SET statement. Values can be int64 or string.
 // Since set variable names are case insensitive, all keys are returned
 // as lower case.
-func ExtractSetValues(sql string) (keyValues map[string]interface{}, charset string, scope string, err error) {
+func ExtractSetValues(sql string) (keyValues map[string]interface{}, scope string, err error) {
 	stmt, err := Parse(sql)
 	if err != nil {
-		return nil, "", "", err
+		return nil, "", err
 	}
 	setStmt, ok := stmt.(*Set)
 	if !ok {
-		return nil, "", "", fmt.Errorf("ast did not yield *sqlparser.Set: %T", stmt)
+		return nil, "", fmt.Errorf("ast did not yield *sqlparser.Set: %T", stmt)
 	}
 	result := make(map[string]interface{})
 	for _, expr := range setStmt.Exprs {
-		if !expr.Name.Qualifier.IsEmpty() {
-			return nil, "", "", fmt.Errorf("invalid syntax: %v", String(expr.Name))
-		}
-		key := expr.Name.Name.Lowered()
+		key := expr.Name.Lowered()
 
 		switch expr := expr.Expr.(type) {
 		case *SQLVal:
@@ -291,19 +288,19 @@ func ExtractSetValues(sql string) (keyValues map[string]interface{}, charset str
 			case IntVal:
 				num, err := strconv.ParseInt(string(expr.Val), 0, 64)
 				if err != nil {
-					return nil, "", "", err
+					return nil, "", err
 				}
 				result[key] = num
 			default:
-				return nil, "", "", fmt.Errorf("invalid value type: %v", String(expr))
+				return nil, "", fmt.Errorf("invalid value type: %v", String(expr))
 			}
 		case *NullVal:
 			result[key] = nil
 		case *Default:
 			result[key] = "default"
 		default:
-			return nil, "", "", fmt.Errorf("invalid syntax: %s", String(expr))
+			return nil, "", fmt.Errorf("invalid syntax: %s", String(expr))
 		}
 	}
-	return result, setStmt.Charset.Lowered(), strings.ToLower(setStmt.Scope), nil
+	return result, strings.ToLower(setStmt.Scope), nil
 }
diff --git a/analyzer_test.go b/analyzer_test.go
@@ -360,20 +360,16 @@ func TestStringIn(t *testing.T) {
 
 func TestExtractSetValues(t *testing.T) {
 	testcases := []struct {
-		sql     string
-		out     map[string]interface{}
-		charset string
-		scope   string
-		err     string
+		sql   string
+		out   map[string]interface{}
+		scope string
+		err   string
 	}{{
 		sql: "invalid",
 		err: "syntax error at position 8 near 'invalid'",
 	}, {
 		sql: "select * from t",
 		err: "ast did not yield *sqlparser.Set: *sqlparser.Select",
-	}, {
-		sql: "set a.autocommit=1",
-		err: "invalid syntax: a.autocommit",
 	}, {
 		sql: "set autocommit=1+1",
 		err: "invalid syntax: 1 + 1",
@@ -393,21 +389,17 @@ func TestExtractSetValues(t *testing.T) {
 		sql: "SET foo = 0x1234",
 		err: "invalid value type: 0x1234",
 	}, {
-		sql:     "SET names utf8",
-		out:     map[string]interface{}{},
-		charset: "utf8",
+		sql: "SET names utf8",
+		out: map[string]interface{}{"names": "utf8"},
 	}, {
-		sql:     "SET names ascii collation ascii_bin",
-		out:     map[string]interface{}{},
-		charset: "ascii",
+		sql: "SET names ascii collate ascii_bin",
+		out: map[string]interface{}{"names": "ascii"},
 	}, {
-		sql:     "SET charset default",
-		out:     map[string]interface{}{},
-		charset: "default",
+		sql: "SET charset default",
+		out: map[string]interface{}{"charset": "default"},
 	}, {
-		sql:     "SET character set ascii",
-		out:     map[string]interface{}{},
-		charset: "ascii",
+		sql: "SET character set ascii",
+		out: map[string]interface{}{"charset": "ascii"},
 	}, {
 		sql:   "SET SESSION wait_timeout = 3600",
 		out:   map[string]interface{}{"wait_timeout": int64(3600)},
@@ -418,7 +410,7 @@ func TestExtractSetValues(t *testing.T) {
 		scope: "global",
 	}}
 	for _, tcase := range testcases {
-		out, charset, _, err := ExtractSetValues(tcase.sql)
+		out, _, err := ExtractSetValues(tcase.sql)
 		if tcase.err != "" {
 			if err == nil || err.Error() != tcase.err {
 				t.Errorf("ExtractSetValues(%s): %v, want '%s'", tcase.sql, err, tcase.err)
@@ -429,9 +421,6 @@ func TestExtractSetValues(t *testing.T) {
 		if !reflect.DeepEqual(out, tcase.out) {
 			t.Errorf("ExtractSetValues(%s): %v, want '%v'", tcase.sql, out, tcase.out)
 		}
-		if charset != tcase.charset {
-			t.Errorf("ExtractSetValues(%s): %v, want '%v'", tcase.sql, charset, tcase.charset)
-		}
 	}
 }
 
diff --git a/ast.go b/ast.go
@@ -562,8 +562,7 @@ func (node *Delete) WalkSubtree(visit Visit) error {
 // Set represents a SET statement.
 type Set struct {
 	Comments Comments
-	Exprs    UpdateExprs
-	Charset  ColIdent
+	Exprs    SetExprs
 	Scope    string
 }
 
@@ -2909,6 +2908,56 @@ func (node *UpdateExpr) WalkSubtree(visit Visit) error {
 	)
 }
 
+// SetExprs represents a list of set expressions.
+type SetExprs []*SetExpr
+
+// Format formats the node.
+func (node SetExprs) Format(buf *TrackedBuffer) {
+	var prefix string
+	for _, n := range node {
+		buf.Myprintf("%s%v", prefix, n)
+		prefix = ", "
+	}
+}
+
+// WalkSubtree walks the nodes of the subtree.
+func (node SetExprs) WalkSubtree(visit Visit) error {
+	for _, n := range node {
+		if err := Walk(visit, n); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// SetExpr represents a set expression.
+type SetExpr struct {
+	Name ColIdent
+	Expr Expr
+}
+
+// Format formats the node.
+func (node *SetExpr) Format(buf *TrackedBuffer) {
+	// We don't have to backtick set variable names.
+	if node.Name.EqualString("charset") || node.Name.EqualString("names") {
+		buf.Myprintf("%s %v", node.Name.String(), node.Expr)
+	} else {
+		buf.Myprintf("%s = %v", node.Name.String(), node.Expr)
+	}
+}
+
+// WalkSubtree walks the nodes of the subtree.
+func (node *SetExpr) WalkSubtree(visit Visit) error {
+	if node == nil {
+		return nil
+	}
+	return Walk(
+		visit,
+		node.Name,
+		node.Expr,
+	)
+}
+
 // OnDup represents an ON DUPLICATE KEY clause.
 type OnDup UpdateExprs
 
diff --git a/parse_next_test.go b/parse_next_test.go
@@ -120,11 +120,11 @@ func TestParseNextEdgeCases(t *testing.T) {
 	}, {
 		name:  "Handle ForceEOF statements",
 		input: "set character set utf8; select 1 from a",
-		want:  []string{"set ", "select 1 from a"},
+		want:  []string{"set charset 'utf8'", "select 1 from a"},
 	}, {
 		name:  "Semicolin inside a string",
 		input: "set character set ';'; select 1 from a",
-		want:  []string{"set ", "select 1 from a"},
+		want:  []string{"set charset ';'", "select 1 from a"},
 	}, {
 		name:  "Partial DDL",
 		input: "create table a; select 1 from a",
diff --git a/parse_test.go b/parse_test.go
@@ -657,24 +657,26 @@ var (
 		input: "set character_set_results = utf8",
 	}, {
 		input:  "set names utf8 collate foo",
-		output: "set ",
+		output: "set names 'utf8'",
 	}, {
 		input:  "set character set utf8",
-		output: "set ",
+		output: "set charset 'utf8'",
 	}, {
 		input:  "set character set 'utf8'",
-		output: "set ",
+		output: "set charset 'utf8'",
 	}, {
 		input:  "set character set \"utf8\"",
-		output: "set ",
+		output: "set charset 'utf8'",
 	}, {
 		input:  "set charset default",
-		output: "set ",
+		output: "set charset default",
 	}, {
 		input:  "set session wait_timeout = 3600",
 		output: "set session wait_timeout = 3600",
 	}, {
 		input: "set /* list */ a = 3, b = 4",
+	}, {
+		input: "set /* mixed list */ a = 3, names 'utf8', charset 'ascii', b = 4",
 	}, {
 		input:  "alter ignore table a add foo",
 		output: "alter table a",
diff --git a/sql.go b/sql.go
diff --git a/sql.y b/sql.y
