Cleanup of db option cacheing

Some bug fixes to last pushed code


mysql-test/mysql-test-run.sh:
  Fix for new valgrind (2.1.1)
mysql-test/r/bdb.result:
  Updated results
mysql-test/t/ps_1general.test:
  removed wrong error condition
sql/ha_berkeley.cc:
  Fix for index_flags() in new code
sql/item_strfunc.cc:
  Cleanup (fixed indentation, removed short variable names)
sql/mysql_priv.h:
  Cleanup of db option cacheing
sql/mysqld.cc:
  Cleanup of db option cacheing
sql/sql_db.cc:
  Cleanup of db option cacheing
sql/sql_parse.cc:
  Cleanup of db option cacheing
sql/sql_table.cc:
  sprintf -> strxmov
sql/table.cc:
  key_read should be tested on key parts, not the whole key

mysql-test/mysql-test-run.sh

@@ -370,7 +370,7 @@ while test $# -gt 0; do
         $ECHO "You need to have the 'valgrind' program in your PATH to run mysql-test-run with option --valgrind. Valgrind's home page is http://developer.kde.org/~sewardj ."
         exit 1
       fi
-      VALGRIND="$VALGRIND --alignment=8 --leak-check=yes --num-callers=16"
+      VALGRIND="$VALGRIND --tool=memcheck --alignment=8 --leak-check=yes --num-callers=16"
       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

mysql-test/r/bdb.result

@@ -785,7 +785,7 @@ Table	Op	Msg_type	Msg_text
 test.t1	optimize	status	OK
 show keys from t1;
 Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment
-t1	0	PRIMARY	1	a	NULL	2	NULL	NULL		BTREE	
+t1	0	PRIMARY	1	a	A	2	NULL	NULL		BTREE	
 drop table t1;
 create table t1 (i int, j int )ENGINE=BDB;
 insert into t1 values (1,2);

mysql-test/t/ps_1general.test

@@ -369,7 +369,6 @@ revoke all privileges on test.t1 from drop_user@localhost ;
 --error 1295
 prepare stmt3 from ' drop user drop_user@localhost ';
 drop user drop_user@localhost;
---error 1141
 
 #### table related commands
 ## describe

sql/ha_berkeley.cc

@@ -344,12 +344,24 @@ ulong ha_berkeley::index_flags(uint idx, uint part, bool all_parts) const
 {
   ulong flags= (HA_READ_NEXT | HA_READ_PREV | HA_READ_ORDER | HA_KEYREAD_ONLY
                 | HA_READ_RANGE);
-  for (uint idx= all_parts ? 0 : part ; idx <= part ; idx++)
+  for (uint i= all_parts ? 0 : part ; i <= part ; i++)
   {
-    if (table->key_info[idx].key_part[part].field->type() == FIELD_TYPE_BLOB)
+    if (table->key_info[idx].key_part[i].field->type() == FIELD_TYPE_BLOB)
     {
       /* We can't use BLOBS to shortcut sorts */
-      flags&= ~ (HA_READ_ORDER | HA_KEYREAD_ONLY | HA_READ_RANGE);
+      flags&= ~(HA_READ_ORDER | HA_KEYREAD_ONLY | HA_READ_RANGE);
+      break;
+    }
+    switch (table->key_info[idx].key_part[i].field->key_type()) {
+    case HA_KEYTYPE_TEXT:
+    case HA_KEYTYPE_VARTEXT:
+      /*
+        As BDB stores only one copy of equal strings, we can't use key read
+        on these
+      */
+      flags&= ~HA_KEYREAD_ONLY;
+      break;
+    default:                                    // Keep compiler happy
       break;
     }
   }

sql/item_strfunc.cc

@@ -2294,43 +2294,49 @@ inline int hexchar_to_int(char c)
   return -1;
 }
 
+
+  /* Convert given hex string to a binary string */
+
 String *Item_func_unhex::val_str(String *str)
 {
+  const char *from, *end;
+  char *to;
+  String *res;
+  uint length;
   DBUG_ASSERT(fixed == 1);
-  /* Convert given hex string to a binary string */
 
-  String *res= args[0]->val_str(str);
-  if (!res || tmp_value.alloc((1+res->length())/2))
+  res= args[0]->val_str(str);
+  if (!res || tmp_value.alloc(length= (1+res->length())/2))
   {
     null_value=1;
     return 0;
   }
 
-  const char *from=res->ptr(), *end;
-  char *to;
-  int r;
-
-  null_value=0;
-  tmp_value.length((1+res->length())/2);
+  from= res->ptr();
+  null_value= 0;
+  tmp_value.length(length);
   to= (char*) tmp_value.ptr();
   if (res->length() % 2)
   {
-    *to++= r= hexchar_to_int(*from++);
-    if ((null_value= (r == -1)))
+    int hex_char;
+    *to++= hex_char= hexchar_to_int(*from++);
+    if ((null_value= (hex_char == -1)))
       return 0;
   }
   for (end=res->ptr()+res->length(); from < end ; from+=2, to++)
   {
-    *to= (r= hexchar_to_int(from[0])) << 4;
-    if ((null_value= (r == -1)))
+    int hex_char;
+    *to= (hex_char= hexchar_to_int(from[0])) << 4;
+    if ((null_value= (hex_char == -1)))
       return 0;
-    *to|= r= hexchar_to_int(from[1]);
-    if ((null_value= (r == -1)))
+    *to|= hex_char= hexchar_to_int(from[1]);
+    if ((null_value= (hex_char == -1)))
       return 0;
   }
   return &tmp_value;
 }
 
+
 void Item_func_binary::print(String *str)
 {
   str->append("cast(", 5);

sql/mysql_priv.h

@@ -801,6 +801,8 @@ bool is_keyword(const char *name, uint len);
 
 #define MY_DB_OPT_FILE "db.opt"
 bool load_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create);
+bool my_dbopt_init(void);
+void my_dbopt_cleanup(void);
 void my_dbopt_free(void);
 
 /*
@@ -895,8 +897,7 @@ extern pthread_mutex_t LOCK_mysql_create_db,LOCK_Acl,LOCK_open,
        LOCK_delayed_status, LOCK_delayed_create, LOCK_crypt, LOCK_timezone,
        LOCK_slave_list, LOCK_active_mi, LOCK_manager,
        LOCK_global_system_variables, LOCK_user_conn;
-extern rw_lock_t LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave,
-       LOCK_dboptions;
+extern rw_lock_t LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave;
 extern pthread_cond_t COND_refresh, COND_thread_count, COND_manager;
 extern pthread_attr_t connection_attrib;
 extern I_List<THD> threads;

sql/mysqld.cc

@@ -382,8 +382,7 @@ pthread_mutex_t LOCK_mysql_create_db, LOCK_Acl, LOCK_open, LOCK_thread_count,
 		LOCK_crypt, LOCK_bytes_sent, LOCK_bytes_received,
 	        LOCK_global_system_variables,
 		LOCK_user_conn, LOCK_slave_list, LOCK_active_mi;
-rw_lock_t	LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave,
-                LOCK_dboptions;
+rw_lock_t	LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave;
 pthread_cond_t COND_refresh,COND_thread_count, COND_slave_stopped,
 	       COND_slave_start;
 pthread_cond_t COND_thread_cache,COND_flush_thread_cache;
@@ -886,6 +885,7 @@ extern "C" void unireg_abort(int exit_code)
 }
 #endif
 
+
 void clean_up(bool print_message)
 {
   DBUG_PRINT("exit",("clean_up"));
@@ -988,7 +988,6 @@ static void clean_up_mutexes()
   (void) pthread_mutex_destroy(&LOCK_mysql_create_db);
   (void) pthread_mutex_destroy(&LOCK_Acl);
   (void) rwlock_destroy(&LOCK_grant);
-  (void) rwlock_destroy(&LOCK_dboptions);
   (void) pthread_mutex_destroy(&LOCK_open);
   (void) pthread_mutex_destroy(&LOCK_thread_count);
   (void) pthread_mutex_destroy(&LOCK_mapped_file);
@@ -2277,7 +2276,6 @@ static int init_common_variables(const char *conf_file_name, int argc,
   */
   global_system_variables.time_zone= my_tz_SYSTEM;
   
-
   /*
     Init mutexes for the global MYSQL_LOG objects.
     As safe_mutex depends on what MY_INIT() does, we can't init the mutexes of
@@ -2384,6 +2382,9 @@ static int init_common_variables(const char *conf_file_name, int argc,
 
   if (use_temp_pool && bitmap_init(&temp_pool,0,1024,1))
     return 1;
+  if (my_dbopt_init())
+    return 1;
+
   return 0;
 }
 
@@ -2411,7 +2412,6 @@ static int init_thread_environment()
   (void) my_rwlock_init(&LOCK_sys_init_connect, NULL);
   (void) my_rwlock_init(&LOCK_sys_init_slave, NULL);
   (void) my_rwlock_init(&LOCK_grant, NULL);
-  (void) my_rwlock_init(&LOCK_dboptions, NULL);
   (void) pthread_cond_init(&COND_thread_count,NULL);
   (void) pthread_cond_init(&COND_refresh,NULL);
   (void) pthread_cond_init(&COND_thread_cache,NULL);
@@ -4677,26 +4677,26 @@ replicating a LOAD DATA INFILE command.",
    "The size of the buffer used for index blocks for MyISAM tables. Increase this to get better index handling (for all reads and multiple writes) to as much as you can afford; 64M on a 256M machine that mainly runs MySQL is quite common.",
    (gptr*) &dflt_key_cache_var.param_buff_size,
    (gptr*) 0,
-   0, (enum get_opt_var_type) (GET_ULL | GET_ASK_ADDR),
+   0, (GET_ULL | GET_ASK_ADDR),
    REQUIRED_ARG, KEY_CACHE_SIZE, MALLOC_OVERHEAD, (long) ~0, MALLOC_OVERHEAD,
    IO_SIZE, 0},
   {"key_cache_block_size", OPT_KEY_CACHE_BLOCK_SIZE,
    "The default size of key cache blocks",
    (gptr*) &dflt_key_cache_var.param_block_size,
    (gptr*) 0,
-   0, (enum get_opt_var_type) (GET_ULONG | GET_ASK_ADDR), REQUIRED_ARG,
+   0, (GET_ULONG | GET_ASK_ADDR), REQUIRED_ARG,
    KEY_CACHE_BLOCK_SIZE , 512, 1024*16, MALLOC_OVERHEAD, 512, 0},
   {"key_cache_division_limit", OPT_KEY_CACHE_DIVISION_LIMIT,
    "The minimum percentage of warm blocks in key cache",
    (gptr*) &dflt_key_cache_var.param_division_limit,
    (gptr*) 0,
-   0, (enum get_opt_var_type) (GET_ULONG | GET_ASK_ADDR) , REQUIRED_ARG, 100,
+   0, (GET_ULONG | GET_ASK_ADDR) , REQUIRED_ARG, 100,
    1, 100, 0, 1, 0},
   {"key_cache_age_threshold", OPT_KEY_CACHE_AGE_THRESHOLD,
    "This characterizes the number of hits a hot block has to be untouched until it is considered aged enough to be downgraded to a warm block. This specifies the percentage ratio of that number of hits to the total number of blocks in key cache",
    (gptr*) &dflt_key_cache_var.param_age_threshold,
    (gptr*) 0,
-   0, (enum get_opt_var_type) (GET_ULONG | GET_ASK_ADDR), REQUIRED_ARG, 
+   0, (GET_ULONG | GET_ASK_ADDR), REQUIRED_ARG, 
    300, 100, ~0L, 0, 100, 0},
   {"long_query_time", OPT_LONG_QUERY_TIME,
    "Log all queries that have taken more than long_query_time seconds to execute to file.",

sql/sql_db.cc

@@ -42,6 +42,7 @@ static long mysql_rm_known_files(THD *thd, MY_DIR *dirp,
 /* Database options hash */
 static HASH dboptions;
 static my_bool dboptions_init= 0;
+static rw_lock_t LOCK_dboptions;
 
 /* Structure for database options */
 typedef struct my_dbopt_st
@@ -51,6 +52,7 @@ typedef struct my_dbopt_st
   CHARSET_INFO *charset;	/* Database default character set */
 } my_dbopt_t;
 
+
 /*
   Function we use in the creation of our hash to get key.
 */
@@ -73,39 +75,58 @@ static void free_dbopt(void *dbopt)
 
 
 /* 
-  Initialize database option hash.
+  Initialize database option hash
+
+  SYNOPSIS
+    my_dbopt_init()
+
+  NOTES
+    Must be called before any other database function is called.
+
+  RETURN
+    0	ok
+    1	Fatal error
 */
 
-static my_bool my_dbopt_init(void)
+bool my_dbopt_init(void)
 {
-  my_bool rc;
-  rw_wrlock(&LOCK_dboptions);
+  bool error= 0;
+  (void) my_rwlock_init(&LOCK_dboptions, NULL);
   if (!dboptions_init)
   {
     dboptions_init= 1;
-    rc= hash_init(&dboptions, lower_case_table_names ? 
+    error= hash_init(&dboptions, lower_case_table_names ? 
                      &my_charset_bin : system_charset_info,
                      32, 0, 0, (hash_get_key) dboptions_get_key,
                      free_dbopt,0);
   }
-  else
-    rc= 0;
-  rw_unlock(&LOCK_dboptions);
-  return rc;
+  return error;
 }
 
 
 /* 
   Free database option hash.
 */
+
 void my_dbopt_free(void)
 {
-  rw_wrlock(&LOCK_dboptions);
   if (dboptions_init)
   {
-    hash_free(&dboptions);
     dboptions_init= 0;
+    hash_free(&dboptions);
+    (void) rwlock_destroy(&LOCK_dboptions);
   }
+}
+
+
+void my_dbopt_cleanup(void)
+{
+  rw_wrlock(&LOCK_dboptions);
+  hash_free(&dboptions);
+  hash_init(&dboptions, lower_case_table_names ? 
+            &my_charset_bin : system_charset_info,
+            32, 0, 0, (hash_get_key) dboptions_get_key,
+            free_dbopt,0);
   rw_unlock(&LOCK_dboptions);
 }
 
@@ -126,10 +147,7 @@ static my_bool get_dbopt(const char *dbname, HA_CREATE_INFO *create)
 {
   my_dbopt_t *opt;
   uint length;
-  my_bool rc;
-  
-  if (my_dbopt_init())
-    return 1;
+  my_bool error= 1;
   
   length= (uint) strlen(dbname);
   
@@ -137,13 +155,10 @@ static my_bool get_dbopt(const char *dbname, HA_CREATE_INFO *create)
   if ((opt= (my_dbopt_t*) hash_search(&dboptions, (byte*) dbname, length)))
   {
     create->default_table_charset= opt->charset;
-    rc= 0;
+    error= 0;
   }
-  else
-    rc= 1;
   rw_unlock(&LOCK_dboptions);
-  
-  return rc;
+  return error;
 }
 
 
@@ -163,43 +178,41 @@ static my_bool put_dbopt(const char *dbname, HA_CREATE_INFO *create)
 {
   my_dbopt_t *opt;
   uint length;
-  my_bool rc;
-  
-  if (my_dbopt_init())
-    return 1;
+  my_bool error= 0;
+  DBUG_ENTER("put_dbopt");
 
   length= (uint) strlen(dbname);
   
   rw_wrlock(&LOCK_dboptions);
-  if ((opt= (my_dbopt_t*) hash_search(&dboptions, (byte*) dbname, length)))
-  { 
-    /* Options are already in hash, update them */
-    opt->charset= create->default_table_charset;
-    rc= 0;
-  }
-  else
+  if (!(opt= (my_dbopt_t*) hash_search(&dboptions, (byte*) dbname, length)))
   { 
     /* Options are not in the hash, insert them */
     char *tmp_name;
     if (!my_multi_malloc(MYF(MY_WME | MY_ZEROFILL),
-                         &opt, sizeof(*opt), &tmp_name, length+1, NullS))
+                         &opt, (uint) sizeof(*opt), &tmp_name, length+1,
+                         NullS))
     {
-      rc= 1;
-      goto ret;
+      error= 1;
+      goto end;
     }
     
     opt->name= tmp_name;
-    opt->name_length= length;
-    opt->charset= create->default_table_charset;
     strmov(opt->name, dbname);
+    opt->name_length= length;
     
-    if ((rc= my_hash_insert(&dboptions, (byte*) opt)))
+    if ((error= my_hash_insert(&dboptions, (byte*) opt)))
+    {
       my_free((gptr) opt, MYF(0));
+      goto end;
+    }
   }
 
-ret:
+  /* Update / write options in hash */
+  opt->charset= create->default_table_charset;
+
+end:
   rw_unlock(&LOCK_dboptions);  
-  return rc;
+  DBUG_RETURN(error);
 }
 
 
@@ -243,10 +256,11 @@ static bool write_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create)
   if ((file=my_create(path, CREATE_MODE,O_RDWR | O_TRUNC,MYF(MY_WME))) >= 0)
   {
     ulong length;
-    length= my_sprintf(buf,(buf,
-			    "default-character-set=%s\ndefault-collation=%s\n",
+    length= (ulong) (strxnmov(buf, sizeof(buf), "default-character-set=",
                               create->default_table_charset->csname,
-                            create->default_table_charset->name));
+                              "\ndefault-collation=",
+                              create->default_table_charset->name,
+                              "\n", NullS) - buf);
 
     /* Error is written by my_write */
     if (!my_write(file,(byte*) buf, length, MYF(MY_NABP+MY_WME)))
@@ -362,10 +376,11 @@ int mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info,
 		    bool silent)
 {
   char	 path[FN_REFLEN+16];
-  long result=1;
-  int error = 0;
+  long result= 1;
+  int error= 0;
+  uint length;
   MY_STAT stat_info;
-  uint create_options = create_info ? create_info->options : 0;
+  uint create_options= create_info ? create_info->options : 0;
   DBUG_ENTER("mysql_create_db");
   
   VOID(pthread_mutex_lock(&LOCK_mysql_create_db));
@@ -386,10 +401,10 @@ int mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info,
    if (!(create_options & HA_LEX_CREATE_IF_NOT_EXISTS))
     {
       my_error(ER_DB_CREATE_EXISTS,MYF(0),db);
-      error = -1;
+      error= -1;
       goto exit;
     }
-    result = 0;
+    result= 0;
   }
   else
   {
@@ -402,19 +417,20 @@ int mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info,
     if (my_mkdir(path,0777,MYF(0)) < 0)
     {
       my_error(ER_CANT_CREATE_DB,MYF(0),db,my_errno);
-      error = -1;
+      error= -1;
       goto exit;
     }
   }
 
-  unpack_dirname(path, path);
-  strcat(path,MY_DB_OPT_FILE);
+  length= unpack_dirname(path, path);
+  strmov(path+ length, MY_DB_OPT_FILE);
   if (write_db_opt(thd, path, create_info))
   {
     /*
       Could not create options file.
       Restore things to beginning.
     */
+    path[length]= 0;
     if (rmdir(path) >= 0)
     {
       error= -1;
@@ -466,7 +482,7 @@ int mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info)
 {
   char path[FN_REFLEN+16];
   long result=1;
-  int error = 0;
+  int error= 0;
   DBUG_ENTER("mysql_alter_db");
 
   VOID(pthread_mutex_lock(&LOCK_mysql_create_db));
@@ -476,7 +492,7 @@ int mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info)
     goto exit2;
 
   /* Check directory */
-  (void)sprintf(path,"%s/%s/%s", mysql_data_home, db, MY_DB_OPT_FILE);
+  strxmov(path, mysql_data_home, "/", db, "/", MY_DB_OPT_FILE, NullS);
   fn_format(path, path, "", "", MYF(MY_UNPACK_FILENAME));
   if ((error=write_db_opt(thd, path, create_info)))
     goto exit;
@@ -530,7 +546,7 @@ int mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info)
 int mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent)
 {
   long deleted=0;
-  int error = 0;
+  int error= 0;
   char	path[FN_REFLEN+16], tmp_db[NAME_LEN+1];
   MY_DIR *dirp;
   uint length;
@@ -553,7 +569,7 @@ int mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent)
   path[length]= '\0';				// Remove file name
 
   /* See if the directory exists */
-  if (!(dirp = my_dir(path,MYF(MY_DONT_SORT))))
+  if (!(dirp= my_dir(path,MYF(MY_DONT_SORT))))
   {
     if (!if_exists)
     {

sql/sql_parse.cc

@@ -4562,6 +4562,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
   register TABLE_LIST *ptr;
   char *alias_str;
   DBUG_ENTER("add_table_to_list");
+  DBUG_PRINT("enter",("lock_option: %d", lock_type));
 
   if (!table)
     DBUG_RETURN(0);				// End of memory
@@ -4805,7 +4806,7 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
       if (lock_global_read_lock(thd))
 	return 1;
     }
-    my_dbopt_free();
+    my_dbopt_cleanup();
     result=close_cached_tables(thd,(options & REFRESH_FAST) ? 0 : 1, tables);
   }
   if (options & REFRESH_HOSTS)

sql/sql_table.cc

@@ -1168,7 +1168,7 @@ int mysql_create_table(THD *thd,const char *db, const char *table_name,
     HA_CREATE_INFO db_info;
     uint length;
     char  path[FN_REFLEN];
-    (void) sprintf(path,"%s/%s", mysql_data_home, db);
+    strxmov(path, mysql_data_home, "/", db, NullS);
     length= unpack_dirname(path,path);             // Convert if not unix
     strmov(path+length, MY_DB_OPT_FILE);
     load_db_opt(thd, path, &db_info);

sql/table.cc

@@ -569,7 +569,7 @@ int openfrm(const char *name, const char *alias, uint db_stat, uint prgflag,
 	  if (field->key_length() == key_part->length &&
 	      !(field->flags & BLOB_FLAG))
 	  {
-            if (outparam->file->index_flags(key, i, 1) & HA_KEYREAD_ONLY)
+            if (outparam->file->index_flags(key, i, 0) & HA_KEYREAD_ONLY)
             {
               outparam->read_only_keys.clear_bit(key);
               outparam->keys_for_keyread.set_bit(key);