MDEV-15456 Server crashes upon adding or dropping a partition in ALTE…

…R under LOCK TABLE after ER_SAME_NAME_PARTITION ALTER TABLE ... ADD PARTITION modifies the open TABLE structure, and sets table->need_reopen=1 to reset these modifications in case of an error. But under LOCK TABLES the table isn't get reopened, despite need_reopen. Fixed by reopening need_reopen tables under LOCK TABLE.
MariaDB · Apr 20, 2018 · bcb36ee · kevgs · Apr 24, 2018 · vuvova
1 parent 86718fd
commit bcb36ee
Show file tree

Hide file tree

Showing 12 changed files with 58 additions and 15 deletions.
diff --git a/mysql-test/suite/parts/inc/part_alter_values.inc b/mysql-test/suite/parts/inc/part_alter_values.inc
@@ -35,3 +35,13 @@ ALTER TABLE t1 REORGANIZE PARTITION p1 INTO
   PARTITION p3 VALUES IN (4,5,6)
 );
 DROP TABLE t1;
+
+#
+# MDEV-15456 Server crashes upon adding or dropping a partition in ALTER under LOCK TABLE after ER_SAME_NAME_PARTITION
+#
+create table t1 (i int) partition by range(i) (partition p0 values less than (10));
+lock table t1 write;
+--error ER_SAME_NAME_PARTITION
+alter table t1 add partition (partition p0 values less than (20));
+alter table t1 add partition (partition p1 values less than (20)) /* comment */;
+drop table t1;
diff --git a/mysql-test/suite/parts/r/partition_alter_innodb.result b/mysql-test/suite/parts/r/partition_alter_innodb.result
@@ -47,3 +47,9 @@ PARTITION p3 VALUES IN (4,5,6)
 );
 ERROR HY000: Syntax error: LIST PARTITIONING requires definition of VALUES IN for each partition
 DROP TABLE t1;
+create table t1 (i int) partition by range(i) (partition p0 values less than (10));
+lock table t1 write;
+alter table t1 add partition (partition p0 values less than (20));
+ERROR HY000: Duplicate partition name p0
+alter table t1 add partition (partition p1 values less than (20)) /* comment */;
+drop table t1;
diff --git a/mysql-test/suite/parts/r/partition_alter_maria.result b/mysql-test/suite/parts/r/partition_alter_maria.result
@@ -69,3 +69,9 @@ PARTITION p3 VALUES IN (4,5,6)
 );
 ERROR HY000: Syntax error: LIST PARTITIONING requires definition of VALUES IN for each partition
 DROP TABLE t1;
+create table t1 (i int) partition by range(i) (partition p0 values less than (10));
+lock table t1 write;
+alter table t1 add partition (partition p0 values less than (20));
+ERROR HY000: Duplicate partition name p0
+alter table t1 add partition (partition p1 values less than (20)) /* comment */;
+drop table t1;
diff --git a/mysql-test/suite/parts/r/partition_alter_myisam.result b/mysql-test/suite/parts/r/partition_alter_myisam.result
@@ -42,3 +42,9 @@ PARTITION p3 VALUES IN (4,5,6)
 );
 ERROR HY000: Syntax error: LIST PARTITIONING requires definition of VALUES IN for each partition
 DROP TABLE t1;
+create table t1 (i int) partition by range(i) (partition p0 values less than (10));
+lock table t1 write;
+alter table t1 add partition (partition p0 values less than (20));
+ERROR HY000: Duplicate partition name p0
+alter table t1 add partition (partition p1 values less than (20)) /* comment */;
+drop table t1;
diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc
@@ -238,7 +238,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
 
   if (thd->locked_tables_list.locked_tables())
   {
-    if (thd->locked_tables_list.reopen_tables(thd))
+    if (thd->locked_tables_list.reopen_tables(thd, false))
       goto end;
     /* Restore the table in the table list with the new opened table */
     table_list->table= pos_in_locked_tables->table;

diff --git a/sql/sql_base.cc b/sql/sql_base.cc
@@ -568,7 +568,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables,
       old locks. This should always succeed (unless some external process
       has removed the tables)
     */
-    thd->locked_tables_list.reopen_tables(thd);
+    thd->locked_tables_list.reopen_tables(thd, false);
     /*
       Since downgrade_lock() won't do anything with shared
       metadata lock it is much simpler to go through all open tables rather
@@ -952,7 +952,10 @@ void close_thread_tables(THD *thd)
       we will exit this function a few lines below.
     */
     if (! thd->lex->requires_prelocking())
+    {
+      thd->locked_tables_list.reopen_tables(thd, true);
       DBUG_VOID_RETURN;
+    }
 
     /*
       We are in the top-level statement of a prelocked statement,
@@ -2973,7 +2976,7 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count)
 */
 
 bool
-Locked_tables_list::reopen_tables(THD *thd)
+Locked_tables_list::reopen_tables(THD *thd, bool need_reopen)
 {
   Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN);
   size_t reopen_count= 0;
@@ -2984,8 +2987,20 @@ Locked_tables_list::reopen_tables(THD *thd)
   for (TABLE_LIST *table_list= m_locked_tables;
        table_list; table_list= table_list->next_global)
   {
-    if (table_list->table)                      /* The table was not closed */
-      continue;
+    if (need_reopen)
+    {
+      if (!table_list->table || !table_list->table->needs_reopen())
+        continue;
+      /* no need to remove the table from the TDC here, thus (TABLE*)1 */
+      close_all_tables_for_name(thd, table_list->table->s,
+                                HA_EXTRA_NOT_USED, (TABLE*)1);
+      DBUG_ASSERT(table_list->table == NULL);
+    }
+    else
+    {
+      if (table_list->table)                      /* The table was not closed */
+          continue;
+    }
 
     /* Links into thd->open_tables upon success */
     if (open_table(thd, table_list, thd->mem_root, &ot_ctx))

diff --git a/sql/sql_class.h b/sql/sql_class.h
@@ -1603,7 +1603,7 @@ class Locked_tables_list
   void unlink_all_closed_tables(THD *thd,
                                 MYSQL_LOCK *lock,
                                 size_t reopen_count);
-  bool reopen_tables(THD *thd);
+  bool reopen_tables(THD *thd, bool need_reopen);
   bool restore_lock(THD *thd, TABLE_LIST *dst_table_list, TABLE *table,
                     MYSQL_LOCK *lock);
   void add_back_last_deleted_lock(TABLE_LIST *dst_table_list);

diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
@@ -6558,7 +6558,7 @@ static void alter_partition_lock_handling(ALTER_PARTITION_PARAM_TYPE *lpt)
       thd->set_stmt_da(&tmp_stmt_da);
     }
 
-    if (thd->locked_tables_list.reopen_tables(thd))
+    if (thd->locked_tables_list.reopen_tables(thd, false))
       sql_print_warning("We failed to reacquire LOCKs in ALTER TABLE");
 
     if (stmt_da)
@@ -6762,7 +6762,7 @@ void handle_alter_part_error(ALTER_PARTITION_PARAM_TYPE *lpt,
       thd->set_stmt_da(&tmp_stmt_da);
     }
 
-    if (thd->locked_tables_list.reopen_tables(thd))
+    if (thd->locked_tables_list.reopen_tables(thd, false))
       sql_print_warning("We failed to reacquire LOCKs in ALTER TABLE");
 
     if (stmt_da)

diff --git a/sql/sql_partition_admin.cc b/sql/sql_partition_admin.cc
@@ -630,7 +630,7 @@ bool Sql_cmd_alter_table_exchange_partition::
     better to keep master/slave in consistent state. Alternative would be to
     try to revert the exchange operation and issue error.
   */
-  (void) thd->locked_tables_list.reopen_tables(thd);
+  (void) thd->locked_tables_list.reopen_tables(thd, false);
 
   if ((error= write_bin_log(thd, TRUE, thd->query(), thd->query_length())))
   {

diff --git a/sql/sql_table.cc b/sql/sql_table.cc
@@ -5059,7 +5059,7 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table,
       This should always work as we have a meta lock on the table.
      */
     thd->locked_tables_list.add_back_last_deleted_lock(pos_in_locked_tables);
-    if (thd->locked_tables_list.reopen_tables(thd))
+    if (thd->locked_tables_list.reopen_tables(thd, false))
     {
       thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0);
       result= 1;
@@ -5408,7 +5408,7 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table,
       This should always work as we have a meta lock on the table.
      */
     thd->locked_tables_list.add_back_last_deleted_lock(pos_in_locked_tables);
-    if (thd->locked_tables_list.reopen_tables(thd))
+    if (thd->locked_tables_list.reopen_tables(thd, false))
     {
       thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0);
       res= 1;                                   // We got an error
@@ -7239,7 +7239,7 @@ static bool mysql_inplace_alter_table(THD *thd,
                               HA_EXTRA_PREPARE_FOR_RENAME :
                               HA_EXTRA_NOT_USED,
                               NULL);
-    if (thd->locked_tables_list.reopen_tables(thd))
+    if (thd->locked_tables_list.reopen_tables(thd, false))
       thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0);
     /* QQ; do something about metadata locks ? */
   }
@@ -9224,7 +9224,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
 
 end_inplace:
 
-  if (thd->locked_tables_list.reopen_tables(thd))
+  if (thd->locked_tables_list.reopen_tables(thd, false))
     goto err_with_mdl_after_alter;
 
   THD_STAGE_INFO(thd, stage_end);

diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
@@ -574,7 +574,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
     Ignore the return value for now. It's better to
     keep master/slave in consistent state.
   */
-  if (thd->locked_tables_list.reopen_tables(thd))
+  if (thd->locked_tables_list.reopen_tables(thd, false))
     thd->clear_error();
 
   /*

diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc
@@ -494,7 +494,7 @@ bool Sql_cmd_truncate_table::truncate_table(THD *thd, TABLE_LIST *table_ref)
       */
       error= dd_recreate_table(thd, table_ref->db, table_ref->table_name);
 
-      if (thd->locked_tables_mode && thd->locked_tables_list.reopen_tables(thd))
+      if (thd->locked_tables_mode && thd->locked_tables_list.reopen_tables(thd, false))
           thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0);
 
       /* No need to binlog a failed truncate-by-recreate. */