diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index 8e509e5019a..b77c12f6b37 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -41,10 +41,9 @@ func TestExecutorSet(t *testing.T) { executorEnv, _, _, _ := createExecutorEnv() testcases := []struct { - in string - out *vtgatepb.Session - err string - target string + in string + out *vtgatepb.Session + err string }{{ in: "set autocommit = 1", out: &vtgatepb.Session{Autocommit: true}, @@ -216,53 +215,18 @@ func TestExecutorSet(t *testing.T) { }, { in: "set character set ascii", err: "unexpected value for charset/names: ascii", - }, { - in: "set net_write_timeout = 600", - out: &vtgatepb.Session{Autocommit: true}, - }, { - in: "set net_read_timeout = 600", - out: &vtgatepb.Session{Autocommit: true}, - }, { - in: "set sql_quote_show_create = 1", - out: &vtgatepb.Session{Autocommit: true}, - }, { - in: "set foreign_key_checks = 0", - out: &vtgatepb.Session{Autocommit: true}, - }, { - in: "set unique_checks = 0", - out: &vtgatepb.Session{Autocommit: true}, }, { in: "set skip_query_plan_cache = 1", out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{SkipQueryPlanCache: true}}, }, { in: "set skip_query_plan_cache = 0", out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}}, - }, { - in: "set sql_auto_is_null = 0", - target: "TestExecutor", - out: &vtgatepb.Session{Autocommit: true, TargetString: "TestExecutor"}, - }, { - in: "set sql_auto_is_null = 1", - target: "TestExecutor", - out: &vtgatepb.Session{Autocommit: true, TargetString: "TestExecutor"}, }, { in: "set tx_read_only = 2", err: "unexpected value for tx_read_only: 2", }, { in: "set transaction_read_only = 2", err: "unexpected value for transaction_read_only: 2", - }, { - in: "set tx_isolation = 'invalid'", - err: "unexpected value for tx_isolation: invalid", - }, { - in: "set @foo = 'bar'", - out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo"}, []interface{}{"bar"}), Autocommit: true}, - }, { - in: "set @foo = 2", - out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo"}, []interface{}{2}), Autocommit: true}, - }, { - in: "set @foo = 2.1, @bar = 'baz'", - out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo", "bar"}, []interface{}{2.1, "baz"}), Autocommit: true}, }, { in: "set session transaction isolation level repeatable read", out: &vtgatepb.Session{Autocommit: true}, @@ -290,9 +254,10 @@ func TestExecutorSet(t *testing.T) { }} for _, tcase := range testcases { t.Run(tcase.in, func(t *testing.T) { - session := NewSafeSession(&vtgatepb.Session{Autocommit: true, TargetString: tcase.target}) + session := NewSafeSession(&vtgatepb.Session{Autocommit: true}) _, err := executorEnv.Execute(context.Background(), "TestExecute", session, tcase.in, nil) if tcase.err == "" { + require.NoError(t, err) utils.MustMatch(t, tcase.out, session.Session, "new executor") } else { require.EqualError(t, err, tcase.err) @@ -305,8 +270,14 @@ func TestExecutorSetOp(t *testing.T) { executor, _, _, sbclookup := createLegacyExecutorEnv() sbclookup.SetResults([]*sqltypes.Result{ - sqltypes.MakeTestResult(sqltypes.MakeTestFields("'STRICT_ALL_TABLES,NO_AUTO_UPDATES'", "varchar"), "STRICT_ALL_TABLES,NO_AUTO_UPDATES"), - sqltypes.MakeTestResult(sqltypes.MakeTestFields("1", "int64"), "1"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("sql_mode", "varchar"), "STRICT_ALL_TABLES,NO_AUTO_UPDATES"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("sql_safe_updates", "int64"), "1"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("tx_isolation", "varchar"), "read-committed"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("sql_quote_show_create", "int64"), "0"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("foreign_key_checks", "int64")), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("unique_checks", "int64"), "0"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("net_write_timeout", "int64"), "600"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("net_read_timeout", "int64"), "300"), }) testcases := []struct { @@ -314,13 +285,28 @@ func TestExecutorSetOp(t *testing.T) { warning []*querypb.QueryWarning sysVars map[string]string }{{ - in: "set big_tables = 1", + in: "set big_tables = 1", //ignore }, { in: "set sql_mode = 'STRICT_ALL_TABLES,NO_AUTO_UPDATES'", sysVars: map[string]string{"sql_mode": "'STRICT_ALL_TABLES,NO_AUTO_UPDATES'"}, }, { in: "set sql_safe_updates = 1", sysVars: map[string]string{"sql_safe_updates": "1"}, + }, { + in: "set tx_isolation = 'read-committed'", + sysVars: map[string]string{"tx_isolation": "'read-committed'"}, + }, { + in: "set sql_quote_show_create = 0", + sysVars: map[string]string{"sql_quote_show_create": "0"}, + }, { + in: "set foreign_key_checks = 1", + }, { + in: "set unique_checks = 0", + sysVars: map[string]string{"unique_checks": "0"}, + }, { + in: "set net_write_timeout = 600", + }, { + in: "set net_read_timeout = 600", }} for _, tcase := range testcases { t.Run(tcase.in, func(t *testing.T) { diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go index 4559512334d..8800df00784 100644 --- a/go/vt/vtgate/planbuilder/set.go +++ b/go/vt/vtgate/planbuilder/set.go @@ -36,9 +36,12 @@ type planFunc = func(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.Se var sysVarPlanningFunc = map[string]planFunc{} var notSupported = []string{ + "audit_log_read_buffer_size", "auto_increment_increment", "auto_increment_offset", "binlog_direct_non_transactional_updates", + "binlog_row_image", + "binlog_rows_query_log_events", "innodb_ft_enable_stopword", "innodb_ft_user_stopword_table", "max_points_in_geometry", @@ -69,8 +72,9 @@ var notSupported = []string{ "preload_buffer_size", "rbr_exec_mode", "sql_log_off", + "thread_pool_high_priority_connection", + "thread_pool_prio_kickup_timer", "transaction_write_set_extraction", - "audit_log_read_buffer_size", } var ignoreThese = []string{ @@ -101,14 +105,56 @@ var ignoreThese = []string{ "wait_timeout", } -var saveSettingsToSession = []string{ +var useReservedConn = []string{ + "default_week_format", + "end_markers_in_json", + "eq_range_index_dive_limit", + "explicit_defaults_for_timestamp", + "foreign_key_checks", + "group_concat_max_len", + "max_heap_table_size", + "max_seeks_for_key", + "max_tmp_tables", + "min_examined_row_limit", + "old_passwords", + "optimizer_prune_level", + "optimizer_search_depth", + "optimizer_switch", + "optimizer_trace", + "optimizer_trace_features", + "optimizer_trace_limit", + "optimizer_trace_max_mem_size", + "transaction_isolation", + "tx_isolation", + "optimizer_trace_offset", + "parser_max_mem_size", + "profiling", + "profiling_history_size", + "query_alloc_block_size", + "range_alloc_block_size", + "range_optimizer_max_mem_size", + "read_buffer_size", + "read_rnd_buffer_size", + "show_create_table_verbosity", + "show_old_temporals", + "sort_buffer_size", + "sql_big_selects", "sql_mode", + "sql_notes", + "sql_quote_show_create", "sql_safe_updates", + "sql_warnings", + "tmp_table_size", + "transaction_prealloc_size", + "unique_checks", + "updatable_views_with_limit", } -var allowSetIfValueAlreadySet = []string{} - -var vitessShouldBeAwareOf = []string{ +// TODO: Most of these settings should be moved into SysSetOpAware, and change Vitess behaviour. +// Until then, SET statements against these settings are allowed +// as long as they have the same value as the underlying database +var checkAndIgnore = []string{ + "binlog_format", "block_encryption_mode", "character_set_client", "character_set_connection", @@ -131,22 +177,23 @@ var vitessShouldBeAwareOf = []string{ "max_length_for_sort_data", "max_sort_length", "max_user_connections", + "net_read_timeout", + "net_retry_count", + "net_write_timeout", "session_track_gtids", "session_track_schema", "session_track_state_change", "session_track_system_variables", "session_track_transaction_info", + "sql_auto_is_null", "time_zone", - "transaction_isolation", "version_tokens_session", - "sql_auto_is_null", } func init() { forSettings(ignoreThese, buildSetOpIgnore) - forSettings(saveSettingsToSession, buildSetOpVarSet) - forSettings(allowSetIfValueAlreadySet, buildSetOpCheckAndIgnore) - forSettings(vitessShouldBeAwareOf, buildSetOpCheckAndIgnore) + forSettings(useReservedConn, buildSetOpVarSet) + forSettings(checkAndIgnore, buildSetOpCheckAndIgnore) forSettings(notSupported, buildNotSupported) }