Merge pull request #7431 from planetscale/set-stmt-fix

Fix Set Statement in Tablet when executed with bindvars

go/test/endtoend/vtgate/reservedconn/udv_test.go

@@ -49,11 +49,9 @@ func TestSetUDV(t *testing.T) {
 		query:        "select @foo",
 		expectedRows: "[[NULL]]", rowsAffected: 1,
 	}, {
-		query:        "set @foo = 'abc', @bar = 42, @baz = 30.5, @tablet = concat('foo','bar')",
-		expectedRows: "", rowsAffected: 0,
+		query: "set @foo = 'abc', @bar = 42, @baz = 30.5, @tablet = concat('foo','bar')",
 	}, {
-		query:        "/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE */",
-		expectedRows: "", rowsAffected: 0,
+		query: "/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE */",
 	}, { // This is handled at vtgate.
 		query:        "select @foo, @bar, @baz, @tablet",
 		expectedRows: `[[VARBINARY("abc") INT64(42) FLOAT64(30.5) VARBINARY("foobar")]]`, rowsAffected: 1,
@@ -83,7 +81,7 @@ func TestSetUDV(t *testing.T) {
 		expectedRows: ``, rowsAffected: 1,
 	}, {
 		query:        "select id, val2 from test where val2=@bar",
-		expectedRows: ``, rowsAffected: 0,
+		expectedRows: ``,
 	}, {
 		query:        "update test set val2 = @bar where val1 = @foo",
 		expectedRows: ``, rowsAffected: 1,
@@ -98,7 +96,7 @@ func TestSetUDV(t *testing.T) {
 		expectedRows: `[[INT64(42) VARCHAR("foobar")]]`, rowsAffected: 1,
 	}, {
 		query:        "set @foo = now(), @bar = now(), @dd = date('2020-10-20'), @tt = time('10:15')",
-		expectedRows: `[]`, rowsAffected: 0,
+		expectedRows: `[]`,
 	}, {
 		query:        "select @foo = @bar, @dd, @tt",
 		expectedRows: `[[INT64(1) VARCHAR("2020-10-20") VARCHAR("10:15:00")]]`, rowsAffected: 1,
@@ -125,6 +123,47 @@ func TestSetUDV(t *testing.T) {
 	}
 }
 
+func TestMysqlDumpInitialLog(t *testing.T) {
+	defer cluster.PanicHandler(t)
+	ctx := context.Background()
+	vtParams := mysql.ConnParams{
+		Host: "localhost",
+		Port: clusterInstance.VtgateMySQLPort,
+	}
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	queries := []string{
+		"/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;",
+		"/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;",
+		"/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;",
+		"/*!50503 SET NAMES utf8mb4 */;",
+		"/*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */;",
+		"/*!40103 SET TIME_ZONE='+00:00' */;",
+		"/*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */;",
+		"/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;",
+		"/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;",
+		"/*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;",
+		"/*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;",
+		"/*!40101 SET SQL_MODE=@OLD_SQL_MODE */;",
+		"/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;",
+		"/*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */;",
+		"/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;",
+		"/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;",
+		"/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;",
+		"/*!40111 SET SQL_NOTES=@OLD_SQL_NOTES */;",
+	}
+
+	for _, query := range queries {
+		t.Run(query, func(t *testing.T) {
+			_, more, err := conn.ExecuteFetchMulti(query, 1000, true)
+			require.NoError(t, err)
+			require.False(t, more)
+		})
+	}
+}
+
 func TestUserDefinedVariableResolvedAtTablet(t *testing.T) {
 	ctx := context.Background()
 	vtParams := mysql.ConnParams{

go/vt/vttablet/tabletserver/planbuilder/plan.go

@@ -294,11 +294,7 @@ func checkForPoolingUnsafeConstructs(expr sqlparser.SQLNode) error {
 	return sqlparser.Walk(func(in sqlparser.SQLNode) (kontinue bool, err error) {
 		switch node := in.(type) {
 		case *sqlparser.Set:
-			for _, setExpr := range node.Exprs {
-				if setExpr.Name.AtCount() > 0 {
-					return false, genError(node)
-				}
-			}
+			return false, genError(node)
 		case *sqlparser.FuncExpr:
 			if sqlparser.IsLockingFunc(node) {
 				return false, genError(node)

go/vt/vttablet/tabletserver/planbuilder/testdata/exec_cases.txt

@@ -595,38 +595,6 @@ options:PassthroughDMLs
   "FullQuery": "delete from a limit 10"
 }
 
-# int
-"set  a=1"
-{
-  "PlanID": "Set",
-  "TableName": "",
-  "FullQuery": "set a = 1"
-}
-
-# float
-"set  a=1.2"
-{
-  "PlanID": "Set",
-  "TableName": "",
-  "FullQuery": "set a = 1.2"
-}
-
-# string
-"set a='b'"
-{
-  "PlanID": "Set",
-  "TableName": "",
-  "FullQuery": "set a = 'b'"
-}
-
-# multi
-"set a=1, b=2"
-{
-  "PlanID": "Set",
-  "TableName": "",
-  "FullQuery": "set a = 1, b = 2"
-}
-
 # create
 "create table a(a int, b varchar(8))"
 {

go/vt/vttablet/tabletserver/planbuilder/testdata/pool_unsafe_cases.txt

@@ -19,5 +19,13 @@
 "release_lock('foo') not allowed without a reserved connections"
 
 # setting system variables must happen inside reserved connections
-"set @sql_safe_updates = false"
-"set @sql_safe_updates = false not allowed without a reserved connections"
+"set sql_safe_updates = false"
+"set sql_safe_updates = false not allowed without a reserved connections"
+
+# setting system variables must happen inside reserved connections
+"set @@sql_safe_updates = false"
+"set @@sql_safe_updates = false not allowed without a reserved connections"
+
+# setting system variables must happen inside reserved connections
+"set @udv = false"
+"set @udv = false not allowed without a reserved connections"

go/vt/vttablet/tabletserver/query_executor.go

@@ -137,7 +137,7 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
 		return qr, nil
 	case p.PlanSelectLock:
 		return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "%s disallowed outside transaction", qre.plan.PlanID.String())
-	case p.PlanSet, p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
+	case p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
 		return qre.execOther()
 	case p.PlanSavepoint, p.PlanRelease, p.PlanSRollback:
 		return qre.execOther()
@@ -197,14 +197,14 @@ func (qre *QueryExecutor) execAsTransaction(f func(conn *StatefulConnection) (*s
 
 func (qre *QueryExecutor) txConnExec(conn *StatefulConnection) (*sqltypes.Result, error) {
 	switch qre.plan.PlanID {
-	case p.PlanInsert, p.PlanUpdate, p.PlanDelete:
+	case p.PlanInsert, p.PlanUpdate, p.PlanDelete, p.PlanSet:
 		return qre.txFetch(conn, true)
 	case p.PlanInsertMessage:
 		qre.bindVars["#time_now"] = sqltypes.Int64BindVariable(time.Now().UnixNano())
 		return qre.txFetch(conn, true)
 	case p.PlanUpdateLimit, p.PlanDeleteLimit:
 		return qre.execDMLLimit(conn)
-	case p.PlanSet, p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
+	case p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
 		return qre.execStatefulConn(conn, qre.query, true)
 	case p.PlanSavepoint, p.PlanRelease, p.PlanSRollback:
 		return qre.execStatefulConn(conn, qre.query, true)

go/vt/vttablet/tabletserver/query_executor_test.go

@@ -112,15 +112,6 @@ func TestQueryExecutorPlans(t *testing.T) {
 		// Because the fields would have been cached before, the field query will
 		// not get re-executed.
 		inTxWant: "select * from t limit 1",
-	}, {
-		input: "set a=1",
-		dbResponses: []dbResponse{{
-			query:  "set a=1",
-			result: dmlResult,
-		}},
-		resultWant: dmlResult,
-		planWant:   "Set",
-		logWant:    "set a=1",
 	}, {
 		input: "show engines",
 		dbResponses: []dbResponse{{
@@ -262,7 +253,7 @@ func TestQueryExecutorPlans(t *testing.T) {
 		inTxWant:   "RELEASE savepoint a",
 	}}
 	for _, tcase := range testcases {
-		func() {
+		t.Run(tcase.input, func(t *testing.T) {
 			db := setUpQueryExecutorTest(t)
 			defer db.Close()
 			for _, dbr := range tcase.dbResponses {
@@ -300,7 +291,7 @@ func TestQueryExecutorPlans(t *testing.T) {
 				want = tcase.inTxWant
 			}
 			assert.Equal(t, want, qre.logStats.RewrittenSQL(), "in tx: %v", tcase.input)
-		}()
+		})
 	}
 }