Fix for bug #7899 "CREATE TABLE .. SELECT .. and CONVERT_TZ() function

does not work well together". Now using simplier and more correct
implementation of st_lex::unlink_first_table()/link_first_table_back() 
(It also nicely handles case when global table list is created because
of implictly used time zone tables). (2nd attempt)

Fix for bug #7705 "CONVERT_TZ() crashes with subquery/WHERE on index
column". Implemented new approach for caching objects for constant
time zone arguments. Now instead of determining whenever these arguments
are constants and performing time zone lookup at fix_fields() stage, we
do it on first get_date() invocation.

Cleanup of global @@time_zone variable handling.


mysql-test/r/timezone2.result:
  Added test for bugs #7705 "CONVERT_TZ() crashes with subquery/WHERE on
  index column" and #7899 "CREATE TABLE .. SELECT .. and CONVERT_TZ()
  function does not work well together".
mysql-test/t/timezone2.test:
  Added test for bugs #7705 "CONVERT_TZ() crashes with subquery/WHERE on
  index column" and #7899 "CREATE TABLE .. SELECT .. and CONVERT_TZ()
  function does not work well together".
sql/item_timefunc.cc:
  Item_func_convert_tz():
    New approach for caching objects for constant time zone arguments.
    Now instead of determining whenever these arguments are constants
    and performing time zone lookup at fix_fields() stage, we do it
    on first get_date() invocation. This works better in cases when 
    const_item() for these arguments returns true only on get_date()
    stage but not on fix_fields() stage (e.g. this happens in quries
    with joins or derived tables).
sql/item_timefunc.h:
  Item_func_convert_tz():
    Added from_tz_cached/to_tz_cached members indicating whenever we
    already have Time_zone object representing one of constant time zone
    arguments.
sql/set_var.cc:
  Cleaned up global @@time_zone variable handling. Now we use proper
  locking when we are setting or reading its value.
sql/set_var.h:
  Removed declaration of sys_var_thd_time_zone::get_tz_ptr() method, which
  no longer used.
sql/sql_lex.cc:
  st_lex::unlink_first_table(), st_lex::link_first_table_back():
   Simplify implementation according to Monty's suggestion.
   Instead doing something special if global and local table lists
   are the same, we simply save/restore pointers to first elements
   of both global and local lists (which works even when this lists
   are the same!). This handles nicely the case when we have separate
   global table list becuase time zone tables are implicitly used.
sql/tztime.cc:
  Backport of Monty's fixes from 5.0, which give us nicer error messages
  if we haven't found time zone with such name or its description.

mysql-test/r/timezone2.result

@@ -303,3 +303,11 @@ delete from mysql.db where user like 'mysqltest\_%';
 delete from mysql.tables_priv where user like 'mysqltest\_%';
 flush privileges;
 drop table t1, t2;
+select convert_tz('2005-01-14 17:00:00', 'UTC', custTimeZone) from (select 'UTC' as custTimeZone) as tmp;
+convert_tz('2005-01-14 17:00:00', 'UTC', custTimeZone)
+2005-01-14 17:00:00
+create table t1 select convert_tz(NULL, NULL, NULL);
+select * from t1;
+convert_tz(NULL, NULL, NULL)
+NULL
+drop table t1;

mysql-test/t/timezone2.test

@@ -266,3 +266,20 @@ delete from mysql.db where user like 'mysqltest\_%';
 delete from mysql.tables_priv where user like 'mysqltest\_%';
 flush privileges;
 drop table t1, t2;
+
+#
+# Test for bug #7705 "CONVERT_TZ() crashes with subquery/WHERE on index 
+# column". Queries in which one of time zone arguments of CONVERT_TZ() is
+# determined as constant only at val() stage (not at fix_fields() stage),
+# should not crash server.
+#
+select convert_tz('2005-01-14 17:00:00', 'UTC', custTimeZone) from (select 'UTC' as custTimeZone) as tmp;
+
+#
+# Test for bug #7899 "CREATE TABLE .. SELECT .. and CONVERT_TZ() function
+# does not work well together". The following statement should return only
+# one NULL row and not result of full join.
+#
+create table t1 select convert_tz(NULL, NULL, NULL);
+select * from t1;
+drop table t1;

sql/item_timefunc.cc

@@ -1656,6 +1656,7 @@ void Item_func_convert_tz::fix_length_and_dec()
   collation.set(&my_charset_bin);
   decimals= 0;
   max_length= MAX_DATETIME_WIDTH*MY_CHARSET_BIN_MB_MAXLEN;
+  maybe_null= 1;
 }
 
 
@@ -1668,12 +1669,6 @@ Item_func_convert_tz::fix_fields(THD *thd_arg, TABLE_LIST *tables_arg, Item **re
 
   tz_tables= thd_arg->lex->time_zone_tables_used;
 
-  if (args[1]->const_item())
-    from_tz= my_tz_find(args[1]->val_str(&str), tz_tables);
-
-  if (args[2]->const_item())
-    to_tz= my_tz_find(args[2]->val_str(&str), tz_tables);
-
   return 0;
 }
 
@@ -1713,13 +1708,19 @@ bool Item_func_convert_tz::get_date(TIME *ltime,
   my_time_t my_time_tmp;
   bool not_used;
   String str;
-  
+
-  if (!args[1]->const_item())
+  if (!from_tz_cached)
+  {
     from_tz= my_tz_find(args[1]->val_str(&str), tz_tables);
-  
+    from_tz_cached= args[1]->const_item();
-  if (!args[2]->const_item())
+  }
+
+  if (!to_tz_cached)
+  {
     to_tz= my_tz_find(args[2]->val_str(&str), tz_tables);
-  
+    to_tz_cached= args[2]->const_item();
+  }
+
   if (from_tz==0 || to_tz==0 || get_arg0_date(ltime, 0))
   {
     null_value= 1;
@@ -1741,6 +1742,13 @@ bool Item_func_convert_tz::get_date(TIME *ltime,
 }
 
 
+void Item_func_convert_tz::cleanup()
+{
+  from_tz_cached= to_tz_cached= 0;
+  Item_date_func::cleanup();
+}
+
+
 void Item_date_add_interval::fix_length_and_dec()
 {
   enum_field_types arg0_field_type;

sql/item_timefunc.h

@@ -545,12 +545,15 @@ class Item_func_convert_tz :public Item_date_func
   TABLE_LIST *tz_tables;
   /*
     If time zone parameters are constants we are caching objects that
-    represent them.
+    represent them (we use separate from_tz_cached/to_tz_cached members
+    to indicate this fact, since NULL is legal value for from_tz/to_tz
+    members.
   */
+  bool from_tz_cached, to_tz_cached;
   Time_zone *from_tz, *to_tz;
  public:
   Item_func_convert_tz(Item *a, Item *b, Item *c):
-    Item_date_func(a, b, c) {}
+    Item_date_func(a, b, c), from_tz_cached(0), to_tz_cached(0) {}
   longlong val_int();
   double val() { return (double) val_int(); }
   String *val_str(String *str);
@@ -558,6 +561,7 @@ class Item_func_convert_tz :public Item_date_func
   bool fix_fields(THD *, struct st_table_list *, Item **);
   void fix_length_and_dec();
   bool get_date(TIME *res, uint fuzzy_date);
+  void cleanup();
 };
 
 

sql/set_var.cc

@@ -2438,8 +2438,15 @@ bool sys_var_thd_time_zone::check(THD *thd, set_var *var)
 
 bool sys_var_thd_time_zone::update(THD *thd, set_var *var)
 {
-  /* We are using Time_zone object found during check() phase */ 
+  /* We are using Time_zone object found during check() phase. */
-  *get_tz_ptr(thd,var->type)= var->save_result.time_zone;
+  if (var->type == OPT_GLOBAL)
+  {
+    pthread_mutex_lock(&LOCK_global_system_variables);
+    global_system_variables.time_zone= var->save_result.time_zone;
+    pthread_mutex_unlock(&LOCK_global_system_variables);
+  }
+  else
+    thd->variables.time_zone= var->save_result.time_zone;
   return 0;
 }
 
@@ -2451,27 +2458,25 @@ byte *sys_var_thd_time_zone::value_ptr(THD *thd, enum_var_type type,
     We can use ptr() instead of c_ptr() here because String contaning
     time zone name is guaranteed to be zero ended.
   */
-  return (byte *)((*get_tz_ptr(thd,type))->get_name()->ptr());
-}
-
-
-Time_zone** sys_var_thd_time_zone::get_tz_ptr(THD *thd, 
-                                              enum_var_type type)
-{
   if (type == OPT_GLOBAL)
-    return &global_system_variables.time_zone;
+    return (byte *)(global_system_variables.time_zone->get_name()->ptr());
   else
-    return &thd->variables.time_zone;
+    return (byte *)(thd->variables.time_zone->get_name()->ptr());
 }
 
 
 void sys_var_thd_time_zone::set_default(THD *thd, enum_var_type type)
 {
+ pthread_mutex_lock(&LOCK_global_system_variables);
  if (type == OPT_GLOBAL)
  {
    if (default_tz_name)
    {
      String str(default_tz_name, &my_charset_latin1);
+     /*
+       We are guaranteed to find this time zone since its existence
+       is checked during start-up.
+     */
      global_system_variables.time_zone=
        my_tz_find(&str, thd->lex->time_zone_tables_used);
    }
@@ -2480,6 +2485,7 @@ void sys_var_thd_time_zone::set_default(THD *thd, enum_var_type type)
  }
  else
    thd->variables.time_zone= global_system_variables.time_zone;
+ pthread_mutex_unlock(&LOCK_global_system_variables);
 }
 
 /*

sql/set_var.h

@@ -736,7 +736,6 @@ public:
   bool update(THD *thd, set_var *var);
   byte *value_ptr(THD *thd, enum_var_type type, LEX_STRING *base);
   virtual void set_default(THD *thd, enum_var_type type);
-  Time_zone **get_tz_ptr(THD *thd, enum_var_type type);
 };
 
 /****************************************************************************

sql/sql_lex.cc

@@ -1719,8 +1719,8 @@ st_lex::st_lex()
     global_first	Save first global table here
     local_first		Save first local table here
 
-  NORES
+  NOTES
-   global_first & local_first are used to save result for link_first_table_back
+    This function assumes that outer select list is non-empty.
 
   RETURN
     global list without first table
@@ -1730,25 +1730,25 @@ TABLE_LIST *st_lex::unlink_first_table(TABLE_LIST *tables,
 				       TABLE_LIST **global_first,
 				       TABLE_LIST **local_first)
 {
+  DBUG_ASSERT(select_lex.table_list.first != 0);
+  /*
+    Save pointers to first elements of global table list and list
+    of tables used in outer select. It does not harm if these lists
+    are the same.
+  */
   *global_first= tables;
   *local_first= (TABLE_LIST*)select_lex.table_list.first;
-  /*
+
-    Exclude from global table list
+  /* Exclude first elements from these lists */
-  */
+  select_lex.table_list.first= (byte*) (*local_first)->next;
   tables= tables->next;
-  /*
-    and from local list if it is not the same
-  */
-  select_lex.table_list.first= ((&select_lex != all_selects_list) ?
-				(byte*) (*local_first)->next :
-				(byte*) tables);
   (*global_first)->next= 0;
   return tables;
 }
 
 
 /*
-  Link table back that was unlinked with unlink_first_table()
+  Link table which was unlinked with unlink_first_table() back.
 
   SYNOPSIS
     link_first_table_back()
@@ -1764,16 +1764,7 @@ TABLE_LIST *st_lex::link_first_table_back(TABLE_LIST *tables,
 					  TABLE_LIST *local_first)
 {
   global_first->next= tables;
-  if (&select_lex != all_selects_list)
+  select_lex.table_list.first= (byte*) local_first;
-  {
-    /*
-      we do not touch local table 'next' field => we need just
-      put the table in the list
-    */
-    select_lex.table_list.first= (byte*) local_first;
-  }
-  else
-    select_lex.table_list.first= (byte*) global_first;
   return global_first;
 }
 

sql/tztime.cc

@@ -1773,7 +1773,13 @@ tz_load_from_open_tables(const String *tz_name, TABLE_LIST *tz_tables)
   if (table->file->index_read(table->record[0], (byte*)table->field[0]->ptr,
                               0, HA_READ_KEY_EXACT))
   {
-    sql_print_error("Can't find description of time zone.");
+#ifdef EXTRA_DEBUG    
+    /*
+      Most probably user has mistyped time zone name, so no need to bark here
+      unless we need it for debugging.
+    */
+    sql_print_error("Can't find description of time zone '%s'", tz_name_buff);
+#endif
     goto end;
   }
 
@@ -1794,7 +1800,7 @@ tz_load_from_open_tables(const String *tz_name, TABLE_LIST *tz_tables)
   if (table->file->index_read(table->record[0], (byte*)table->field[0]->ptr,
                               0, HA_READ_KEY_EXACT))
   {
-    sql_print_error("Can't find description of time zone.");
+    sql_print_error("Can't find description of time zone '%u'", tzid);
     goto end;
   }