MDEV-8287 DROP TABLE suppresses all engine errors

in ha_delete_table()
* only convert ENOENT and HA_ERR_NO_SUCH_TABLE to warnings
* only return real error codes (that is, not ENOENT and
  not HA_ERR_NO_SUCH_TABLE)
* intercept HA_ERR_ROW_IS_REFERENCED to generate backward
  compatible ER_ROW_IS_REFERENCED

in mysql_rm_table_no_locks()
* no special code to handle HA_ERR_ROW_IS_REFERENCED
* no special code to handle ENOENT and HA_ERR_NO_SUCH_TABLE
* return multi-table error ER_BAD_TABLE_ERROR <table list> only
  when there were many errors, not when there were many
  tables to drop (but only one table generated an error)

2 files changed, 62 insertions, 51 deletions

diff --git a/sql/handler.cc b/sql/handler.cc
index 70bce6f3963..1f8daf3927b 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -2298,9 +2298,11 @@ handle_condition(THD *,
 }
 
 
-/** @brief
-  This should return ENOENT if the file doesn't exists.
-  The .frm file will be deleted only if we return 0 or ENOENT
+/** delete a table in the engine
+
+  @note
+  ENOENT and HA_ERR_NO_SUCH_TABLE are not considered errors.
+  The .frm file will be deleted only if we return 0.
 */
 int ha_delete_table(THD *thd, handlerton *table_type, const char *path,
                     const char *db, const char *alias, bool generate_warning)
@@ -2315,47 +2317,66 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path,
   /* table_type is NULL in ALTER TABLE when renaming only .frm files */
   if (table_type == NULL || table_type == view_pseudo_hton ||
       ! (file=get_new_handler((TABLE_SHARE*)0, thd->mem_root, table_type)))
-    DBUG_RETURN(HA_ERR_NO_SUCH_TABLE);
+    DBUG_RETURN(0);
 
   bzero((char*) &dummy_table, sizeof(dummy_table));
   bzero((char*) &dummy_share, sizeof(dummy_share));
   dummy_table.s= &dummy_share;
 
   path= get_canonical_filename(file, path, tmp_path);
-  if ((error= file->ha_delete_table(path)) && generate_warning)
+  if ((error= file->ha_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.
+      it's not an error if the table doesn't exist in the engine.
+      warn the user, but still report DROP being a success
     */
-    Ha_delete_table_error_handler ha_delete_table_error_handler;
-
-    /* Fill up strucutures that print_error may need */
-    dummy_share.path.str= (char*) path;
-    dummy_share.path.length= strlen(path);
-    dummy_share.normalized_path= dummy_share.path;
-    dummy_share.db.str= (char*) db;
-    dummy_share.db.length= strlen(db);
-    dummy_share.table_name.str= (char*) alias;
-    dummy_share.table_name.length= strlen(alias);
-    dummy_table.alias.set(alias, dummy_share.table_name.length,
-                          table_alias_charset);
+    bool intercept= error == ENOENT || error == HA_ERR_NO_SUCH_TABLE;
 
-    file->change_table_ptr(&dummy_table, &dummy_share);
-
-    thd->push_internal_handler(&ha_delete_table_error_handler);
-    file->print_error(error, 0);
+    if (!intercept || generate_warning)
+    {
+      /*
+        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 */
+      dummy_share.path.str= (char*) path;
+      dummy_share.path.length= strlen(path);
+      dummy_share.normalized_path= dummy_share.path;
+      dummy_share.db.str= (char*) db;
+      dummy_share.db.length= strlen(db);
+      dummy_share.table_name.str= (char*) alias;
+      dummy_share.table_name.length= strlen(alias);
+      dummy_table.alias.set(alias, dummy_share.table_name.length,
+                            table_alias_charset);
+
+      file->change_table_ptr(&dummy_table, &dummy_share);
+
+#if MYSQL_VERSION_ID > 100105
+      // XXX as an ugly 10.0-only hack we intercept HA_ERR_ROW_IS_REFERENCED,
+      // to report it under the old historical error number.
+#error remove HA_ERR_ROW_IS_REFERENCED, use ME_JUST_WARNING instead of a handler
+#endif
+      if (intercept || error == HA_ERR_ROW_IS_REFERENCED)
+        thd->push_internal_handler(&ha_delete_table_error_handler);
 
-    thd->pop_internal_handler();
+      file->print_error(error, 0);
 
-    /*
-      XXX: should we convert *all* errors to warnings here?
-      What if the error is fatal?
-    */
-    push_warning(thd, Sql_condition::WARN_LEVEL_WARN, error,
-                ha_delete_table_error_handler.buff);
+      if (intercept || error == HA_ERR_ROW_IS_REFERENCED)
+      {
+        thd->pop_internal_handler();
+        if (error == HA_ERR_ROW_IS_REFERENCED)
+          my_message(ER_ROW_IS_REFERENCED, ER(ER_ROW_IS_REFERENCED), MYF(0));
+        else
+          push_warning(thd, Sql_condition::WARN_LEVEL_WARN, error,
+                       ha_delete_table_error_handler.buff);
+      }
+    }
+    if (intercept)
+      error= 0;
   }
   delete file;
 
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 3a7930f6963..20cfccaa3d9 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -2201,15 +2201,13 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
   char path[FN_REFLEN + 1], wrong_tables_buff[160], *alias= NULL;
   String wrong_tables(wrong_tables_buff, sizeof(wrong_tables_buff)-1,
                       system_charset_info);
-  uint path_length= 0;
+  uint path_length= 0, errors= 0;
   int error= 0;
   int non_temp_tables_count= 0;
-  bool foreign_key_error=0;
   bool non_tmp_error= 0;
   bool trans_tmp_table_deleted= 0, non_trans_tmp_table_deleted= 0;
   bool non_tmp_table_deleted= 0;
   bool is_drop_tmp_if_exists_added= 0;
-  bool one_table= tables->next_local == 0;
   bool was_view= 0;
   String built_query;
   String built_trans_tmp_query, built_non_trans_tmp_query;
@@ -2495,12 +2493,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
       error= ha_delete_table(thd, table_type, path, db, table->table_name,
                              !dont_log_query);
 
-      if (error == HA_ERR_ROW_IS_REFERENCED)
-      {
-	/* the table is referenced by a foreign key constraint */
-	foreign_key_error= 1;
-      }
-      if (!error || error == ENOENT || error == HA_ERR_NO_SUCH_TABLE)
+      if (!error)
       {
         int frm_delete_error, trigger_drop_error= 0;
 	/* Delete the table definition file */
@@ -2518,11 +2511,8 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
         if (trigger_drop_error ||
             (frm_delete_error && frm_delete_error != ENOENT))
           error= 1;
-        else if (!frm_delete_error || !error || if_exists)
-        {
-          error= 0;
+        else if (frm_delete_error && if_exists)
           thd->clear_error();
-        }
       }
       non_tmp_error= error ? TRUE : non_tmp_error;
     }
@@ -2533,6 +2523,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
       wrong_tables.append(db);
       wrong_tables.append('.');
       wrong_tables.append(table->table_name);
+      errors++;
     }
     else
     {
@@ -2556,14 +2547,13 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
 err:
   if (wrong_tables.length())
   {
-    if (one_table && was_view)
+    DBUG_ASSERT(errors);
+    if (errors == 1 && was_view)
       my_printf_error(ER_IT_IS_A_VIEW, ER(ER_IT_IS_A_VIEW), MYF(0),
                       wrong_tables.c_ptr_safe());
-    else if (!foreign_key_error)
+    else if (errors > 1 || !thd->is_error())
       my_printf_error(ER_BAD_TABLE_ERROR, ER(ER_BAD_TABLE_ERROR), MYF(0),
                       wrong_tables.c_ptr_safe());
-    else
-      my_message(ER_ROW_IS_REFERENCED, ER(ER_ROW_IS_REFERENCED), MYF(0));
     error= 1;
   }
 
@@ -2614,8 +2604,8 @@ err:
           /* Chop of the last comma */
           built_query.chop();
           built_query.append(" /* generated by server */");
-          int error_code = (non_tmp_error ?
-            (foreign_key_error ? ER_ROW_IS_REFERENCED : ER_BAD_TABLE_ERROR) : 0);
+          int error_code = non_tmp_error ?  thd->get_stmt_da()->sql_errno()
+                                         : 0;
           error |= thd->binlog_query(THD::STMT_QUERY_TYPE,
                                      built_query.ptr(),
                                      built_query.length(),