leave some settings to be handled by the legacy code for now

Signed-off-by: Andres Taylor <[email protected]>
vitessio · Jul 24, 2020 · acad359 · acad359
1 parent 05dd2fe
commit acad359
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 26 deletions.
diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go
@@ -39,7 +39,7 @@ import (
 )
 
 func TestExecutorSet(t *testing.T) {
-	executor, _, _, _ := createLegacyExecutorEnv()
+	executorEnv, _, _, _ := createExecutorEnv()
 
 	testcases := []struct {
 		in  string
@@ -238,17 +238,11 @@ func TestExecutorSet(t *testing.T) {
 		in:  "set skip_query_plan_cache = 0",
 		out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}},
 	}, {
-		in: "set sql_auto_is_null = 0",
-		out: &vtgatepb.Session{Autocommit: true, Warnings: []*querypb.QueryWarning{{
-			Code:    1235,
-			Message: "Ignored inapplicable SET sql_auto_is_null = 0",
-		}}},
+		in:  "set sql_auto_is_null = 0",
+		out: &vtgatepb.Session{Autocommit: true},
 	}, {
-		in: "set sql_auto_is_null = 1",
-		out: &vtgatepb.Session{Autocommit: true, Warnings: []*querypb.QueryWarning{{
-			Code:    1235,
-			Message: "Ignored inapplicable SET sql_auto_is_null = 1",
-		}}},
+		in:  "set sql_auto_is_null = 1",
+		out: &vtgatepb.Session{Autocommit: true, RowCount: -1},
 	}, {
 		in:  "set tx_read_only = 2",
 		err: "unexpected value for tx_read_only: 2",
@@ -295,11 +289,11 @@ func TestExecutorSet(t *testing.T) {
 	for _, tcase := range testcases {
 		t.Run(tcase.in, func(t *testing.T) {
 			session := NewSafeSession(&vtgatepb.Session{Autocommit: true})
-			_, err := executor.Execute(context.Background(), "TestExecute", session, tcase.in, nil)
-			if err != nil {
-				require.EqualError(t, err, tcase.err)
+			_, err := executorEnv.Execute(context.Background(), "TestExecute", session, tcase.in, nil)
+			if tcase.err == "" {
+				utils.MustMatch(t, tcase.out, session.Session, "new executor")
 			} else {
-				utils.MustMatch(t, tcase.out, session.Session, "session output was not as expected")
+				require.EqualError(t, err, tcase.err)
 			}
 		})
 	}

diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go
@@ -98,20 +98,54 @@ var ignoreThese = []string{
 	"query_prealloc_size",
 	"sql_buffer_result",
 	"transaction_alloc_block_size",
-	"sql_auto_is_null",
-	"wait_timeout",
 }
 
 var saveSettingsToSession = []string{
 	"sql_mode",
 	"sql_safe_updates",
 }
+
 var allowSetIfValueAlreadySet = []string{}
 
+var vitessShouldBeAwareOf = []string{
+	"block_encryption_mode",
+	"character_set_client",
+	"character_set_connection",
+	"character_set_database",
+	"character_set_filesystem",
+	"character_set_server",
+	"collation_connection",
+	"collation_database",
+	"collation_server",
+	"completion_type",
+	"div_precision_increment",
+	"innodb_lock_wait_timeout",
+	"interactive_timeout",
+	"lc_time_names",
+	"lock_wait_timeout",
+	"max_allowed_packet",
+	"max_error_count",
+	"max_execution_time",
+	"max_join_size",
+	"max_length_for_sort_data",
+	"max_sort_length",
+	"max_user_connections",
+	"session_track_gtids",
+	"session_track_schema",
+	"session_track_state_change",
+	"session_track_system_variables",
+	"session_track_transaction_info",
+	"time_zone",
+	"transaction_isolation",
+	"version_tokens_session",
+	"wait_timeout",
+}
+
 func init() {
 	forSettings(ignoreThese, buildSetOpIgnore)
 	forSettings(saveSettingsToSession, buildSetOpVarSet)
 	forSettings(allowSetIfValueAlreadySet, buildSetOpCheckAndIgnore)
+	forSettings(vitessShouldBeAwareOf, buildSetOpCheckAndIgnore)
 	forSettings(notSupported, buildNotSupported)
 }
 

diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_cases.txt
@@ -135,10 +135,10 @@
 }
 
 # multiple sysvar cases
-"SET @@SESSION.sql_mode = CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO'), @@SESSION.sql_auto_is_null = 0, @@SESSION.wait_timeout = 2147483"
+"SET @@SESSION.sql_mode = CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO'), @@SESSION.sql_safe_updates = 0"
 {
   "QueryType": "SET",
-  "Original": "SET @@SESSION.sql_mode = CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO'), @@SESSION.sql_auto_is_null = 0, @@SESSION.wait_timeout = 2147483",
+  "Original": "SET @@SESSION.sql_mode = CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO'), @@SESSION.sql_safe_updates = 0",
   "Instructions": {
     "OperatorType": "Set",
     "Ops": [
@@ -152,14 +152,13 @@
         "Expr": "CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO')"
       },
       {
-        "Type": "SysVarIgnore",
-        "Name": "sql_auto_is_null",
+        "Type": "SysVarSet",
+        "Name": "sql_safe_updates",
+        "Keyspace": {
+          "Name": "main",
+          "Sharded": false
+        },
         "Expr": "0"
-      },
-      {
-        "Type": "SysVarIgnore",
-        "Name": "wait_timeout",
-        "Expr": "2147483"
       }
     ],
     "Inputs": [