MDEV-17726: A better fix

THD::close_temporary_tables(): Revert the change.

ha_innobase::delete_table(): Move the work-around inside
a debug assertion, and check thd_kill_level() instead of thd_killed(),
because the latter would not hold for KILL_CONNECTION.

2 files changed, 20 insertions, 13 deletions

diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc
index 574ed96ada8..3201b306fad 100644
--- a/sql/temporary_tables.cc
+++ b/sql/temporary_tables.cc
@@ -1,5 +1,5 @@
 /*
-  Copyright (c) 2016,2018 MariaDB Corporation
+  Copyright (c) 2016 MariaDB Corporation
 
   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
@@ -476,7 +476,6 @@ bool THD::close_temporary_tables()
   }
 
   DBUG_ASSERT(!rgi_slave);
-  lex->sql_command = SQLCOM_DROP_TABLE;
 
   /*
     Ensure we don't have open HANDLERs for tables we are about to close.
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index be91befcf1a..293a80ecbcc 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -13187,17 +13187,25 @@ inline int ha_innobase::delete_table(const char* name, enum_sql_command sqlcom)
 int ha_innobase::delete_table(const char* name)
 {
 	enum_sql_command sqlcom = enum_sql_command(thd_sql_command(ha_thd()));
-
-        if (sqlcom == SQLCOM_TRUNCATE
-            && thd_killed(ha_thd())
-            && (m_prebuilt == NULL
-                || dict_table_is_temporary(m_prebuilt->table))) {
-                sqlcom = SQLCOM_DROP_TABLE;
-        }
-
-	/* SQLCOM_TRUNCATE will be passed via ha_innobase::truncate() only. */
-        DBUG_ASSERT(sqlcom != SQLCOM_TRUNCATE);
-        return delete_table(name, sqlcom);
+	/* SQLCOM_TRUNCATE should be passed via ha_innobase::truncate() only.
+
+	On client disconnect, when dropping temporary tables, the
+	previous sqlcom would not be overwritten.  In such a case, we
+	will have thd_kill_level() != NOT_KILLED, !m_prebuilt can
+	hold, and sqlcom could be anything, including TRUNCATE.
+
+	The sqlcom only matters for persistent tables; no persistent
+	metadata or FOREIGN KEY metadata is kept for temporary
+	tables. Therefore, we relax the assertion. If there is a bug
+	that slips through this assertion due to !m_prebuilt, the
+	worst impact should be that on DROP TABLE of a persistent
+	table, FOREIGN KEY constraints will be ignored and their
+	metadata will not be removed. */
+	DBUG_ASSERT(sqlcom != SQLCOM_TRUNCATE
+		    || (thd_kill_level(ha_thd()) != NOT_KILLED
+			&& (!m_prebuilt
+			    || m_prebuilt->table->is_temporary())));
+	return delete_table(name, sqlcom);
 }
 
 /** Remove all tables in the named database inside InnoDB.