Bug#43233: Some server variables are clipped during "update," not "check" stage

Bug#55794: ulonglong options of mysqld show wrong values.

Port the few remaining system variables to the correct mechanism --
range-check in check-stage (and throw error or warning at that point
as needed and depending on STRICTness), update in update stage.
Fix some signedness errors when retrieving sysvar values for display.

mysql-test/r/variables.result

@@ -1511,4 +1511,38 @@ SELECT @@skip_name_resolve;
 SHOW VARIABLES LIKE 'skip_name_resolve';
 Variable_name	Value
 skip_name_resolve	OFF
+#
+# Bug #43233 : Some server variables are clipped during "update,"
+#              not "check" stage
+#
+SET @kbs=@@global.key_buffer_size;
+SET @kcbs=@@global.key_cache_block_size;
+throw errors in STRICT mode
+SET SQL_MODE=STRICT_ALL_TABLES;
+SET @@global.max_binlog_cache_size=-1;
+ERROR 42000: Variable 'max_binlog_cache_size' can't be set to the value of '-1'
+SET @@global.max_join_size=0;
+ERROR 42000: Variable 'max_join_size' can't be set to the value of '0'
+SET @@global.key_buffer_size=0;
+ERROR 42000: Variable 'key_buffer_size' can't be set to the value of '0'
+SET @@global.key_cache_block_size=0;
+ERROR 42000: Variable 'key_cache_block_size' can't be set to the value of '0'
+throw warnings in default mode
+SET SQL_MODE=DEFAULT;
+SET @@global.max_binlog_cache_size=-1;
+Warnings:
+Warning	1292	Truncated incorrect max_binlog_cache_size value: '-1'
+SET @@global.max_join_size=0;
+Warnings:
+Warning	1292	Truncated incorrect max_join_size value: '0'
+SET @@global.key_buffer_size=0;
+Warnings:
+Warning	1292	Truncated incorrect key_buffer_size value: '0'
+SET @@global.key_cache_block_size=0;
+Warnings:
+Warning	1292	Truncated incorrect key_cache_block_size value: '0'
+SET @@global.max_binlog_cache_size=DEFAULT;
+SET @@global.max_join_size=DEFAULT;
+SET @@global.key_buffer_size=@kbs;
+SET @@global.key_cache_block_size=@kcbs;
 End of 5.1 tests

mysql-test/t/variables.test

@@ -1255,4 +1255,45 @@ SET GLOBAL max_binlog_cache_size = @old_max_binlog_cache_size;
 SELECT @@skip_name_resolve;
 SHOW VARIABLES LIKE 'skip_name_resolve';
 
+--echo #
+--echo # Bug #43233 : Some server variables are clipped during "update,"
+--echo #              not "check" stage
+--echo #
+
+SET @kbs=@@global.key_buffer_size;
+SET @kcbs=@@global.key_cache_block_size;
+
+--echo throw errors in STRICT mode
+SET SQL_MODE=STRICT_ALL_TABLES;
+
+# sys_var_ulonglong_ptr: sys_max_binlog_cache_size
+--error ER_WRONG_VALUE_FOR_VAR
+SET @@global.max_binlog_cache_size=-1;
+
+# sys_var_thd_ha_rows: "max_join_size" et al.
+--error ER_WRONG_VALUE_FOR_VAR
+SET @@global.max_join_size=0;
+
+# sys_var_key_buffer_size: "key_buffer_size"
+--error ER_WRONG_VALUE_FOR_VAR
+SET @@global.key_buffer_size=0;
+
+# sys_var_key_cache_long: "key_cache_block_size" et al.
+--error ER_WRONG_VALUE_FOR_VAR
+SET @@global.key_cache_block_size=0;
+
+--echo throw warnings in default mode
+SET SQL_MODE=DEFAULT;
+
+SET @@global.max_binlog_cache_size=-1;
+SET @@global.max_join_size=0;
+SET @@global.key_buffer_size=0;
+SET @@global.key_cache_block_size=0;
+
+# cleanup
+SET @@global.max_binlog_cache_size=DEFAULT;
+SET @@global.max_join_size=DEFAULT;
+SET @@global.key_buffer_size=@kbs;
+SET @@global.key_cache_block_size=@kcbs;
+
 --echo End of 5.1 tests

sql/item_func.cc

@@ -4843,7 +4843,7 @@ void Item_func_get_system_var::fix_length_and_dec()
       decimals=0;
       break;
     case SHOW_LONGLONG:
-      unsigned_flag= FALSE;
+      unsigned_flag= TRUE;
       max_length= MY_INT64_NUM_DECIMAL_DIGITS;
       decimals=0;
       break;
@@ -4984,7 +4984,7 @@ longlong Item_func_get_system_var::val_int()
   {
     case SHOW_INT:      get_sys_var_safe (uint);
     case SHOW_LONG:     get_sys_var_safe (ulong);
-    case SHOW_LONGLONG: get_sys_var_safe (longlong);
+    case SHOW_LONGLONG: get_sys_var_safe (ulonglong);
     case SHOW_HA_ROWS:  get_sys_var_safe (ha_rows);
     case SHOW_BOOL:     get_sys_var_safe (bool);
     case SHOW_MY_BOOL:  get_sys_var_safe (my_bool);

sql/set_var.cc

@@ -1579,11 +1579,16 @@ void sys_var_long_ptr_global::set_default(THD *thd, enum_var_type type)
 }
 
 
+bool sys_var_ulonglong_ptr::check(THD *thd, set_var *var)
+{
+  return get_unsigned(thd, var, 0, GET_ULL);
+}
+
+
 bool sys_var_ulonglong_ptr::update(THD *thd, set_var *var)
 {
   ulonglong tmp= var->save_result.ulonglong_value;
   pthread_mutex_lock(&LOCK_global_system_variables);
-  bound_unsigned(thd, &tmp, option_limits);
   *value= (ulonglong) tmp;
   pthread_mutex_unlock(&LOCK_global_system_variables);
   return 0;
@@ -1675,25 +1680,30 @@ uchar *sys_var_thd_ulong::value_ptr(THD *thd, enum_var_type type,
 }
 
 
-bool sys_var_thd_ha_rows::update(THD *thd, set_var *var)
+bool sys_var_thd_ha_rows::check(THD *thd, set_var *var)
 {
-  ulonglong tmp= var->save_result.ulonglong_value;
-
-  /* Don't use bigger value than given with --maximum-variable-name=.. */
-  if ((ha_rows) tmp > max_system_variables.*offset)
-    tmp= max_system_variables.*offset;
+  return get_unsigned(thd, var, max_system_variables.*offset,
+#ifdef BIG_TABLES
+                      GET_ULL
+#else
+                      GET_ULONG
+#endif
+                     );
+}
 
-  bound_unsigned(thd, &tmp, option_limits);
 
+bool sys_var_thd_ha_rows::update(THD *thd, set_var *var)
+{
   if (var->type == OPT_GLOBAL)
   {
     /* Lock is needed to make things safe on 32 bit systems */
-    pthread_mutex_lock(&LOCK_global_system_variables);    
-    global_system_variables.*offset= (ha_rows) tmp;
+    pthread_mutex_lock(&LOCK_global_system_variables);
+    global_system_variables.*offset= (ha_rows)
+                                     var->save_result.ulonglong_value;
     pthread_mutex_unlock(&LOCK_global_system_variables);
   }
   else
-    thd->variables.*offset= (ha_rows) tmp;
+    thd->variables.*offset= (ha_rows) var->save_result.ulonglong_value;
   return 0;
 }
 
@@ -2305,6 +2315,12 @@ uchar *sys_var_key_cache_param::value_ptr(THD *thd, enum_var_type type,
 }
 
 
+bool sys_var_key_buffer_size::check(THD *thd, set_var *var)
+{
+  return get_unsigned(thd, var, 0, GET_ULL);
+}
+
+
 bool sys_var_key_buffer_size::update(THD *thd, set_var *var)
 {
   ulonglong tmp= var->save_result.ulonglong_value;
@@ -2318,10 +2334,10 @@ bool sys_var_key_buffer_size::update(THD *thd, set_var *var)
 
   pthread_mutex_lock(&LOCK_global_system_variables);
   key_cache= get_key_cache(base_name);
-                            
+
   if (!key_cache)
   {
-    /* Key cache didn't exists */
+    /* Key cache didn't exist */
     if (!tmp)					// Tried to delete cache
       goto end;					// Ok, nothing to do
     if (!(key_cache= create_key_cache(base_name->str, base_name->length)))
@@ -2371,7 +2387,6 @@ bool sys_var_key_buffer_size::update(THD *thd, set_var *var)
     goto end;
   }
 
-  bound_unsigned(thd, &tmp, option_limits);
   key_cache->param_buff_size= (ulonglong) tmp;
 
   /* If key cache didn't exist initialize it, else resize it */
@@ -2388,10 +2403,19 @@ bool sys_var_key_buffer_size::update(THD *thd, set_var *var)
 
 end:
   pthread_mutex_unlock(&LOCK_global_system_variables);
+
+ var->save_result.ulonglong_value = SIZE_T_MAX;
+
   return error;
 }
 
 
+bool sys_var_key_cache_long::check(THD *thd, set_var *var)
+{
+  return get_unsigned(thd, var, 0, GET_ULONG);
+}
+
+
 /**
   @todo
   Abort if some other thread is changing the key cache.
@@ -2400,7 +2424,6 @@ end:
 */
 bool sys_var_key_cache_long::update(THD *thd, set_var *var)
 {
-  ulonglong tmp= var->value->val_int();
   LEX_STRING *base_name= &var->base;
   bool error= 0;
 
@@ -2425,8 +2448,8 @@ bool sys_var_key_cache_long::update(THD *thd, set_var *var)
   if (key_cache->in_init)
     goto end;
 
-  bound_unsigned(thd, &tmp, option_limits);
-  *((ulong*) (((char*) key_cache) + offset))= (ulong) tmp;
+  *((ulong*) (((char*) key_cache) + offset))= (ulong)
+                                              var->save_result.ulonglong_value;
 
   /*
     Don't create a new key cache if it didn't exist

sql/set_var.h

@@ -196,6 +196,7 @@ public:
 		       sys_after_update_func func)
     :sys_var(name_arg,func), value(value_ptr_arg)
   { chain_sys_var(chain); }
+  bool check(THD *thd, set_var *var);
   bool update(THD *thd, set_var *var);
   void set_default(THD *thd, enum_var_type type);
   SHOW_TYPE show_type() { return SHOW_LONGLONG; }
@@ -442,6 +443,7 @@ public:
 		      sys_after_update_func func)
     :sys_var_thd(name_arg,func), offset(offset_arg)
   { chain_sys_var(chain); }
+  bool check(THD *thd, set_var *var);
   bool update(THD *thd, set_var *var);
   void set_default(THD *thd, enum_var_type type);
   SHOW_TYPE show_type() { return SHOW_HA_ROWS; }
@@ -854,6 +856,7 @@ public:
     :sys_var_key_cache_param(chain, name_arg,
                              offsetof(KEY_CACHE, param_buff_size))
   {}
+  bool check(THD *thd, set_var *var);
   bool update(THD *thd, set_var *var);
   SHOW_TYPE show_type() { return SHOW_LONGLONG; }
 };
@@ -865,6 +868,7 @@ public:
   sys_var_key_cache_long(sys_var_chain *chain, const char *name_arg, size_t offset_arg)
     :sys_var_key_cache_param(chain, name_arg, offset_arg)
   {}
+  bool check(THD *thd, set_var *var);
   bool update(THD *thd, set_var *var);
   SHOW_TYPE show_type() { return SHOW_LONG; }
 };