sql_mode set to ' ' from '' only for SET_VAR, stored as-is in the ses…

…sion Signed-off-by: Harshit Gangal <harshit@planetscale.com>
planetscale · Feb 18, 2025 · 5085d6c · 5085d6c
1 parent 5637c45
commit 5085d6c
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 8 deletions.
diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go
@@ -352,11 +352,6 @@ func (svs *SysVarReservedConn) checkAndUpdateSysVar(ctx context.Context, vcursor
 	var buf strings.Builder
 	value.EncodeSQL(&buf)
 	s := buf.String()
-	if s == `''` {
-		// SET_VAR(sql_mode, '') is not accepted by MySQL, giving a warning:
-		// | Warning | 1064 | Optimizer hint syntax error near ''') */
-		s = `' '`
-	}
 	vcursor.Session().SetSysVar(svs.Name, s)
 
 	// If the condition below is true, we want to use reserved connection instead of SET_VAR query hint.

diff --git a/go/vt/vtgate/engine/set_test.go b/go/vt/vtgate/engine/set_test.go
@@ -451,7 +451,7 @@ func TestSetTable(t *testing.T) {
 		expectedQueryLog: []string{
 			`ResolveDestinations ks [] Destinations:DestinationKeyspaceID(00)`,
 			`ExecuteMultiShard ks.-20: select @@sql_mode orig, '' new {} false false`,
-			"SysVar set with (sql_mode,' ')",
+			"SysVar set with (sql_mode,'')",
 			"SET_VAR can be used",
 		},
 		qr: []*sqltypes.Result{sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"),
@@ -491,7 +491,7 @@ func TestSetTable(t *testing.T) {
 		expectedQueryLog: []string{
 			`ResolveDestinations ks [] Destinations:DestinationKeyspaceID(00)`,
 			`ExecuteMultiShard ks.-20: select @@sql_mode orig, '' new {} false false`,
-			"SysVar set with (sql_mode,' ')",
+			"SysVar set with (sql_mode,'')",
 			"SET_VAR can be used",
 		},
 		qr: []*sqltypes.Result{sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"),

diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go
@@ -441,7 +441,7 @@ func TestSetSystemVariablesWithSetVarInvalidSQLMode(t *testing.T) {
 		{Sql: "select /*+ SET_VAR(sql_mode = ' ') */ age, city + :vtg1 /* INT64 */, weight_string(age) from `user` group by age, weight_string(age) order by age asc", BindVariables: map[string]*querypb.BindVariable{"vtg1": {Type: sqltypes.Int64, Value: []byte("1")}}},
 	}
 	utils.MustMatch(t, wantQueries, sbc1.Queries)
-	require.Equal(t, "' '", session.SystemVariables["sql_mode"])
+	require.Equal(t, "''", session.SystemVariables["sql_mode"])
 }
 
 func TestSelectVindexFunc(t *testing.T) {

diff --git a/go/vt/vtgate/executorcontext/vcursor_impl.go b/go/vt/vtgate/executorcontext/vcursor_impl.go
@@ -219,6 +219,11 @@ func (vc *VCursorImpl) PrepareSetVarComment() string {
 	var res []string
 	vc.Session().GetSystemVariables(func(k, v string) {
 		if sysvars.SupportsSetVar(k) {
+			if k == "sql_mode" && v == "''" {
+				// SET_VAR(sql_mode, '') is not accepted by MySQL, giving a warning:
+				// | Warning | 1064 | Optimizer hint syntax error near ''') */
+				v = "' '"
+			}
 			res = append(res, fmt.Sprintf("SET_VAR(%s = %s)", k, v))
 		}
 	})