Optimized code for setting user variables with := and fixed some bugs in old code (Bug #1194)

Use forced close of socket to make mysqld shutdown faster when used under valgrind


mysql-test/mysql-test-run.sh:
  Added --skip-bdb for valgrind
mysql-test/r/user_var.result:
  Extended test for user variables
mysql-test/t/user_var.test:
  Extended test for user variables
sql/item_func.cc:
  Optimized code for setting user variables with := and fixed some bugs in old code
sql/item_func.h:
  Optimized code for setting user variables
sql/log.cc:
  Fixed comments
sql/mysqld.cc:
  Use forced close of socket to make mysqld shutdown faster when used under valgrind
sql/sql_class.h:
  Optimized code for setting user variables

mysql-test/mysql-test-run.sh

@@ -337,8 +337,8 @@ while test $# -gt 0; do
       ;;
     --valgrind)
       VALGRIND="valgrind --alignment=8 --leak-check=yes --num-callers=16"
-      EXTRA_MASTER_MYSQLD_OPT="$EXTRA_MASTER_MYSQLD_OPT --skip-safemalloc"
-      EXTRA_SLAVE_MYSQLD_OPT="$EXTRA_SLAVE_MYSQLD_OPT --skip-safemalloc"
+      EXTRA_MASTER_MYSQLD_OPT="$EXTRA_MASTER_MYSQLD_OPT --skip-safemalloc --skip-bdb"
+      EXTRA_SLAVE_MYSQLD_OPT="$EXTRA_SLAVE_MYSQLD_OPT --skip-safemalloc --skip-bdb"
       SLEEP_TIME_AFTER_RESTART=10
       SLEEP_TIME_FOR_DELETE=120
       USE_RUNNING_SERVER=""

mysql-test/r/user_var.result

@@ -30,6 +30,7 @@ explain select * from t1 where i=@vv1;
 table	type	possible_keys	key	key_len	ref	rows	Extra
 t1	ref	i	i	4	const	1	Using where
 drop table t1,t2;
+set @a=0,@b=0;
 select @a:=10,   @b:=1,   @a > @b, @a < @b;
 @a:=10	@b:=1	@a > @b	@a < @b
 10	1	1	0
@@ -38,18 +39,18 @@ select @a:="10", @b:="1", @a > @b, @a < @b;
 10	1	1	0
 select @a:=10,   @b:=2,   @a > @b, @a < @b;
 @a:=10	@b:=2	@a > @b	@a < @b
-10	2	1	0
+10	2	0	1
 select @a:="10", @b:="2", @a > @b, @a < @b;
 @a:="10"	@b:="2"	@a > @b	@a < @b
-10	2	0	1
+10	2	1	0
 select @a:=1;
 @a:=1
 1
 select @a, @a:=1;
 @a	@a:=1
 1	1
-create table t1 (id int);
-insert into t1 values (1);
+create table t1 (id int, d double, c char(10));
+insert into t1 values (1,2.0, "test");
 select @c:=0;
 @c:=0
 0
@@ -70,7 +71,29 @@ select @c:=0;
 select @c:=@c+1;
 @c:=@c+1
 1
+select @d,(@d:=id),@d from t1;
+@d	(@d:=id)	@d
+NULL	1	1
+select @e,(@e:=d),@e from t1;
+@e	(@e:=d)	@e
+NULL	2	2
+select @f,(@f:=c),@f from t1;
+@f	(@f:=c)	@f
+NULL	test	test
+set @g=1;
+select @g,(@g:=c),@g from t1;
+@g	(@g:=c)	@g
+1	test	test
+select @c, @d, @e, @f;
+@c	@d	@e	@f
+1	1	2	test
+select @d:=id, @e:=id, @f:=id, @g:=@id from t1;
+@d:=id	@e:=id	@f:=id	@g:=@id
+1	1	1	NULL
+select @c, @d, @e, @f, @g;
+@c	@d	@e	@f	@g
+1	1	1	1	NULL
 drop table t1;
 select @a:=10, @b:=2, @a>@b, @a:="10", @b:="2", @a>@b, @a:=10, @b:=2, @a>@b, @a:="10", @b:="2", @a>@b;
 @a:=10	@b:=2	@a>@b	@a:="10"	@b:="2"	@a>@b	@a:=10	@b:=2	@a>@b	@a:="10"	@b:="2"	@a>@b
-10	2	1	10	2	0	10	2	1	10	2	0
+10	2	1	10	2	1	10	2	1	10	2	1

mysql-test/t/user_var.test

@@ -19,8 +19,11 @@ explain select * from t1 where i=@vv1;
 drop table t1,t2;
 
 # Check types of variables
+set @a=0,@b=0;
 select @a:=10,   @b:=1,   @a > @b, @a < @b;
+# Note that here a and b will be avaluated as number
 select @a:="10", @b:="1", @a > @b, @a < @b;
+# Note that here a and b will be avaluated as strings
 select @a:=10,   @b:=2,   @a > @b, @a < @b;
 select @a:="10", @b:="2", @a > @b, @a < @b;
 
@@ -28,8 +31,8 @@ select @a:="10", @b:="2", @a > @b, @a < @b;
 select @a:=1;
 select @a, @a:=1;
 
-create table t1 (id int);
-insert into t1 values (1);
+create table t1 (id int, d double, c char(10));
+insert into t1 values (1,2.0, "test");
 select @c:=0;
 update t1 SET id=(@c:=@c+1);
 select @c;
@@ -38,7 +41,15 @@ update t1 set id=(@c:=@c+1);
 select @c;
 select @c:=0;
 select @c:=@c+1;
+select @d,(@d:=id),@d from t1;
+select @e,(@e:=d),@e from t1;
+select @f,(@f:=c),@f from t1;
+set @g=1;
+select @g,(@g:=c),@g from t1;
+select @c, @d, @e, @f;
+select @d:=id, @e:=id, @f:=id, @g:=@id from t1;
+select @c, @d, @e, @f, @g;
 drop table t1;
 
-# just fof fun :)
+# just for fun :)
 select @a:=10, @b:=2, @a>@b, @a:="10", @b:="2", @a>@b, @a:=10, @b:=2, @a>@b, @a:="10", @b:="2", @a>@b;

sql/item_func.cc

@@ -1835,8 +1835,8 @@ bool Item_func_set_user_var::fix_fields(THD *thd,TABLE_LIST *tables)
   if (Item_func::fix_fields(thd,tables) ||
       !(entry= get_variable(&thd->user_vars, name, 1)))
     return 1;
-  entry->type= cached_result_type;
-  entry->update_query_id=thd->query_id;
+  entry->update_query_id= thd->query_id;
+  cached_result_type= args[0]->result_type();
   return 0;
 }
 
@@ -1847,10 +1847,10 @@ Item_func_set_user_var::fix_length_and_dec()
   maybe_null=args[0]->maybe_null;
   max_length=args[0]->max_length;
   decimals=args[0]->decimals;
-  cached_result_type=args[0]->result_type();
 }
 
-void Item_func_set_user_var::update_hash(void *ptr, uint length,
+
+bool Item_func_set_user_var::update_hash(const void *ptr, uint length,
 					 Item_result type)
 {
   if ((null_value=args[0]->null_value))
@@ -1863,6 +1863,8 @@ void Item_func_set_user_var::update_hash(void *ptr, uint length,
   }
   else
   {
+    if (type == STRING_RESULT)
+      length++;					// Store strings with end \0
     if (length <= extra_size)
     {
       /* Save value in value struct */
@@ -1887,73 +1889,151 @@ void Item_func_set_user_var::update_hash(void *ptr, uint length,
 	  goto err;
       }
     }
+    if (type == STRING_RESULT)
+    {
+      length--;					// Fix length change above
+      entry->value[length]= 0;			// Store end \0
+    }
     memcpy(entry->value,ptr,length);
     entry->length= length;
     entry->type=type;
   }
-  return;
+  return 0;
 
  err:
   current_thd->fatal_error=1;			// Probably end of memory
   null_value=1;
-  return;
+  return 1;					// Error
 }
 
-bool
-Item_func_set_user_var::update()
+
+/* Get the value of a variable as a double */
+
+double user_var_entry::val(my_bool *null_value)
 {
-  DBUG_ENTER("Item_func_set_user_var::update");
-  switch (cached_result_type) {
-  case REAL_RESULT:   (void)native_val();     break;
-  case INT_RESULT:    (void)native_val_int(); break;
-  case STRING_RESULT: (void)native_val_str(); break;
+  if ((*null_value= (value == 0)))
+    return 0.0;
+
+  switch (type) {
+  case REAL_RESULT:
+    return *(double*) value;
+  case INT_RESULT:
+    return (double) *(longlong*) value;
+  case STRING_RESULT:
+    return atof(value);				// This is null terminated
   }
-  DBUG_RETURN(current_thd->fatal_error);
+  return 0.0;					// Impossible
 }
 
 
-double
-Item_func_set_user_var::val()
+/* Get the value of a variable as an integer */
+
+longlong user_var_entry::val_int(my_bool *null_value)
 {
-  DBUG_ENTER("Item_func_set_user_var::val");
-  switch (cached_result_type) {
-  case REAL_RESULT: return native_val(); break;
-  case INT_RESULT:  return native_val_int(); break;
+  if ((*null_value= (value == 0)))
+    return LL(0);
+
+  switch (type) {
+  case REAL_RESULT:
+    return (longlong) *(double*) value;
+  case INT_RESULT:
+    return *(longlong*) value;
   case STRING_RESULT:
-  {
-    String *res= native_val_str(); 
-    return !res ? 0 : atof(res->c_ptr());
+    return strtoull(value,NULL,10);		// String is null terminated
   }
+  return LL(0);					// Impossible
+}
+
+
+/* Get the value of a variable as a string */
+
+String *user_var_entry::val_str(my_bool *null_value, String *str,
+				uint decimals)
+{
+  if ((*null_value= (value == 0)))
+    return (String*) 0;
+
+  switch (type) {
+  case REAL_RESULT:
+    str->set(*(double*) value, decimals);
+    break;
+  case INT_RESULT:
+    str->set(*(longlong*) value);
+    break;
+  case STRING_RESULT:
+    if (str->copy(value, length))
+      str= 0;					// EOM error
   }
-  DBUG_RETURN(args[0]->val());
+  return(str);
 }
 
-longlong
-Item_func_set_user_var::val_int()
+
+/*
+  This functions is invoked on SET @variable or @variable:= expression.
+
+  SYNOPSIS
+    Item_func_set_user_var::update()
+
+  NOTES
+    We have to store the expression as such in the variable, independent of
+    the value method used by the user
+
+  RETURN
+    0	Ok
+    1	EOM Error
+
+*/
+
+
+bool
+Item_func_set_user_var::update()
 {
-  DBUG_ENTER("Item_func_set_user_var::val_int");
+  bool res;
+  DBUG_ENTER("Item_func_set_user_var::update");
+  LINT_INIT(res);
+
   switch (cached_result_type) {
-  case REAL_RESULT: return (longlong)native_val(); break;
-  case INT_RESULT:  return native_val_int(); break;
-  case STRING_RESULT:
+  case REAL_RESULT:
   {
-    String *res= native_val_str(); 
-    return !res ? 0 : strtoull(res->c_ptr(),NULL,10);
+    double value=args[0]->val();
+    res= update_hash((void*) &value,sizeof(value), REAL_RESULT);
+    break;
   }
+  case INT_RESULT:
+  {
+    longlong value=args[0]->val_int();
+    res= update_hash((void*) &value,sizeof(longlong), INT_RESULT);
+    break;
   }
-  DBUG_RETURN(args[0]->val_int());
+  case STRING_RESULT:
+    String *tmp;
+    tmp=args[0]->val_str(&value);
+    if (!tmp)					// Null value
+      res= update_hash((void*) 0,0,STRING_RESULT);
+    else
+      res= update_hash((void*) tmp->ptr(),tmp->length(),STRING_RESULT);
+    break;
+  }
+  DBUG_RETURN(res);
 }
 
-String *
-Item_func_set_user_var::val_str(String *str)
+
+double Item_func_set_user_var::val()
 {
-  DBUG_ENTER("Item_func_set_user_var::val_str");
-  switch (cached_result_type) {
-  case REAL_RESULT:   str->set(native_val(),decimals); break;
-  case INT_RESULT:    str->set(native_val_int(),decimals); break;
-  case STRING_RESULT: str= native_val_str(str); break;
-  }
-  DBUG_RETURN(str);
+  update();					// Store expression
+  return entry->val(&null_value);
+}
+
+longlong Item_func_set_user_var::val_int()
+{
+  update();					// Store expression
+  return entry->val_int(&null_value);
+}
+
+String *Item_func_set_user_var::val_str(String *str)
+{
+  update();					// Store expression
+  return entry->val_str(&null_value, str, decimals);
 }
 
 
@@ -1967,75 +2047,30 @@ void Item_func_set_user_var::print(String *str)
 }
 
 
-user_var_entry *Item_func_get_user_var::get_entry()
-{
-  if (!var_entry  || ! var_entry->value)
-  {
-    null_value=1;
-    return 0;
-  }
-  null_value=0;
-  return var_entry;
-}
-
 
 String *
 Item_func_get_user_var::val_str(String *str)
 {
   DBUG_ENTER("Item_func_get_user_var::val_str");
-  user_var_entry *entry=get_entry();
-  if (!entry)
-    return NULL;
-  switch (entry->type) {
-  case REAL_RESULT:
-    str->set(*(double*) entry->value,decimals);
-    break;
-  case INT_RESULT:
-    str->set(*(longlong*) entry->value);
-    break;
-  case STRING_RESULT:
-    if (str->copy(entry->value, entry->length-1))
-    {
-      null_value=1;
-      return NULL;
-    }
-    break;
-  }
-  DBUG_RETURN(str);
+  if (!var_entry)
+    return (String*) 0;				// No such variable
+  DBUG_RETURN(var_entry->val_str(&null_value, str, decimals));
 }
 
 
 double Item_func_get_user_var::val()
 {
-  user_var_entry *entry=get_entry();
-  if (!entry)
-    return 0.0;
-  switch (entry->type) {
-  case REAL_RESULT:
-    return *(double*) entry->value;
-  case INT_RESULT:
-    return (double) *(longlong*) entry->value;
-  case STRING_RESULT:
-    return atof(entry->value);			// This is null terminated
-  }
-  return 0.0;					// Impossible
+  if (!var_entry)
+    return 0.0;					// No such variable
+  return (var_entry->val(&null_value));
 }
 
 
 longlong Item_func_get_user_var::val_int()
 {
-  user_var_entry *entry=get_entry();
-  if (!entry)
-    return LL(0);
-  switch (entry->type) {
-  case REAL_RESULT:
-    return (longlong) *(double*) entry->value;
-  case INT_RESULT:
-    return *(longlong*) entry->value;
-  case STRING_RESULT:
-    return strtoull(entry->value,NULL,10);	// String is null terminated
-  }
-  return LL(0);					// Impossible
+  if (!var_entry)
+    return LL(0);				// No such variable
+  return (var_entry->val_int(&null_value));
 }
 
 
@@ -2045,12 +2080,15 @@ void Item_func_get_user_var::fix_length_and_dec()
   maybe_null=1;
   decimals=NOT_FIXED_DEC;
   max_length=MAX_BLOB_WIDTH;
-  var_entry= get_variable(&thd->user_vars, name, 0);
+  if (!(var_entry= get_variable(&thd->user_vars, name, 0)))
+    null_value= 1;
 }
 
 
 bool Item_func_get_user_var::const_item() const
-{ return var_entry && current_thd->query_id != var_entry->update_query_id; }
+{
+  return var_entry && current_thd->query_id != var_entry->update_query_id;
+}
 
 
 enum Item_result Item_func_get_user_var::result_type() const

sql/item_func.h

@@ -888,45 +888,16 @@ class Item_func_set_user_var :public Item_func
   enum Item_result cached_result_type;
   LEX_STRING name;
   user_var_entry *entry;
-
-  double native_val()
-    {
-      double value=args[0]->val();
-      update_hash((void*) &value,sizeof(value), REAL_RESULT);
-      return value;
-    }
-  
-  longlong native_val_int()
-    {
-      longlong value=args[0]->val_int();
-      update_hash((void*) &value,sizeof(longlong),INT_RESULT);
-      return value;
-    }
-  
-  String *native_val_str(String *str)
-    {
-      char buffer[MAX_FIELD_WIDTH];
-      String *res=args[0]->val_str(str);
-      if (!res)					// Null value
-	update_hash((void*) 0,0,STRING_RESULT);
-      else
-	update_hash(res->c_ptr(),res->length()+1,STRING_RESULT);
-      return res;
-    }
-
-  String *native_val_str()
-    {
   char buffer[MAX_FIELD_WIDTH];
-      String tmp(buffer,sizeof(buffer));
-      return native_val_str(&tmp);
-    }
+  String value;
 
 public:
-  Item_func_set_user_var(LEX_STRING a,Item *b): Item_func(b), name(a) {}
+  Item_func_set_user_var(LEX_STRING a,Item *b):
+    Item_func(b), name(a), value(buffer,sizeof(buffer)) {}
   double val();
   longlong val_int();
   String *val_str(String *str);
-  void update_hash(void *ptr, uint length, enum Item_result type);
+  bool update_hash(const void *ptr, uint length, enum Item_result type);
   bool update();
   enum Item_result result_type () const { return cached_result_type; }
   bool fix_fields(THD *thd,struct st_table_list *tables);
@@ -945,7 +916,6 @@ class Item_func_get_user_var :public Item_func
 public:
   Item_func_get_user_var(LEX_STRING a):
     Item_func(), name(a) {}
-  user_var_entry *get_entry();
   double val();
   longlong val_int();
   String *val_str(String* str);

sql/log.cc

@@ -1315,9 +1315,9 @@ bool MYSQL_LOG::write(THD *thd, IO_CACHE *cache, bool commit_or_rollback)
       /*
         Now this Query_log_event has artificial log_pos 0. It must be adjusted
         to reflect the real position in the log. Not doing it would confuse the
-        slave: it would prevent this one from knowing where he is in the master's
-        binlog, which would result in wrong positions being shown to the user,
-        MASTER_POS_WAIT undue waiting etc.
+	slave: it would prevent this one from knowing where he is in the
+	master's binlog, which would result in wrong positions being shown to
+	the user, MASTER_POS_WAIT undue waiting etc.
       */
       qinfo.set_log_pos(this);
       if (qinfo.write(&log_file))

sql/mysqld.cc

@@ -55,7 +55,9 @@
 char pstack_file_name[80];
 #endif /* __linux__ */
 
-#if defined(HAVE_DEC_3_2_THREADS) || defined(SIGNALS_DONT_BREAK_READ)
+/* We have HAVE_purify below as this speeds up the shutdown of MySQL */
+
+#if defined(HAVE_DEC_3_2_THREADS) || defined(SIGNALS_DONT_BREAK_READ) || defined(HAVE_purify) && defined(__linux__)
 #define HAVE_CLOSE_SERVER_SOCK 1
 #endif  
 
@@ -534,12 +536,14 @@ static void close_connections(void)
     struct timespec abstime;
     int error;
     LINT_INIT(error);
+    DBUG_PRINT("info",("Waiting for select_thread"));
+
 #ifndef DONT_USE_THR_ALARM
     if (pthread_kill(select_thread,THR_CLIENT_ALARM))
       break;					// allready dead
 #endif
     set_timespec(abstime, 2);
-    for (uint tmp=0 ; tmp < 10 ; tmp++)
+    for (uint tmp=0 ; tmp < 10 && select_thread_in_use; tmp++)
     {
       error=pthread_cond_timedwait(&COND_thread_count,&LOCK_thread_count,
 				   &abstime);
@@ -700,8 +704,8 @@ static void close_server_sock()
     VOID(shutdown(tmp_sock,2));
 #if defined(__NETWARE__)
     /*
-      The following code is disabled for normal systems as it causes MySQL
-      AIX 4.3 during shutdown (not tested, but likely)
+      The following code is disabled for normal systems as it may cause MySQL
+      to hang on AIX 4.3 during shutdown
     */
     DBUG_PRINT("info",("calling closesocket on unix/IP socket"));
     VOID(closesocket(tmp_sock));

sql/sql_class.h

@@ -783,8 +783,13 @@ class user_var_entry
   char *value;
   ulong length, update_query_id;
   Item_result type;
+
+  double val(my_bool *null_value);
+  longlong val_int(my_bool *null_value);
+  String *val_str(my_bool *null_value, String *str, uint decimals);
 };
 
+
 /* Class for unique (removing of duplicates) */
 
 class Unique :public Sql_alloc