List:Commits« Previous MessageNext Message »
From:konstantin Date:October 31 2007 10:40pm
Subject:bk commit into 5.1 tree (kostja:1.2610) BUG#12713
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of kostja. When kostja does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-11-01 01:40:22+03:00, kostja@bodhi.(none) +5 -0
  Implement review requests (Serg) for the patch that deploys 
  Internal_error_handler in ha_delete_table().
  
  This patch was produced while working on Bug#12713.

  sql/ha_ndbcluster.cc@stripped, 2007-11-01 01:40:19+03:00, kostja@bodhi.(none) +2 -2
    Update to the changed signature of mysql_rm_table_part2().

  sql/handler.cc@stripped, 2007-11-01 01:40:19+03:00, kostja@bodhi.(none) +3 -51
    Move interception of the possible error to the place where it is relevant:
    quick_rm_table() and mysql_rm_tabl_part2().

  sql/handler.h@stripped, 2007-11-01 01:40:19+03:00, kostja@bodhi.(none) +2 -2
    No need for generate_warning parameter of ha_delete_table any more.

  sql/sql_db.cc@stripped, 2007-11-01 01:40:19+03:00, kostja@bodhi.(none) +2 -1
    Update to the changed signature of mysql_rm_table_part2().

  sql/sql_table.cc@stripped, 2007-11-01 01:40:19+03:00, kostja@bodhi.(none) +78 -12
    Update to the changed signature of mysql_rm_table_part2().
    Use an 'internal error handler' mechanism to silence a possible 'no such 
    table' error issued by ha_delete_table.

diff -Nrup a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc
--- a/sql/ha_ndbcluster.cc	2007-09-14 21:29:16 +04:00
+++ b/sql/ha_ndbcluster.cc	2007-11-01 01:40:19 +03:00
@@ -7105,9 +7105,9 @@ int ndbcluster_find_files(handlerton *ht
       table_list.alias= table_list.table_name= (char*)file_name_str;
       (void)mysql_rm_table_part2(thd, &table_list,
                                  FALSE,   /* if_exists */
-                                 FALSE,   /* drop_temporary */ 
+                                 FALSE,   /* drop_temporary */
                                  FALSE,   /* drop_view */
-                                 TRUE     /* dont_log_query*/);
+                                 FALSE    /* log_query */);
 
       /* Clear error message that is returned when table is deleted */
       thd->clear_error();
diff -Nrup a/sql/handler.cc b/sql/handler.cc
--- a/sql/handler.cc	2007-10-30 22:35:11 +03:00
+++ b/sql/handler.cc	2007-11-01 01:40:19 +03:00
@@ -1414,42 +1414,12 @@ static const char *check_lowercase_names
 }
 
 
-/**
-  An interceptor to hijack the text of the error message without
-  setting an error in the thread. We need the text to present it
-  in the form of a warning to the user.
-*/
-
-struct Ha_delete_table_error_handler: public Internal_error_handler
-{
-public:
-  virtual bool handle_error(uint sql_errno,
-                            const char *message,
-                            MYSQL_ERROR::enum_warning_level level,
-                            THD *thd);
-  char buff[MYSQL_ERRMSG_SIZE];
-};
-
-
-bool
-Ha_delete_table_error_handler::
-handle_error(uint sql_errno,
-             const char *message,
-             MYSQL_ERROR::enum_warning_level level,
-             THD *thd)
-{
-  /* Grab the error message */
-  strmake(buff, message, sizeof(buff)-1);
-  return TRUE;
-}
-
-
 /** @brief
   This should return ENOENT if the file doesn't exists.
   The .frm file will be deleted only if we return 0 or ENOENT
 */
 int ha_delete_table(THD *thd, handlerton *table_type, const char *path,
-                    const char *db, const char *alias, bool generate_warning)
+                    const char *db, const char *alias)
 {
   handler *file;
   char tmp_path[FN_REFLEN];
@@ -1468,17 +1438,9 @@ int ha_delete_table(THD *thd, handlerton
     DBUG_RETURN(ENOENT);
 
   path= check_lowercase_names(file, path, tmp_path);
-  if ((error= file->delete_table(path)) && generate_warning)
+  if ((error= file->delete_table(path)))
   {
-    /*
-      Because file->print_error() use my_error() to generate the error message
-      we use an internal error handler to intercept it and store the text
-      in a temporary buffer. Later the message will be presented to user
-      as a warning.
-    */
-    Ha_delete_table_error_handler ha_delete_table_error_handler;
-
-    /* Fill up strucutures that print_error may need */
+    /* Fill up structures that print_error may need */
     dummy_share.path.str= (char*) path;
     dummy_share.path.length= strlen(path);
     dummy_share.db.str= (char*) db;
@@ -1490,17 +1452,7 @@ int ha_delete_table(THD *thd, handlerton
     file->table_share= &dummy_share;
     file->table= &dummy_table;
 
-    thd->push_internal_handler(&ha_delete_table_error_handler);
     file->print_error(error, 0);
-
-    thd->pop_internal_handler();
-
-    /*
-      XXX: should we convert *all* errors to warnings here?
-      What if the error is fatal?
-    */
-    push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, error,
-                ha_delete_table_error_handler.buff);
   }
   delete file;
   DBUG_RETURN(error);
diff -Nrup a/sql/handler.h b/sql/handler.h
--- a/sql/handler.h	2007-09-07 17:41:46 +04:00
+++ b/sql/handler.h	2007-11-01 01:40:19 +03:00
@@ -969,7 +969,7 @@ class handler :public Sql_alloc
 {
   friend class ha_partition;
   friend int ha_delete_table(THD*,handlerton*,const char*,const char*,
-                             const char*,bool);
+                             const char*);
 
 public:
   typedef ulonglong Table_flags;
@@ -1866,7 +1866,7 @@ int ha_create_table(THD *thd, const char
                     HA_CREATE_INFO *create_info,
 		    bool update_create_info);
 int ha_delete_table(THD *thd, handlerton *db_type, const char *path,
-                    const char *db, const char *alias, bool generate_warning);
+                    const char *db, const char *alias);
 
 /* statistics and info */
 bool ha_show_status(THD *thd, handlerton *db_type, enum ha_stat_type stat);
diff -Nrup a/sql/sql_db.cc b/sql/sql_db.cc
--- a/sql/sql_db.cc	2007-09-11 02:10:32 +04:00
+++ b/sql/sql_db.cc	2007-11-01 01:40:19 +03:00
@@ -1126,7 +1126,8 @@ static long mysql_rm_known_files(THD *th
     }
   }
   if (thd->killed ||
-      (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1)))
+      (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1,
+                                        /* log_query */ FALSE)))
     goto err;
 
   /* Remove RAID directories */
diff -Nrup a/sql/sql_table.cc b/sql/sql_table.cc
--- a/sql/sql_table.cc	2007-10-31 06:53:20 +03:00
+++ b/sql/sql_table.cc	2007-11-01 01:40:19 +03:00
@@ -1454,7 +1454,8 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
     LOCK_open during wait_if_global_read_lock(), other threads could not
     close their tables. This would make a pretty deadlock.
   */
-  error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0);
+  error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0,
+                              /* log query */ TRUE);
 
   if (need_start_waiters)
     start_waiting_global_read_lock(thd);
@@ -1465,6 +1466,46 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
   DBUG_RETURN(FALSE);
 }
 
+
+/**
+  An interceptor to hijack the text of the error message without
+  setting an error in the thread. We need the text to present it
+  in the form of a warning to the user.
+*/
+
+struct Ha_delete_table_error_handler: public Internal_error_handler
+{
+public:
+  virtual bool handle_error(uint sql_errno,
+                            const char *message,
+                            MYSQL_ERROR::enum_warning_level level,
+                            THD *thd);
+  char buff[MYSQL_ERRMSG_SIZE];
+  bool is_handled;
+
+  Ha_delete_table_error_handler() :is_handled(FALSE) {}
+};
+
+
+bool
+Ha_delete_table_error_handler::
+handle_error(uint sql_errno,
+             const char *message,
+             MYSQL_ERROR::enum_warning_level level,
+             THD *thd)
+{
+  if (sql_errno == HA_ERR_NO_SUCH_TABLE)
+  {
+    is_handled= TRUE;
+    /* Grab the error message */
+    strmake(buff, message, sizeof(buff)-1);
+    return TRUE;
+  }
+  return FALSE;
+}
+
+
+
 /*
   Execute the drop of a normal or temporary table
 
@@ -1476,8 +1517,8 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
 			In this case we give an warning of level 'NOTE'
     drop_temporary	Only drop temporary tables
     drop_view		Allow to delete VIEW .frm
-    dont_log_query	Don't write query to log files. This will also not
-			generate warnings if the handler files doesn't exists  
+    log_query           Write query to log files. This will also generate
+                        warnings if the handler files don't exist.
 
   TODO:
     When logging to the binary log, we should log
@@ -1497,7 +1538,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
 
 int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
 			 bool drop_temporary, bool drop_view,
-			 bool dont_log_query)
+			 bool log_query)
 {
   TABLE_LIST *table;
   char path[FN_REFLEN], *alias;
@@ -1512,7 +1553,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
   LINT_INIT(alias);
   LINT_INIT(path_length);
 
-  if (thd->current_stmt_binlog_row_based && !dont_log_query)
+  if (thd->current_stmt_binlog_row_based && log_query)
   {
     built_query.set_charset(system_charset_info);
     if (if_exists)
@@ -1575,7 +1616,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
       being built.  The string always end in a comma and the comma
       will be chopped off before being written to the binary log.
       */
-    if (thd->current_stmt_binlog_row_based && !dont_log_query)
+    if (thd->current_stmt_binlog_row_based && log_query)
     {
       non_temp_tables_count++;
       /*
@@ -1636,6 +1677,15 @@ int mysql_rm_table_part2(THD *thd, TABLE
     }
     else
     {
+      /*
+        ha_delete_table() calls handler::print_error() which uses my_error()
+        to generate the error message. Since we need to convert the message
+        to a warning, we use an internal error handler to intercept it and
+        store the text in a temporary buffer. Later the message will be
+        presented to user as a warning.
+      */
+      Ha_delete_table_error_handler ha_delete_table_error_handler;
+
       char *end;
       if (table_type == NULL)
       {
@@ -1644,8 +1694,19 @@ int mysql_rm_table_part2(THD *thd, TABLE
       }
       // Remove extension for delete
       *(end= path + path_length - reg_ext_length)= '\0';
-      error= ha_delete_table(thd, table_type, path, db, table->table_name,
-                             !dont_log_query);
+
+      thd->push_internal_handler(&ha_delete_table_error_handler);
+
+      error= ha_delete_table(thd, table_type, path, db, table->table_name);
+
+      thd->pop_internal_handler();
+
+      if (error && ha_delete_table_error_handler.is_handled && log_query)
+      {
+        push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, error,
+                     ha_delete_table_error_handler.buff);
+        error= 0;
+      }
       if ((error == ENOENT || error == HA_ERR_NO_SUCH_TABLE) && 
 	  (if_exists || table_type == NULL))
 	error= 0;
@@ -1695,7 +1756,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
   if (some_tables_deleted || tmp_table_deleted || !error)
   {
     query_cache_invalidate3(thd, tables, 0);
-    if (!dont_log_query)
+    if (log_query)
     {
       if (!thd->current_stmt_binlog_row_based ||
           non_temp_tables_count > 0 && !tmp_table_deleted)
@@ -1767,7 +1828,9 @@ bool quick_rm_table(handlerton *base,con
                     const char *table_name, uint flags)
 {
   char path[FN_REFLEN];
-  bool error= 0;
+  int error= 0;
+  THD *thd= current_thd;
+  Ha_delete_table_error_handler ha_delete_table_error_handler;
   DBUG_ENTER("quick_rm_table");
 
   uint path_length= build_table_filename(path, sizeof(path),
@@ -1775,8 +1838,11 @@ bool quick_rm_table(handlerton *base,con
   if (my_delete(path,MYF(0)))
     error= 1; /* purecov: inspected */
   path[path_length - reg_ext_length]= '\0'; // Remove reg_ext
-  DBUG_RETURN(ha_delete_table(current_thd, base, path, db, table_name, 0) ||
-              error);
+  /** Silence a possible 'no such table' error */
+  thd->push_internal_handler(&ha_delete_table_error_handler);
+  error|= ha_delete_table(thd, base, path, db, table_name);
+  thd->pop_internal_handler();
+  DBUG_RETURN(error && !ha_delete_table_error_handler.is_handled);
 }
 
 /*
Thread
bk commit into 5.1 tree (kostja:1.2610) BUG#12713konstantin31 Oct
  • Re: bk commit into 5.1 tree (kostja:1.2610) BUG#12713Konstantin Osipov31 Oct