bzr commit into mysql-5.1 branch (azundris:2722) Bug#40657

[Tatiana A. Nurnberg]

#At file:///misc/mysql/forest/40657/51-40657/ based on revid:azundris@stripped

 2722 Tatiana A. Nurnberg	2009-02-26
      Bug#40657: assertion with out of range variables and traditional sql_mode
      
      In STRICT mode, out-of-bounds values caused an error message
      to be queued (rather than just a warning), without any further
      error-like processing happening. (The error is queued during
      update, at which time it's too late. For it to be processed
      properly, it would need to be queued during check-stage.)
      The assertion rightfully complains that we're trying to send
      an OK while having an error queued.
      
      Changeset breaks a lot of tests out into check-stage. This also
      allows us to send more correct warnings/error messages.
     @ sql/set_var.cc
        cleanup: fold get_unsigned() and fix_unsigned() into one,
        as well as all the semi-common code from the ::check
        functions.

    modified:
      sql/set_var.cc
=== modified file 'sql/set_var.cc'
--- a/sql/set_var.cc	2009-02-24 15:42:18 +0000
+++ b/sql/set_var.cc	2009-02-26 16:36:49 +0000
@@ -1405,22 +1405,38 @@ bool throw_bounds_warning(THD *thd, bool
 /**
   check an unsigned user-supplied value for a systemvariable against bounds.
 
+  TODO: This is a wrapper function to call clipping from within an update()
+        function.  Calling bounds from within update() is fair game in theory,
+        but we can only send warnings from in there, not errors, and besides,
+        it violates our model of separating check from update phase.
+        To avoid breaking out of the server with an ASSERT() in strict mode,
+        we pretend we're not in strict mode when we go through here. Bug#43233
+        was opened to remind us to replace this kludge with The Right Thing,
+        which of course is to do the check in the actual check phase, and then
+        throw an error or warning accordingly.
+
   @param thd             thread handle
   @param num             the value to limit
-  @param option_limits   the bounds-record, or NULL
-
-  @retval                whether or not we needed to bound
+  @param option_limits   the bounds-record, or NULL if none
  */
-static my_bool bound_unsigned(THD *thd, ulonglong *num,
+static void bound_unsigned(THD *thd, ulonglong *num,
                               const struct my_option *option_limits)
 {
-  my_bool   fixed     = FALSE;
-  ulonglong unadjusted= *num;
-
   if (option_limits)
+  {
+    my_bool   fixed     = FALSE;
+    ulonglong unadjusted= *num;
+
     *num= getopt_ull_limit_value(unadjusted, option_limits, &fixed);
 
-  return fixed;
+    if (fixed)
+    {
+      ulong ssm= thd->variables.sql_mode;
+      thd->variables.sql_mode&= ~MODE_STRICT_ALL_TABLES;
+      throw_bounds_warning(thd, fixed, TRUE, option_limits->name, unadjusted);
+      thd->variables.sql_mode= ssm;
+    }
+  }
 }
 
 
@@ -1445,6 +1461,7 @@ static bool get_unsigned(THD *thd, set_v
   int                     warnings= 0;
   ulonglong               unadjusted;
   const struct my_option *limits= var->var->option_limits;
+  struct my_option        fallback;
 
   /* get_unsigned() */
   if (var->value->unsigned_flag)
@@ -1477,6 +1494,20 @@ static bool get_unsigned(THD *thd, set_v
     warnings++;
   }
 
+  /*
+    if the sysvar doesn't have a proper bounds record but the check
+    function would like bounding to ULONG where its size differs from
+    that of ULONGLONG, we make up a bogus limits record here and let
+    the usual suspects handle the actual limiting.
+  */
+
+  if (!limits && bound2ulong)
+  {
+    bzero(&fallback, sizeof(fallback));
+    fallback.var_type= GET_ULONG;
+    limits= &fallback;
+  }
+
   /* fix_unsigned() */
   if (limits)
   {
@@ -1489,20 +1520,6 @@ static bool get_unsigned(THD *thd, set_v
                                                 (longlong) unadjusted))
       return TRUE;
   }
-  else if (bound2ulong)
-  {
-#if SIZEOF_LONG < SIZEOF_LONG_LONG
-    /* Avoid overflows on 32 bit systems */
-    if (var->save_result.ulonglong_value > ULONG_MAX)
-    {
-      var->save_result.ulonglong_value= ULONG_MAX;
-      if ((warnings == 0) && throw_bounds_warning(thd, TRUE, TRUE,
-                                                  var->var->name,
-                                                  (longlong) unadjusted))
-        return TRUE;
-    }
-#endif
-  }
 
   return FALSE;
 }
@@ -2334,7 +2351,7 @@ bool sys_var_key_buffer_size::update(THD
   bound_unsigned(thd, &tmp, option_limits);
   key_cache->param_buff_size= (ulonglong) tmp;
 
-  /* If key cache didn't existed initialize it, else resize it */
+  /* If key cache didn't exist initialize it, else resize it */
   key_cache->in_init= 1;
   pthread_mutex_unlock(&LOCK_global_system_variables);
 


Attachment: [text/bzr-bundle] bzr/[e-mail]