From 3e76be6040c918ac97582b69314ebcf820df1812 Mon Sep 17 00:00:00 2001 From: spongedc Date: Tue, 31 Jul 2018 00:46:21 +0800 Subject: [PATCH 1/9] executor,sessionctx: Add correctness for more system variables --- executor/set_test.go | 53 ++++++++++++++ sessionctx/variable/sysvar.go | 12 ++- sessionctx/variable/varsutil.go | 125 +++++++++++++------------------- 3 files changed, 114 insertions(+), 76 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index a0b9c2209eb60..9f6cf4947f300 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -314,4 +314,57 @@ func (s *testSuite) TestValidateSetVar(c *C) { result = tk.MustQuery("select @@time_zone;") result.Check(testkit.Rows("SYSTEM")) + tk.MustExec("set @@global.max_connections=100001") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_connections value: '100001'")) + result = tk.MustQuery("select @@global.max_connections;") + result.Check(testkit.Rows("100000")) + + tk.MustExec("set @@global.max_connections=-1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_connections value: '-1'")) + result = tk.MustQuery("select @@global.max_connections;") + result.Check(testkit.Rows("1")) + + _, err = tk.Exec("set @@global.max_connections='hello'") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + tk.MustExec("set @@global.max_connect_errors=18446744073709551615") + + tk.MustExec("set @@global.max_connect_errors=-1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_connect_errors value: '-1'")) + result = tk.MustQuery("select @@global.max_connect_errors;") + result.Check(testkit.Rows("1")) + + _, err = tk.Exec("set @@global.max_connect_errors=18446744073709551616") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + tk.MustExec("set @@global.max_connections=100001") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_connections value: '100001'")) + result = tk.MustQuery("select @@global.max_connections;") + result.Check(testkit.Rows("100000")) + + tk.MustExec("set @@global.max_connections=-1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_connections value: '-1'")) + result = tk.MustQuery("select @@global.max_connections;") + result.Check(testkit.Rows("1")) + + _, err = tk.Exec("set @@global.max_connections='hello'") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + tk.MustExec("set @@max_sort_length=1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_sort_length value: '1'")) + result = tk.MustQuery("select @@max_sort_length;") + result.Check(testkit.Rows("4")) + + tk.MustExec("set @@max_sort_length=-100") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_sort_length value: '-100'")) + result = tk.MustQuery("select @@max_sort_length;") + result.Check(testkit.Rows("4")) + + tk.MustExec("set @@max_sort_length=8388609") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_sort_length value: '8388609'")) + result = tk.MustQuery("select @@max_sort_length;") + result.Check(testkit.Rows("8388608")) + + _, err = tk.Exec("set @@max_sort_length='hello'") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) } diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 54bc6c7d83fb6..0fe134afe5ed1 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -127,7 +127,7 @@ var defaultSysVars = []*SysVar{ {ScopeSession, "rand_seed2", ""}, {ScopeGlobal, "validate_password_number_count", "1"}, {ScopeSession, "gtid_next", ""}, - {ScopeGlobal | ScopeSession, "sql_select_limit", "18446744073709551615"}, + {ScopeGlobal | ScopeSession, SQLSelectLimit, "18446744073709551615"}, {ScopeGlobal, "ndb_show_foreign_key_mock_tables", ""}, {ScopeNone, "multi_range_count", "256"}, {ScopeGlobal | ScopeSession, DefaultWeekFormat, "0"}, @@ -135,7 +135,7 @@ var defaultSysVars = []*SysVar{ {ScopeGlobal, "slave_transaction_retries", "10"}, {ScopeGlobal | ScopeSession, "default_storage_engine", "InnoDB"}, {ScopeNone, "ft_query_expansion_limit", "20"}, - {ScopeGlobal, "max_connect_errors", "100"}, + {ScopeGlobal, MaxConnectErrors, "100"}, {ScopeGlobal, "sync_binlog", "0"}, {ScopeNone, "max_digest_length", "1024"}, {ScopeNone, "innodb_force_load_corrupted", "OFF"}, @@ -145,7 +145,7 @@ var defaultSysVars = []*SysVar{ {ScopeGlobal, "log_backward_compatible_user_definitions", ""}, {ScopeNone, "lc_messages_dir", "/usr/local/mysql-5.6.25-osx10.8-x86_64/share/"}, {ScopeGlobal, "ft_boolean_syntax", "+ -><()~*:\"\"&|"}, - {ScopeGlobal, "table_definition_cache", "1400"}, + {ScopeGlobal, "table_definition_cache", "-1"}, {ScopeNone, SkipNameResolve, "0"}, {ScopeNone, "performance_schema_max_file_handles", "32768"}, {ScopeSession, "transaction_allow_batching", ""}, @@ -744,6 +744,12 @@ const ( WarningCount = "warning_count" // ErrorCount is the name for 'error_count' system variable. ErrorCount = "error_count" + // SQLSelectLimit is the name for 'sql_select_limit' system variable. + SQLSelectLimit = "sql_select_limit" + // MaxConnectErrors is the name for 'max_connect_errors' system variable. + MaxConnectErrors = "max_connect_errors" + // TableDefinitionCache is the name for 'table_definition_cache' system variable. + TableDefinitionCache = "table_definition_cache" ) // GlobalVarAccessor is the interface for accessing global scope system and status variables. diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 36cf1e9110743..896dbbcd31c8d 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -162,6 +162,37 @@ func ValidateGetSystemVar(name string, isGlobal bool) error { return nil } +func checkIntegerSystemVar(name, value string, lower int64, upper uint64, vars *SessionVars) (string, error) { + if value[0] == '-' { + val, err := strconv.ParseInt(value, 10, 64) + if err != nil { + return value, ErrWrongTypeForVar.GenByArgs(name) + } + if val < lower { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", lower), nil + } + if val > 0 && uint64(val) > upper { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", upper), nil + } + } else { + val, err := strconv.ParseUint(value, 10, 64) + if err != nil { + return value, ErrWrongTypeForVar.GenByArgs(name) + } + if val < uint64(lower) { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", lower), nil + } + if val > 0 && val > upper { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", upper), nil + } + } + return value, nil +} + // ValidateSetSystemVar checks if system variable satisfies specific restriction. func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, error) { if strings.EqualFold(value, "DEFAULT") { @@ -172,18 +203,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, } switch name { case DefaultWeekFormat: - val, err := strconv.Atoi(value) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 0 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "0", nil - } - if val > 7 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "7", nil - } + return checkIntegerSystemVar(name, value, 0, 7, vars) case DelayKeyWrite: if strings.EqualFold(value, "ON") || value == "1" { return "ON", nil @@ -203,20 +223,9 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return "0", nil } case GroupConcatMaxLen: - val, err := strconv.ParseUint(value, 10, 64) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } // The reasonable range of 'group_concat_max_len' is 4~18446744073709551615(64-bit platforms) // See https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_group_concat_max_len for details - if val < 4 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "4", nil - } - if val > 18446744073709551615 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "18446744073709551615", nil - } + return checkIntegerSystemVar(name, value, 4, 18446744073709551615, vars) case InteractiveTimeout: val, err := strconv.Atoi(value) if err != nil { @@ -227,44 +236,15 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return "1", nil } case MaxConnections: - val, err := strconv.Atoi(value) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 1 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "1", nil - } - if val > 100000 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "100000", nil - } + return checkIntegerSystemVar(name, value, 1, 100000, vars) + case MaxConnectErrors: + return checkIntegerSystemVar(name, value, 1, 18446744073709551615, vars) case MaxSortLength: - val, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 4 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "4", nil - } - if val > 8388608 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "8388608", nil - } + return checkIntegerSystemVar(name, value, 4, 8388608, vars) case MaxSpRecursionDepth: - val, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 0 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "0", nil - } - if val > 255 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "255", nil - } + return checkIntegerSystemVar(name, value, 0, 255, vars) + case MaxUserConnections: + return checkIntegerSystemVar(name, value, 0, 4294967295, vars) case OldPasswords: val, err := strconv.Atoi(value) if err != nil { @@ -278,19 +258,6 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) return "2", nil } - case MaxUserConnections: - val, err := strconv.ParseUint(value, 10, 64) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 0 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "0", nil - } - if val > 4294967295 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "4294967295", nil - } case SessionTrackGtids: if strings.EqualFold(value, "OFF") || value == "0" { return "OFF", nil @@ -300,6 +267,18 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return "ALL_GTIDS", nil } return value, ErrWrongValueForVar.GenByArgs(name, value) + case SQLSelectLimit: + val, err := strconv.ParseInt(value, 10, 64) + if err != nil { + return value, ErrWrongTypeForVar.GenByArgs(name) + } + if val < 0 { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return "0", nil + } + return value, nil + case TableDefinitionCache: + case TimeZone: if strings.EqualFold(value, "SYSTEM") { return "SYSTEM", nil From f0f020830ead95a4fbb6e15b09db42ea806687a3 Mon Sep 17 00:00:00 2001 From: spongedc Date: Tue, 31 Jul 2018 01:01:14 +0800 Subject: [PATCH 2/9] Code refine --- executor/set_test.go | 18 ++++++++++++++++++ sessionctx/variable/sysvar.go | 2 +- sessionctx/variable/varsutil.go | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index 9f6cf4947f300..edd1dc1d61999 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -367,4 +367,22 @@ func (s *testSuite) TestValidateSetVar(c *C) { _, err = tk.Exec("set @@max_sort_length='hello'") c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + tk.MustExec("set @@global.table_definition_cache=399") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect table_definition_cache value: '399'")) + result = tk.MustQuery("select @@global.table_definition_cache;") + result.Check(testkit.Rows("400")) + + tk.MustExec("set @@global.table_definition_cache=-1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect table_definition_cache value: '-1'")) + result = tk.MustQuery("select @@global.table_definition_cache;") + result.Check(testkit.Rows("400")) + + tk.MustExec("set @@max_sort_length=524289") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_sort_length value: '8388608'")) + result = tk.MustQuery("select @@max_sort_length;") + result.Check(testkit.Rows("524288")) + + _, err = tk.Exec("set @@max_sort_length='hello'") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) } diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 0fe134afe5ed1..9dce02d98b9a4 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -145,7 +145,7 @@ var defaultSysVars = []*SysVar{ {ScopeGlobal, "log_backward_compatible_user_definitions", ""}, {ScopeNone, "lc_messages_dir", "/usr/local/mysql-5.6.25-osx10.8-x86_64/share/"}, {ScopeGlobal, "ft_boolean_syntax", "+ -><()~*:\"\"&|"}, - {ScopeGlobal, "table_definition_cache", "-1"}, + {ScopeGlobal, TableDefinitionCache, "-1"}, {ScopeNone, SkipNameResolve, "0"}, {ScopeNone, "performance_schema_max_file_handles", "32768"}, {ScopeSession, "transaction_allow_batching", ""}, diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 896dbbcd31c8d..4c5052f18cf06 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -278,7 +278,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, } return value, nil case TableDefinitionCache: - + return checkIntegerSystemVar(name, value, 400, 524288, vars) case TimeZone: if strings.EqualFold(value, "SYSTEM") { return "SYSTEM", nil From 261495a4e0ae1874f54d67b0b58f6f045b7a5f3b Mon Sep 17 00:00:00 2001 From: spongedc Date: Tue, 31 Jul 2018 07:55:08 +0800 Subject: [PATCH 3/9] Fix tests --- executor/set_test.go | 21 +++++++++++++++++---- sessionctx/variable/varsutil.go | 31 ++++++++++--------------------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index edd1dc1d61999..150799ff66edf 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -378,11 +378,24 @@ func (s *testSuite) TestValidateSetVar(c *C) { result = tk.MustQuery("select @@global.table_definition_cache;") result.Check(testkit.Rows("400")) - tk.MustExec("set @@max_sort_length=524289") - tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_sort_length value: '8388608'")) - result = tk.MustQuery("select @@max_sort_length;") + tk.MustExec("set @@global.table_definition_cache=524289") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect table_definition_cache value: '524289'")) + result = tk.MustQuery("select @@global.table_definition_cache;") result.Check(testkit.Rows("524288")) - _, err = tk.Exec("set @@max_sort_length='hello'") + _, err = tk.Exec("set @@global.table_definition_cache='hello'") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + tk.MustExec("set @@old_passwords=-1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect old_passwords value: '-1'")) + result = tk.MustQuery("select @@old_passwords;") + result.Check(testkit.Rows("0")) + + tk.MustExec("set @@old_passwords=3") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect old_passwords value: '3'")) + result = tk.MustQuery("select @@old_passwords;") + result.Check(testkit.Rows("2")) + + _, err = tk.Exec("set @@old_passwords='hello'") c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) } diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 4c5052f18cf06..87a6f4ef82b0d 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -162,32 +162,32 @@ func ValidateGetSystemVar(name string, isGlobal bool) error { return nil } -func checkIntegerSystemVar(name, value string, lower int64, upper uint64, vars *SessionVars) (string, error) { +func checkIntegerSystemVar(name, value string, min int64, max uint64, vars *SessionVars) (string, error) { if value[0] == '-' { val, err := strconv.ParseInt(value, 10, 64) if err != nil { return value, ErrWrongTypeForVar.GenByArgs(name) } - if val < lower { + if val < min { vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", lower), nil + return fmt.Sprintf("%d", min), nil } - if val > 0 && uint64(val) > upper { + if val > 0 && uint64(val) > max { vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", upper), nil + return fmt.Sprintf("%d", max), nil } } else { val, err := strconv.ParseUint(value, 10, 64) if err != nil { return value, ErrWrongTypeForVar.GenByArgs(name) } - if val < uint64(lower) { + if val < uint64(min) { vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", lower), nil + return fmt.Sprintf("%d", min), nil } - if val > 0 && val > upper { + if val > 0 && val > max { vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", upper), nil + return fmt.Sprintf("%d", max), nil } } return value, nil @@ -246,18 +246,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, case MaxUserConnections: return checkIntegerSystemVar(name, value, 0, 4294967295, vars) case OldPasswords: - val, err := strconv.Atoi(value) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 0 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "0", nil - } - if val > 2 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "2", nil - } + return checkIntegerSystemVar(name, value, 0, 2, vars) case SessionTrackGtids: if strings.EqualFold(value, "OFF") || value == "0" { return "OFF", nil From b6eef7f20fa130ba2a4edb7837299e51367f2cc6 Mon Sep 17 00:00:00 2001 From: spongedc Date: Tue, 31 Jul 2018 10:00:35 +0800 Subject: [PATCH 4/9] Add more check --- executor/set_test.go | 39 +++++++++++++++++++++++++++++++++ sessionctx/variable/sysvar.go | 8 +++++-- sessionctx/variable/varsutil.go | 4 ++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index 150799ff66edf..1f451736fc926 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -398,4 +398,43 @@ func (s *testSuite) TestValidateSetVar(c *C) { _, err = tk.Exec("set @@old_passwords='hello'") c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + tk.MustExec("set @@tmp_table_size=-1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect tmp_table_size value: '-1'")) + result = tk.MustQuery("select @@tmp_table_size;") + result.Check(testkit.Rows("1024")) + + tk.MustExec("set @@tmp_table_size=1020") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect tmp_table_size value: '1020'")) + result = tk.MustQuery("select @@tmp_table_size;") + result.Check(testkit.Rows("1024")) + + tk.MustExec("set @@tmp_table_size=167772161") + result = tk.MustQuery("select @@tmp_table_size;") + result.Check(testkit.Rows("167772161")) + + tk.MustExec("set @@tmp_table_size=18446744073709551615") + result = tk.MustQuery("select @@tmp_table_size;") + result.Check(testkit.Rows("18446744073709551615")) + + _, err = tk.Exec("set @@tmp_table_size=18446744073709551616") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + _, err = tk.Exec("set @@tmp_table_size='hello'") + c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue) + + tk.MustExec("set @@global.connect_timeout=1") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect connect_timeout value: '1'")) + result = tk.MustQuery("select @@global.connect_timeout;") + result.Check(testkit.Rows("2")) + + tk.MustExec("set @@global.connect_timeout=31536000") + result = tk.MustQuery("select @@global.connect_timeout;") + result.Check(testkit.Rows("31536000")) + + tk.MustExec("set @@global.connect_timeout=31536001") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect connect_timeout value: '31536001'")) + result = tk.MustQuery("select @@global.connect_timeout;") + result.Check(testkit.Rows("31536000")) + } diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 9dce02d98b9a4..0f329f0db96fb 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -153,7 +153,7 @@ var defaultSysVars = []*SysVar{ {ScopeNone, "performance_schema_max_statement_classes", "168"}, {ScopeGlobal, "server_id", "0"}, {ScopeGlobal, "innodb_flushing_avg_loops", "30"}, - {ScopeGlobal | ScopeSession, "tmp_table_size", "16777216"}, + {ScopeGlobal | ScopeSession, TmpTableSize, "16777216"}, {ScopeGlobal, "innodb_max_purge_lag", "0"}, {ScopeGlobal | ScopeSession, "preload_buffer_size", "32768"}, {ScopeGlobal, "slave_checkpoint_period", "300"}, @@ -162,7 +162,7 @@ var defaultSysVars = []*SysVar{ {ScopeGlobal, "innodb_flush_log_at_timeout", "1"}, {ScopeGlobal, "innodb_max_undo_log_size", ""}, {ScopeGlobal | ScopeSession, "range_alloc_block_size", "4096"}, - {ScopeGlobal, "connect_timeout", "10"}, + {ScopeGlobal, ConnectTimeout, "10"}, {ScopeGlobal | ScopeSession, "collation_server", "latin1_swedish_ci"}, {ScopeNone, "have_rtree_keys", "YES"}, {ScopeGlobal, "innodb_old_blocks_pct", "37"}, @@ -750,6 +750,10 @@ const ( MaxConnectErrors = "max_connect_errors" // TableDefinitionCache is the name for 'table_definition_cache' system variable. TableDefinitionCache = "table_definition_cache" + // TmpTableSize is the name for 'tmp_table_size' system variable. + TmpTableSize = "tmp_table_size" + // ConnectTimeout is the name for 'connect_timeout' system variable. + ConnectTimeout = "connect_timeout" ) // GlobalVarAccessor is the interface for accessing global scope system and status variables. diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 87a6f4ef82b0d..cd4d25b5805ab 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -202,6 +202,8 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return value, UnknownSystemVar.GenByArgs(name) } switch name { + case ConnectTimeout: + return checkIntegerSystemVar(name, value, 2, 31536000, vars) case DefaultWeekFormat: return checkIntegerSystemVar(name, value, 0, 7, vars) case DelayKeyWrite: @@ -268,6 +270,8 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return value, nil case TableDefinitionCache: return checkIntegerSystemVar(name, value, 400, 524288, vars) + case TmpTableSize: + return checkIntegerSystemVar(name, value, 1024, 18446744073709551615, vars) case TimeZone: if strings.EqualFold(value, "SYSTEM") { return "SYSTEM", nil From 4c9ff911dbaab55a9780ad16c0928b7add5a7905 Mon Sep 17 00:00:00 2001 From: spongedc Date: Thu, 9 Aug 2018 22:01:22 +0800 Subject: [PATCH 5/9] Code format refine --- sessionctx/variable/varsutil.go | 77 ++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index bc03e75f820a1..a911d9233ddf5 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -162,33 +162,42 @@ func ValidateGetSystemVar(name string, isGlobal bool) error { return nil } -func checkIntegerSystemVar(name, value string, min int64, max uint64, vars *SessionVars) (string, error) { +func checkUInt64SystemVar(name, value string, min, max uint64, vars *SessionVars) (string, error) { if value[0] == '-' { - val, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < min { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", min), nil - } - if val > 0 && uint64(val) > max { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", max), nil - } - } else { - val, err := strconv.ParseUint(value, 10, 64) + _, err := strconv.ParseInt(value, 10, 64) if err != nil { return value, ErrWrongTypeForVar.GenByArgs(name) } - if val < uint64(min) { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", min), nil - } - if val > 0 && val > max { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return fmt.Sprintf("%d", max), nil - } + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", min), nil + } + val, err := strconv.ParseUint(value, 10, 64) + if err != nil { + return value, ErrWrongTypeForVar.GenByArgs(name) + } + if val < min { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", min), nil + } + if val > max { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", max), nil + } + return value, nil +} + +func checkInt64SystemVar(name, value string, min, max int64, vars *SessionVars) (string, error) { + val, err := strconv.ParseInt(value, 10, 64) + if err != nil { + return value, ErrWrongTypeForVar.GenByArgs(name) + } + if val < min { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", min), nil + } + if val > max { + vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) + return fmt.Sprintf("%d", max), nil } return value, nil } @@ -203,9 +212,9 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, } switch name { case ConnectTimeout: - return checkIntegerSystemVar(name, value, 2, 31536000, vars) + return checkUInt64SystemVar(name, value, 2, 31536000, vars) case DefaultWeekFormat: - return checkIntegerSystemVar(name, value, 0, 7, vars) + return checkUInt64SystemVar(name, value, 0, 7, vars) case DelayKeyWrite: if strings.EqualFold(value, "ON") || value == "1" { return "ON", nil @@ -227,7 +236,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, case GroupConcatMaxLen: // The reasonable range of 'group_concat_max_len' is 4~18446744073709551615(64-bit platforms) // See https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_group_concat_max_len for details - return checkIntegerSystemVar(name, value, 4, 18446744073709551615, vars) + return checkUInt64SystemVar(name, value, 4, 18446744073709551615, vars) case InteractiveTimeout: val, err := strconv.Atoi(value) if err != nil { @@ -238,17 +247,17 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return "1", nil } case MaxConnections: - return checkIntegerSystemVar(name, value, 1, 100000, vars) + return checkUInt64SystemVar(name, value, 1, 100000, vars) case MaxConnectErrors: - return checkIntegerSystemVar(name, value, 1, 18446744073709551615, vars) + return checkUInt64SystemVar(name, value, 1, 18446744073709551615, vars) case MaxSortLength: - return checkIntegerSystemVar(name, value, 4, 8388608, vars) + return checkUInt64SystemVar(name, value, 4, 8388608, vars) case MaxSpRecursionDepth: - return checkIntegerSystemVar(name, value, 0, 255, vars) + return checkUInt64SystemVar(name, value, 0, 255, vars) case MaxUserConnections: - return checkIntegerSystemVar(name, value, 0, 4294967295, vars) + return checkUInt64SystemVar(name, value, 0, 4294967295, vars) case OldPasswords: - return checkIntegerSystemVar(name, value, 0, 2, vars) + return checkUInt64SystemVar(name, value, 0, 2, vars) case SessionTrackGtids: if strings.EqualFold(value, "OFF") || value == "0" { return "OFF", nil @@ -269,9 +278,9 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, } return value, nil case TableDefinitionCache: - return checkIntegerSystemVar(name, value, 400, 524288, vars) + return checkUInt64SystemVar(name, value, 400, 524288, vars) case TmpTableSize: - return checkIntegerSystemVar(name, value, 1024, 18446744073709551615, vars) + return checkUInt64SystemVar(name, value, 1024, 18446744073709551615, vars) case TimeZone: if strings.EqualFold(value, "SYSTEM") { return "SYSTEM", nil From bcadbc00381d00948000e1d54cf0250ccdc283f1 Mon Sep 17 00:00:00 2001 From: spongedc Date: Fri, 10 Aug 2018 11:08:18 +0800 Subject: [PATCH 6/9] bug fix --- executor/set_test.go | 11 ++++++++++ sessionctx/variable/varsutil.go | 39 +++++++++------------------------ 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index 4b49b96f5a940..ac5d9e9039231 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -431,4 +431,15 @@ func (s *testSuite) TestValidateSetVar(c *C) { result = tk.MustQuery("select @@global.connect_timeout;") result.Check(testkit.Rows("31536000")) + result = tk.MustQuery("select @@sql_select_limit;") + result.Check(testkit.Rows("18446744073709551615")) + tk.MustExec("set @@sql_select_limit=default") + result = tk.MustQuery("select @@sql_select_limit;") + result.Check(testkit.Rows("18446744073709551615")) + + tk.MustExec("set @@global.flush_time=31536001") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect flush_time value: '31536001'")) + + tk.MustExec("set @@global.interactive_timeout=31536001") + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect interactive_timeout value: '31536001'")) } diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index a911d9233ddf5..5685a52e7c4ef 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -25,8 +25,11 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/types" + "math" ) +const secondsPerYear = 31536000 + // SetDDLReorgWorkerCounter sets ddlReorgWorkerCounter count. // Max worker count is maxDDLReorgWorkerCount. func SetDDLReorgWorkerCounter(cnt int32) { @@ -212,7 +215,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, } switch name { case ConnectTimeout: - return checkUInt64SystemVar(name, value, 2, 31536000, vars) + return checkUInt64SystemVar(name, value, 2, secondsPerYear, vars) case DefaultWeekFormat: return checkUInt64SystemVar(name, value, 0, 7, vars) case DelayKeyWrite: @@ -225,31 +228,17 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, } return value, ErrWrongValueForVar.GenByArgs(name, value) case FlushTime: - val, err := strconv.Atoi(value) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 0 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "0", nil - } + return checkUInt64SystemVar(name, value, 0, secondsPerYear, vars) case GroupConcatMaxLen: // The reasonable range of 'group_concat_max_len' is 4~18446744073709551615(64-bit platforms) // See https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_group_concat_max_len for details - return checkUInt64SystemVar(name, value, 4, 18446744073709551615, vars) + return checkUInt64SystemVar(name, value, 4, math.MaxUint64, vars) case InteractiveTimeout: - val, err := strconv.Atoi(value) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 1 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "1", nil - } + return checkUInt64SystemVar(name, value, 1, secondsPerYear, vars) case MaxConnections: return checkUInt64SystemVar(name, value, 1, 100000, vars) case MaxConnectErrors: - return checkUInt64SystemVar(name, value, 1, 18446744073709551615, vars) + return checkUInt64SystemVar(name, value, 1, math.MaxUint64, vars) case MaxSortLength: return checkUInt64SystemVar(name, value, 4, 8388608, vars) case MaxSpRecursionDepth: @@ -268,19 +257,11 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, } return value, ErrWrongValueForVar.GenByArgs(name, value) case SQLSelectLimit: - val, err := strconv.ParseInt(value, 10, 64) - if err != nil { - return value, ErrWrongTypeForVar.GenByArgs(name) - } - if val < 0 { - vars.StmtCtx.AppendWarning(ErrTruncatedWrongValue.GenByArgs(name, value)) - return "0", nil - } - return value, nil + return checkUInt64SystemVar(name, value, 0, math.MaxUint64, vars) case TableDefinitionCache: return checkUInt64SystemVar(name, value, 400, 524288, vars) case TmpTableSize: - return checkUInt64SystemVar(name, value, 1024, 18446744073709551615, vars) + return checkUInt64SystemVar(name, value, 1024, math.MaxUint64, vars) case TimeZone: if strings.EqualFold(value, "SYSTEM") { return "SYSTEM", nil From 927743f573d957f92cd05256d0f526ff839a34d8 Mon Sep 17 00:00:00 2001 From: spongedc Date: Fri, 10 Aug 2018 11:58:49 +0800 Subject: [PATCH 7/9] Code fmt refine --- sessionctx/variable/varsutil.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 5685a52e7c4ef..1aaa36d228c0d 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -16,6 +16,7 @@ package variable import ( "encoding/json" "fmt" + "math" "strconv" "strings" "sync/atomic" @@ -25,7 +26,6 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/types" - "math" ) const secondsPerYear = 31536000 From cda9912472476eefacbbd64c8db79683f3ce802d Mon Sep 17 00:00:00 2001 From: spongedc Date: Wed, 15 Aug 2018 14:31:35 +0800 Subject: [PATCH 8/9] Add comments for test cases Signed-off-by: spongedc --- executor/set_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/executor/set_test.go b/executor/set_test.go index d8bfdfe17b2fc..2203248ddf51d 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -309,6 +309,8 @@ func (s *testSuite) TestValidateSetVar(c *C) { result = tk.MustQuery("select @@time_zone;") result.Check(testkit.Rows("SYSTEM")) + // The following cases test value out of range and illegal type when setting system variables. + // See https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html for more details. tk.MustExec("set @@global.max_connections=100001") tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect max_connections value: '100001'")) result = tk.MustQuery("select @@global.max_connections;") From af3effc0d2f02d28dd84ec5c4e4932d11d1a192b Mon Sep 17 00:00:00 2001 From: spongedc Date: Thu, 16 Aug 2018 14:45:08 +0800 Subject: [PATCH 9/9] Code refine --- sessionctx/variable/varsutil.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 4987be1103172..54a11fe60edfa 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -28,7 +28,8 @@ import ( "github.com/pingcap/tidb/types" ) -const secondsPerYear = 31536000 +// secondsPerYear represents seconds in a normal year. Leap year is not considered here. +const secondsPerYear = 60 * 60 * 24 * 365 // SetDDLReorgWorkerCounter sets ddlReorgWorkerCounter count. // Max worker count is maxDDLReorgWorkerCount.