diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go index 8373aadca3f..430c0367a44 100644 --- a/go/vt/vtgate/engine/set.go +++ b/go/vt/vtgate/engine/set.go @@ -343,29 +343,28 @@ func (svs *SysVarReservedConn) checkAndUpdateSysVar(vcursor VCursor, res evaleng return false, nil } - var newVal string + var value sqltypes.Value if svs.Name == "sql_mode" { - changed, newVal = sqlModeChangedValue(qr) + changed, value = sqlModeChangedValue(qr) if !changed { return false, nil } } else { - buf := new(bytes.Buffer) - value := qr.Rows[0][0] - value.EncodeSQL(buf) - newVal = buf.String() + value = qr.Rows[0][0] } - vcursor.Session().SetSysVar(svs.Name, newVal) + buf := new(bytes.Buffer) + value.EncodeSQL(buf) + vcursor.Session().SetSysVar(svs.Name, buf.String()) vcursor.Session().NeedsReservedConn() return true, nil } -func sqlModeChangedValue(qr *sqltypes.Result) (bool, string) { +func sqlModeChangedValue(qr *sqltypes.Result) (bool, sqltypes.Value) { if len(qr.Fields) != 2 { - return false, "" + return false, sqltypes.Value{} } if len(qr.Rows[0]) != 2 { - return false, "" + return false, sqltypes.Value{} } orig := qr.Rows[0][0].ToString() newVal := qr.Rows[0][1].ToString() @@ -399,7 +398,7 @@ func sqlModeChangedValue(qr *sqltypes.Result) (bool, string) { changed = true } - return changed, newVal + return changed, qr.Rows[0][1] } var _ SetOp = (*SysVarSetAware)(nil) diff --git a/go/vt/vtgate/engine/set_test.go b/go/vt/vtgate/engine/set_test.go index 46dc3705305..42700b6886a 100644 --- a/go/vt/vtgate/engine/set_test.go +++ b/go/vt/vtgate/engine/set_test.go @@ -361,7 +361,7 @@ func TestSetTable(t *testing.T) { expectedQueryLog: []string{ `ResolveDestinations ks [] Destinations:DestinationKeyspaceID(00)`, `ExecuteMultiShard ks.-20: select @@sql_mode orig, 'B,a,A,B,b,a,c' new {} false false`, - "SysVar set with (sql_mode,B,a,A,B,b,a,c)", + "SysVar set with (sql_mode,'B,a,A,B,b,a,c')", }, qr: []*sqltypes.Result{sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"), "a,b|B,a,A,B,b,a,c", @@ -378,11 +378,61 @@ func TestSetTable(t *testing.T) { expectedQueryLog: []string{ `ResolveDestinations ks [] Destinations:DestinationKeyspaceID(00)`, `ExecuteMultiShard ks.-20: select @@sql_mode orig, 'B,b,B,b' new {} false false`, - "SysVar set with (sql_mode,B,b,B,b)", + "SysVar set with (sql_mode,'B,b,B,b')", }, qr: []*sqltypes.Result{sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"), "a,b|B,b,B,b", )}, + }, { + testName: "sql_mode no change - empty list", + setOps: []SetOp{ + &SysVarReservedConn{ + Name: "sql_mode", + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + Expr: "''", + }, + }, + expectedQueryLog: []string{ + `ResolveDestinations ks [] Destinations:DestinationKeyspaceID(00)`, + `ExecuteMultiShard ks.-20: select @@sql_mode orig, '' new {} false false`, + }, + qr: []*sqltypes.Result{sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"), + "|", + )}, + }, { + testName: "sql_mode no change - empty orig", + setOps: []SetOp{ + &SysVarReservedConn{ + Name: "sql_mode", + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + Expr: "'a'", + }, + }, + expectedQueryLog: []string{ + `ResolveDestinations ks [] Destinations:DestinationKeyspaceID(00)`, + `ExecuteMultiShard ks.-20: select @@sql_mode orig, 'a' new {} false false`, + "SysVar set with (sql_mode,'a')", + }, + qr: []*sqltypes.Result{sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"), + "|a", + )}, + }, { + testName: "sql_mode no change - empty new", + setOps: []SetOp{ + &SysVarReservedConn{ + Name: "sql_mode", + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + Expr: "''", + }, + }, + expectedQueryLog: []string{ + `ResolveDestinations ks [] Destinations:DestinationKeyspaceID(00)`, + `ExecuteMultiShard ks.-20: select @@sql_mode orig, '' new {} false false`, + "SysVar set with (sql_mode,'')", + }, + qr: []*sqltypes.Result{sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"), + "a|", + )}, }} for _, tc := range tests { diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index 2baa339778c..8f36adc74d9 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -286,7 +286,7 @@ func TestExecutorSetOp(t *testing.T) { }, { in: "set sql_mode = 'STRICT_ALL_TABLES,NO_AUTO_UPDATES'", sysVars: map[string]string{"sql_mode": "'STRICT_ALL_TABLES,NO_AUTO_UPDATES'"}, - result: returnResult("sql_mode", "varchar", "STRICT_ALL_TABLES,NO_AUTO_UPDATES"), + result: sqltypes.MakeTestResult(sqltypes.MakeTestFields("orig|new", "varchar|varchar"), "|STRICT_ALL_TABLES,NO_AUTO_UPDATES"), }, { // even though the tablet is saying that the value has changed, // useReservedConn is false, so we won't allow this change