Fix VitessAware system variables of type boolean return NULL when MySQL is not involved

go/sqltypes/bind_variables.go

@@ -75,12 +75,12 @@ func Int32BindVariable(v int32) *querypb.BindVariable {
 	return ValueBindVariable(NewInt32(v))
 }
 
-// BoolBindVariable converts an bool to a int32 bind var.
+// BoolBindVariable converts an bool to a int64 bind var.
 func BoolBindVariable(v bool) *querypb.BindVariable {
 	if v {
-		return Int32BindVariable(1)
+		return Int64BindVariable(1)
 	}
-	return Int32BindVariable(0)
+	return Int64BindVariable(0)
 }
 
 // Int64BindVariable converts an int64 to a bind var.

go/test/endtoend/vtgate/reservedconn/main_test.go

@@ -169,3 +169,13 @@ func assertIsEmpty(t *testing.T, conn *mysql.Conn, query string) {
 	qr := checkedExec(t, conn, query)
 	assert.Empty(t, qr.Rows)
 }
+
+func assertResponseMatch(t *testing.T, conn *mysql.Conn, query1, query2 string) {
+	qr1 := checkedExec(t, conn, query1)
+	got1 := fmt.Sprintf("%v", qr1.Rows)
+
+	qr2 := checkedExec(t, conn, query2)
+	got2 := fmt.Sprintf("%v", qr2.Rows)
+
+	assert.Equal(t, got1, got2)
+}

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

@@ -329,16 +329,11 @@ func TestEnableSystemSettings(t *testing.T) {
 	require.NoError(t, err)
 	defer conn.Close()
 
-	// Insert a single row to correctly select @@enable_system_settings.
-	// See: https://github.com/vitessio/vitess/issues/7301
-	checkedExec(t, conn, "delete from test")
-	checkedExec(t, conn, "insert into test (id, val1, val2, val3) values (1, null, 0, 0)")
-
 	// test set @@enable_system_settings to false and true
 	checkedExec(t, conn, "set enable_system_settings = false")
-	assertMatches(t, conn, `select @@enable_system_settings from test`, `[[INT64(0)]]`)
+	assertMatches(t, conn, `select @@enable_system_settings`, `[[INT64(0)]]`)
 	checkedExec(t, conn, "set enable_system_settings = true")
-	assertMatches(t, conn, `select @@enable_system_settings from test`, `[[INT64(1)]]`)
+	assertMatches(t, conn, `select @@enable_system_settings`, `[[INT64(1)]]`)
 
 	// prepare the @@sql_mode variable
 	checkedExec(t, conn, "set sql_mode = 'NO_ZERO_DATE'")
@@ -354,3 +349,27 @@ func TestEnableSystemSettings(t *testing.T) {
 	checkedExec(t, conn, "set sql_mode = ''")                       // changing @@sql_mode to empty string
 	assertMatches(t, conn, "select 	@@sql_mode", `[[VARCHAR("")]]`) // @@sql_mode did change
 }
+
+// Tests type consitency through multiple queries
+func TestSystemVariableType(t *testing.T) {
+	vtParams := mysql.ConnParams{
+		Host: "localhost",
+		Port: clusterInstance.VtgateMySQLPort,
+	}
+	conn, err := mysql.Connect(context.Background(), &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	checkedExec(t, conn, "delete from test")
+	checkedExec(t, conn, "insert into test (id, val1, val2, val3) values (1, null, 0, 0)")
+
+	// regardless of the "from", the select @@autocommit should return the same type
+	query1 := "select @@autocommit"
+	query2 := "select @@autocommit from test"
+
+	checkedExec(t, conn, "set autocommit = false")
+	assertResponseMatch(t, conn, query1, query2)
+
+	checkedExec(t, conn, "set autocommit = true")
+	assertResponseMatch(t, conn, query1, query2)
+}

go/vt/vtgate/evalengine/evalengine.go

@@ -200,34 +200,34 @@ func (v EvalResult) toSQLValue(resultType querypb.Type) sqltypes.Value {
 	switch {
 	case sqltypes.IsSigned(resultType):
 		switch v.typ {
-		case sqltypes.Int64:
-			return sqltypes.MakeTrusted(resultType, strconv.AppendInt(nil, v.ival, 10))
-		case sqltypes.Uint64:
+		case sqltypes.Int64, sqltypes.Int32:
+			return sqltypes.MakeTrusted(resultType, strconv.AppendInt(nil, int64(v.ival), 10))
+		case sqltypes.Uint64, sqltypes.Uint32:
 			return sqltypes.MakeTrusted(resultType, strconv.AppendInt(nil, int64(v.uval), 10))
-		case sqltypes.Float64:
+		case sqltypes.Float64, sqltypes.Float32:
 			return sqltypes.MakeTrusted(resultType, strconv.AppendInt(nil, int64(v.fval), 10))
 		}
 	case sqltypes.IsUnsigned(resultType):
 		switch v.typ {
-		case sqltypes.Uint64:
-			return sqltypes.MakeTrusted(resultType, strconv.AppendUint(nil, v.uval, 10))
-		case sqltypes.Int64:
+		case sqltypes.Uint64, sqltypes.Uint32:
+			return sqltypes.MakeTrusted(resultType, strconv.AppendUint(nil, uint64(v.uval), 10))
+		case sqltypes.Int64, sqltypes.Int32:
 			return sqltypes.MakeTrusted(resultType, strconv.AppendUint(nil, uint64(v.ival), 10))
-		case sqltypes.Float64:
+		case sqltypes.Float64, sqltypes.Float32:
 			return sqltypes.MakeTrusted(resultType, strconv.AppendUint(nil, uint64(v.fval), 10))
 		}
 	case sqltypes.IsFloat(resultType) || resultType == sqltypes.Decimal:
 		switch v.typ {
-		case sqltypes.Int64:
-			return sqltypes.MakeTrusted(resultType, strconv.AppendInt(nil, v.ival, 10))
-		case sqltypes.Uint64:
-			return sqltypes.MakeTrusted(resultType, strconv.AppendUint(nil, v.uval, 10))
-		case sqltypes.Float64:
+		case sqltypes.Int64, sqltypes.Int32:
+			return sqltypes.MakeTrusted(resultType, strconv.AppendInt(nil, int64(v.ival), 10))
+		case sqltypes.Uint64, sqltypes.Uint32:
+			return sqltypes.MakeTrusted(resultType, strconv.AppendUint(nil, uint64(v.uval), 10))
+		case sqltypes.Float64, sqltypes.Float32:
 			format := byte('g')
 			if resultType == sqltypes.Decimal {
 				format = 'f'
 			}
-			return sqltypes.MakeTrusted(resultType, strconv.AppendFloat(nil, v.fval, format, -1, 64))
+			return sqltypes.MakeTrusted(resultType, strconv.AppendFloat(nil, float64(v.fval), format, -1, 64))
 		}
 	default:
 		return sqltypes.MakeTrusted(resultType, v.bytes)

go/vt/vtgate/executor_select_test.go

@@ -313,7 +313,7 @@ func TestSelectLastInsertId(t *testing.T) {
 		}},
 	}
 	require.NoError(t, err)
-	utils.MustMatch(t, result, wantResult, "Mismatch")
+	utils.MustMatch(t, wantResult, result, "Mismatch")
 }
 
 func TestSelectSystemVariables(t *testing.T) {
@@ -334,10 +334,10 @@ func TestSelectSystemVariables(t *testing.T) {
 	result, err := executorExec(executor, sql, map[string]*querypb.BindVariable{})
 	wantResult := &sqltypes.Result{
 		Fields: []*querypb.Field{
-			{Name: "@@autocommit", Type: sqltypes.Int32},
-			{Name: "@@client_found_rows", Type: sqltypes.Int32},
-			{Name: "@@skip_query_plan_cache", Type: sqltypes.Int32},
-			{Name: "@@enable_system_settings", Type: sqltypes.Int32},
+			{Name: "@@autocommit", Type: sqltypes.Int64},
+			{Name: "@@client_found_rows", Type: sqltypes.Int64},
+			{Name: "@@skip_query_plan_cache", Type: sqltypes.Int64},
+			{Name: "@@enable_system_settings", Type: sqltypes.Int64},
 			{Name: "@@sql_select_limit", Type: sqltypes.Int64},
 			{Name: "@@transaction_mode", Type: sqltypes.VarBinary},
 			{Name: "@@workload", Type: sqltypes.VarBinary},
@@ -349,10 +349,10 @@ func TestSelectSystemVariables(t *testing.T) {
 		RowsAffected: 1,
 		Rows: [][]sqltypes.Value{{
 			// the following are the uninitialised session values
-			sqltypes.NULL,
-			sqltypes.NULL,
-			sqltypes.NULL,
-			sqltypes.NULL,
+			sqltypes.NewInt64(0),
+			sqltypes.NewInt64(0),
+			sqltypes.NewInt64(0),
+			sqltypes.NewInt64(0),
 			sqltypes.NewInt64(0),
 			sqltypes.NewVarBinary("UNSPECIFIED"),
 			sqltypes.NewVarBinary(""),
@@ -364,7 +364,39 @@ func TestSelectSystemVariables(t *testing.T) {
 		}},
 	}
 	require.NoError(t, err)
-	utils.MustMatch(t, result, wantResult, "Mismatch")
+	utils.MustMatch(t, wantResult, result, "Mismatch")
+}
+
+func TestSelectInitializedVitessAwareVariable(t *testing.T) {
+	executor, _, _, _ := createLegacyExecutorEnv()
+	executor.normalize = true
+	logChan := QueryLogger.Subscribe("Test")
+	defer QueryLogger.Unsubscribe(logChan)
+
+	masterSession.Autocommit = true
+	masterSession.EnableSystemSettings = true
+
+	defer func() {
+		masterSession.Autocommit = false
+		masterSession.EnableSystemSettings = false
+	}()
+
+	sql := "select @@autocommit, @@enable_system_settings"
+
+	result, err := executorExec(executor, sql, nil)
+	wantResult := &sqltypes.Result{
+		Fields: []*querypb.Field{
+			{Name: "@@autocommit", Type: sqltypes.Int64},
+			{Name: "@@enable_system_settings", Type: sqltypes.Int64},
+		},
+		RowsAffected: 1,
+		Rows: [][]sqltypes.Value{{
+			sqltypes.NewInt64(1),
+			sqltypes.NewInt64(1),
+		}},
+	}
+	require.NoError(t, err)
+	utils.MustMatch(t, wantResult, result, "Mismatch")
 }
 
 func TestSelectUserDefindVariable(t *testing.T) {
@@ -385,7 +417,7 @@ func TestSelectUserDefindVariable(t *testing.T) {
 			sqltypes.NULL,
 		}},
 	}
-	utils.MustMatch(t, result, wantResult, "Mismatch")
+	utils.MustMatch(t, wantResult, result, "Mismatch")
 
 	masterSession = &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo"}, []interface{}{"bar"})}
 	result, err = executorExec(executor, sql, map[string]*querypb.BindVariable{})
@@ -399,7 +431,7 @@ func TestSelectUserDefindVariable(t *testing.T) {
 			sqltypes.NewVarBinary("bar"),
 		}},
 	}
-	utils.MustMatch(t, result, wantResult, "Mismatch")
+	utils.MustMatch(t, wantResult, result, "Mismatch")
 }
 
 func TestFoundRows(t *testing.T) {
@@ -424,7 +456,7 @@ func TestFoundRows(t *testing.T) {
 		}},
 	}
 	require.NoError(t, err)
-	utils.MustMatch(t, result, wantResult, "Mismatch")
+	utils.MustMatch(t, wantResult, result, "Mismatch")
 }
 
 func TestRowCount(t *testing.T) {
@@ -454,7 +486,7 @@ func testRowCount(t *testing.T, executor *Executor, wantRowCount int64) {
 		}},
 	}
 	require.NoError(t, err)
-	utils.MustMatch(t, result, wantResult, "Mismatch")
+	utils.MustMatch(t, wantResult, result, "Mismatch")
 }
 
 func TestSelectLastInsertIdInUnion(t *testing.T) {
@@ -558,7 +590,7 @@ func TestSelectDatabase(t *testing.T) {
 		}},
 	}
 	require.NoError(t, err)
-	utils.MustMatch(t, result, wantResult, "Mismatch")
+	utils.MustMatch(t, wantResult, result, "Mismatch")
 
 }
 

go/vt/vttablet/endtoend/misc_test.go

@@ -760,3 +760,37 @@ func TestBeginExecuteWithFailingPreQueriesAndCheckConnectionState(t *testing.T)
 	require.NoError(t, err)
 	require.Empty(t, qr.Rows)
 }
+
+func TestSelectBooleanSystemVariables(t *testing.T) {
+	client := framework.NewClient()
+
+	type testCase struct {
+		Variable string
+		Value    bool
+		Type     querypb.Type
+	}
+
+	newTestCase := func(varname string, vartype querypb.Type, value bool) testCase {
+		return testCase{Variable: varname, Value: value, Type: vartype}
+	}
+
+	tcs := []testCase{
+		newTestCase("autocommit", querypb.Type_INT64, true),
+		newTestCase("autocommit", querypb.Type_INT64, false),
+		newTestCase("enable_system_settings", querypb.Type_INT64, true),
+		newTestCase("enable_system_settings", querypb.Type_INT64, false),
+	}
+
+	for _, tc := range tcs {
+		qr, err := client.Execute(
+			fmt.Sprintf("select :%s", tc.Variable),
+			map[string]*querypb.BindVariable{tc.Variable: sqltypes.BoolBindVariable(tc.Value)},
+		)
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		require.NotEmpty(t, qr.Fields, "fields should not be empty")
+		require.Equal(t, tc.Type, qr.Fields[0].Type, fmt.Sprintf("invalid type, wants: %+v, but got: %+v\n", tc.Type, qr.Fields[0].Type))
+	}
+}