allow innodb_lock_wait_timeout as system variable (vitessio#16574)

Signed-off-by: Harshit Gangal <[email protected]>

go/test/endtoend/vtgate/reservedconn/sysvar_test.go

@@ -461,3 +461,35 @@ func TestSysVarTxIsolation(t *testing.T) {
 	// second run, to ensuring the setting is applied on the session and not just on next query after settings.
 	utils.AssertContains(t, conn, "select @@transaction_isolation, connection_id()", `SERIALIZABLE`)
 }
+
+// TestSysVarInnodbWaitTimeout tests the innodb_lock_wait_timeout system variable
+func TestSysVarInnodbWaitTimeout(t *testing.T) {
+	conn, err := mysql.Connect(context.Background(), &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	// default from mysql
+	utils.AssertMatches(t, conn, "select @@innodb_lock_wait_timeout", `[[UINT64(20)]]`)
+	utils.AssertMatches(t, conn, "select @@global.innodb_lock_wait_timeout", `[[UINT64(20)]]`)
+	// ensuring it goes to mysql
+	utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout", `UINT64(20)`)
+	utils.AssertContains(t, conn, "select @@global.innodb_lock_wait_timeout", `UINT64(20)`)
+
+	// setting to different value.
+	utils.Exec(t, conn, "set @@innodb_lock_wait_timeout = 120")
+	utils.AssertMatches(t, conn, "select @@innodb_lock_wait_timeout", `[[INT64(120)]]`)
+	// ensuring it goes to mysql
+	utils.AssertContains(t, conn, "select @@global.innodb_lock_wait_timeout, connection_id()", `UINT64(20)`)
+	utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(120)`)
+	// second run, to ensuring the setting is applied on the session and not just on next query after settings.
+	utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(120)`)
+
+	// changing setting to different value.
+	utils.Exec(t, conn, "set @@innodb_lock_wait_timeout = 240")
+	utils.AssertMatches(t, conn, "select @@innodb_lock_wait_timeout", `[[INT64(240)]]`)
+	// ensuring it goes to mysql
+	utils.AssertContains(t, conn, "select @@global.innodb_lock_wait_timeout, connection_id()", `UINT64(20)`)
+	utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(240)`)
+	// second run, to ensuring the setting is applied on the session and not just on next query after settings.
+	utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(240)`)
+}

go/vt/sqlparser/ast_rewriting.go

@@ -301,10 +301,12 @@ func (er *astRewriter) rewriteVariable(cursor *Cursor, node *Variable) {
 	if v, isSet := cursor.Parent().(*SetExpr); isSet && v.Var == node {
 		return
 	}
+	// no rewriting for global scope variable.
+	// this should be returned from the underlying database.
 	switch node.Scope {
 	case VariableScope:
 		er.udvRewrite(cursor, node)
-	case GlobalScope, SessionScope, NextTxScope:
+	case SessionScope, NextTxScope:
 		er.sysVarRewrite(cursor, node)
 	}
 }

go/vt/sysvars/sysvars.go

@@ -191,6 +191,7 @@ var (
 		{Name: ForeignKeyChecks, IsBoolean: true, SupportSetVar: true},
 		{Name: "group_concat_max_len", SupportSetVar: true},
 		{Name: "information_schema_stats_expiry"},
+		{Name: "innodb_lock_wait_timeout"},
 		{Name: "max_heap_table_size", SupportSetVar: true},
 		{Name: "max_seeks_for_key", SupportSetVar: true},
 		{Name: "max_tmp_tables"},
@@ -246,7 +247,6 @@ var (
 		{Name: "collation_server"},
 		{Name: "completion_type"},
 		{Name: "div_precision_increment", SupportSetVar: true},
-		{Name: "innodb_lock_wait_timeout"},
 		{Name: "interactive_timeout"},
 		{Name: "lc_time_names"},
 		{Name: "lock_wait_timeout", SupportSetVar: true},

go/vt/vtgate/executor_select_test.go

@@ -4348,3 +4348,50 @@ func TestStreamJoinQuery(t *testing.T) {
 		utils.MustMatch(t, wantResult.Rows[idx], result.Rows[idx], "mismatched on: ", strconv.Itoa(idx))
 	}
 }
+
+// TestSysVarGlobalAndSession tests that global and session variables are set correctly.
+// It also tests that setting a global variable does not affect the session variable and vice versa.
+// Also, test what happens on running select @@global and select @@session for a system variable.
+func TestSysVarGlobalAndSession(t *testing.T) {
+	executor, sbc1, _, _, _ := createExecutorEnv(t)
+	executor.normalize = true
+	session := NewAutocommitSession(&vtgatepb.Session{EnableSystemSettings: true, SystemVariables: map[string]string{}})
+
+	sbc1.SetResults([]*sqltypes.Result{
+		sqltypes.MakeTestResult(sqltypes.MakeTestFields("innodb_lock_wait_timeout", "uint64"), "20"),
+		sqltypes.MakeTestResult(sqltypes.MakeTestFields("innodb_lock_wait_timeout", "uint64"), "20"),
+		sqltypes.MakeTestResult(sqltypes.MakeTestFields("1", "int64")),
+		sqltypes.MakeTestResult(sqltypes.MakeTestFields("new", "uint64"), "40"),
+		sqltypes.MakeTestResult(sqltypes.MakeTestFields("reserve_execute", "uint64")),
+		sqltypes.MakeTestResult(sqltypes.MakeTestFields("@@global.innodb_lock_wait_timeout", "uint64"), "20"),
+	})
+	qr, err := executor.Execute(context.Background(), nil, "TestSetStmt", session,
+		"select @@innodb_lock_wait_timeout", nil)
+	require.NoError(t, err)
+	require.Equal(t, `[[UINT64(20)]]`, fmt.Sprintf("%v", qr.Rows))
+
+	qr, err = executor.Execute(context.Background(), nil, "TestSetStmt", session,
+		"select @@global.innodb_lock_wait_timeout", nil)
+	require.NoError(t, err)
+	require.Equal(t, `[[UINT64(20)]]`, fmt.Sprintf("%v", qr.Rows))
+
+	_, err = executor.Execute(context.Background(), nil, "TestSetStmt", session,
+		"set @@global.innodb_lock_wait_timeout = 120", nil)
+	require.NoError(t, err)
+	require.Empty(t, session.SystemVariables["innodb_lock_wait_timeout"])
+
+	_, err = executor.Execute(context.Background(), nil, "TestSetStmt", session,
+		"set @@innodb_lock_wait_timeout = 40", nil)
+	require.NoError(t, err)
+	require.EqualValues(t, "40", session.SystemVariables["innodb_lock_wait_timeout"])
+
+	qr, err = executor.Execute(context.Background(), nil, "TestSetStmt", session,
+		"select @@innodb_lock_wait_timeout", nil)
+	require.NoError(t, err)
+	require.Equal(t, `[[INT64(40)]]`, fmt.Sprintf("%v", qr.Rows))
+
+	qr, err = executor.Execute(context.Background(), nil, "TestSetStmt", session,
+		"select @@global.innodb_lock_wait_timeout", nil)
+	require.NoError(t, err)
+	require.Equal(t, `[[UINT64(20)]]`, fmt.Sprintf("%v", qr.Rows))
+}

go/vt/vtgate/executor_set_test.go

@@ -364,6 +364,13 @@ func TestExecutorSetOp(t *testing.T) {
 		in:      "set tx_isolation = 'read-committed'",
 		sysVars: map[string]string{"tx_isolation": "'read-committed'"},
 		result:  returnResult("tx_isolation", "varchar", "read-committed"),
+	}, {
+		in:      "set @@innodb_lock_wait_timeout=120",
+		sysVars: map[string]string{"innodb_lock_wait_timeout": "120"},
+		result:  returnResult("innodb_lock_wait_timeout", "int64", "120"),
+	}, {
+		in:     "set @@global.innodb_lock_wait_timeout=120",
+		result: returnResult("innodb_lock_wait_timeout", "int64", "120"),
 	}}
 	for _, tcase := range testcases {
 		t.Run(tcase.in, func(t *testing.T) {