From a80e7f136ee263cee3fbbdbbbd6767ccd3246ce8 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 24 Jul 2020 15:28:02 +0200 Subject: [PATCH] less warnings when not applying system settings Signed-off-by: Andres Taylor --- .../vtgate/setstatement/sysvar_test.go | 56 ++++++++----------- go/vt/vtgate/engine/set.go | 19 +++++-- go/vt/vtgate/engine/set_test.go | 9 --- go/vt/vtgate/executor_set_test.go | 7 +-- go/vt/vtgate/planbuilder/set.go | 3 +- 5 files changed, 39 insertions(+), 55 deletions(-) diff --git a/go/test/endtoend/vtgate/setstatement/sysvar_test.go b/go/test/endtoend/vtgate/setstatement/sysvar_test.go index c0c1c8e576b..afc0f7e7e88 100644 --- a/go/test/endtoend/vtgate/setstatement/sysvar_test.go +++ b/go/test/endtoend/vtgate/setstatement/sysvar_test.go @@ -55,26 +55,29 @@ func TestSetSysVar(t *testing.T) { Port: clusterInstance.VtgateMySQLPort, } type queriesWithExpectations struct { - query string - expectedRows string - rowsAffected int - errMsg string - expectedWarning string + name, expr, expected string } queries := []queriesWithExpectations{{ - query: `set @@default_storage_engine = INNODB`, - expectedRows: ``, rowsAffected: 0, - expectedWarning: "[[VARCHAR(\"Warning\") UINT16(1235) VARCHAR(\"Ignored inapplicable SET default_storage_engine = INNODB\")]]", + name: "default_storage_engine", + expr: "INNODB", + expected: `[[VARCHAR("InnoDB")]]`, }, { - query: `set @@sql_mode = @@sql_mode`, - expectedRows: ``, rowsAffected: 0, + name: "sql_mode", + expr: "''", + expected: `[[VARCHAR("")]]`, }, { - query: `set @@sql_mode = concat(@@sql_mode,"")`, - expectedRows: ``, rowsAffected: 0, + name: "sql_mode", + expr: `concat(@@sql_mode,"NO_ZERO_DATE")`, + expected: `[[VARCHAR("NO_ZERO_DATE")]]`, }, { - query: `set @@SQL_SAFE_UPDATES = 1`, - expectedRows: ``, rowsAffected: 0, + name: "sql_mode", + expr: "@@sql_mode", + expected: `[[VARCHAR("NO_ZERO_DATE")]]`, + }, { + name: "SQL_SAFE_UPDATES", + expr: "1", + expected: "[[INT64(1)]]", }} conn, err := mysql.Connect(ctx, &vtParams) @@ -82,26 +85,11 @@ func TestSetSysVar(t *testing.T) { defer conn.Close() for i, q := range queries { - t.Run(fmt.Sprintf("%d-%s", i, q.query), func(t *testing.T) { - qr, err := exec(t, conn, q.query) - if q.errMsg != "" { - require.Contains(t, err.Error(), q.errMsg) - } else { - require.NoError(t, err) - require.Equal(t, uint64(q.rowsAffected), qr.RowsAffected, "rows affected wrong for query: %s", q.query) - if q.expectedRows != "" { - result := fmt.Sprintf("%v", qr.Rows) - if diff := cmp.Diff(q.expectedRows, result); diff != "" { - t.Errorf("%s\nfor query: %s", diff, q.query) - } - } - if q.expectedWarning != "" { - qr := checkedExec(t, conn, "show warnings") - if got, want := fmt.Sprintf("%v", qr.Rows), q.expectedWarning; got != want { - t.Errorf("select:\n%v want\n%v", got, want) - } - } - } + query := fmt.Sprintf("set %s = %s", q.name, q.expr) + t.Run(fmt.Sprintf("%d-%s", i, query), func(t *testing.T) { + _, err := exec(t, conn, query) + require.NoError(t, err) + assertMatches(t, conn, fmt.Sprintf("select @@%s", q.name), q.expected) }) } } diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go index 74c168ba481..042d34528f0 100644 --- a/go/vt/vtgate/engine/set.go +++ b/go/vt/vtgate/engine/set.go @@ -21,12 +21,12 @@ import ( "encoding/json" "fmt" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/srvtopo" "vitess.io/vitess/go/vt/vtgate/evalengine" - "vitess.io/vitess/go/mysql" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" querypb "vitess.io/vitess/go/vt/proto/query" @@ -77,6 +77,15 @@ type ( TargetDestination key.Destination `json:",omitempty"` Expr string } + + // SysVarSetSpecial implements the SetOp interface and will write the changes variable into the session + // The special part is that these settings change the sessions behaviour in different ways + SysVarSetSpecial struct { + Name string + Keyspace *vindexes.Keyspace + TargetDestination key.Destination `json:",omitempty"` + Expr string + } ) var _ Primitive = (*Set)(nil) @@ -197,8 +206,8 @@ func (svi *SysVarIgnore) VariableName() string { } //Execute implements the SetOp interface method. -func (svi *SysVarIgnore) Execute(vcursor VCursor, _ evalengine.ExpressionEnv) error { - vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: mysql.ERNotSupportedYet, Message: fmt.Sprintf("Ignored inapplicable SET %v = %v", svi.Name, svi.Expr)}) +func (svi *SysVarIgnore) Execute(VCursor, evalengine.ExpressionEnv) error { + log.Infof("Ignored inapplicable SET %v = %v", svi.Name, svi.Expr) return nil } @@ -237,7 +246,7 @@ func (svci *SysVarCheckAndIgnore) Execute(vcursor VCursor, env evalengine.Expres return err } if result.RowsAffected == 0 { - vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: mysql.ERNotSupportedYet, Message: fmt.Sprintf("Modification not allowed using set construct for: %s", svci.Name)}) + log.Infof("Ignored inapplicable SET %v = %v", svci.Name, svci.Expr) } return nil } diff --git a/go/vt/vtgate/engine/set_test.go b/go/vt/vtgate/engine/set_test.go index 308a7470c7f..da4b7ae3ec7 100644 --- a/go/vt/vtgate/engine/set_test.go +++ b/go/vt/vtgate/engine/set_test.go @@ -106,9 +106,6 @@ func TestSetTable(t *testing.T) { Expr: "42", }, }, - expectedWarning: []*querypb.QueryWarning{ - {Code: 1235, Message: "Ignored inapplicable SET x = 42"}, - }, }, { testName: "sysvar check and ignore", @@ -152,9 +149,6 @@ func TestSetTable(t *testing.T) { `ResolveDestinations ks [] Destinations:DestinationAnyShard()`, `ExecuteMultiShard ks.-20: select 1 from dual where @@x = dummy_expr {} false false`, }, - expectedWarning: []*querypb.QueryWarning{ - {Code: 1235, Message: "Modification not allowed using set construct for: x"}, - }, }, { testName: "sysvar checkAndIgnore multi destination error", @@ -200,9 +194,6 @@ func TestSetTable(t *testing.T) { `ResolveDestinations ks [] Destinations:DestinationAnyShard()`, `ExecuteMultiShard ks.-20: select 1 from dual where @@z = dummy_expr {} false false`, }, - expectedWarning: []*querypb.QueryWarning{ - {Code: 1235, Message: "Ignored inapplicable SET y = 2"}, - }, qr: []*sqltypes.Result{sqltypes.MakeTestResult( sqltypes.MakeTestFields( "id", diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index bda1558b42c..ec9e24402cd 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -19,7 +19,6 @@ package vtgate import ( "testing" - "vitess.io/vitess/go/mysql" querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/test/utils" @@ -239,7 +238,7 @@ func TestExecutorSet(t *testing.T) { out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}}, }, { in: "set sql_auto_is_null = 0", - out: &vtgatepb.Session{Autocommit: true}, + out: &vtgatepb.Session{Autocommit: true, RowCount: -1}, }, { in: "set sql_auto_is_null = 1", out: &vtgatepb.Session{Autocommit: true, RowCount: -1}, @@ -313,10 +312,6 @@ func TestExecutorSetOp(t *testing.T) { sysVars map[string]string }{{ in: "set big_tables = 1", - warning: []*querypb.QueryWarning{{ - Code: mysql.ERNotSupportedYet, - Message: "Ignored inapplicable SET big_tables = 1", - }}, }, { in: "set sql_mode = 'STRICT_ALL_TABLES,NO_AUTO_UPDATES'", sysVars: map[string]string{"sql_mode": "'STRICT_ALL_TABLES,NO_AUTO_UPDATES'"}, diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go index 0980fc10094..8bfd5ddf06d 100644 --- a/go/vt/vtgate/planbuilder/set.go +++ b/go/vt/vtgate/planbuilder/set.go @@ -98,6 +98,7 @@ var ignoreThese = []string{ "query_prealloc_size", "sql_buffer_result", "transaction_alloc_block_size", + "wait_timeout", } var saveSettingsToSession = []string{ @@ -138,7 +139,7 @@ var vitessShouldBeAwareOf = []string{ "time_zone", "transaction_isolation", "version_tokens_session", - "wait_timeout", + "sql_auto_is_null", } func init() {