From b26e603aebc0c375751cc1d08029b3fb603a0373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 28 Nov 2018 15:17:56 +0200 Subject: [PATCH] MDEV-17859 Operating system errors in file operations after failed CREATE This is a regression due to MDEV-17816. When creating a table fails, we must roll back the dictionary transaction. Because the rollback may rename tables, and because InnoDB lacks proper undo logging for CREATE operations, we must drop the incompletely created table before rolling back the transaction, which could include a RENAME operation. But, we must not blindly drop the table by name; after all, the operation could have failed because another table by the same name already existed. create_table_info_t::m_drop_before_rollback: A flag that is set if the table needs to be dropped before transaction rollback. create_table_info_t::create_table(): Remove some duplicated error handling. ha_innobase::create(): On error, only drop the table if it was actually created. --- mysql-test/suite/innodb/r/truncate.result | 15 +++++++++++++ mysql-test/suite/innodb/t/truncate.test | 17 ++++++++++++++ storage/innobase/handler/ha_innodb.cc | 27 ++++++++++++----------- storage/innobase/handler/ha_innodb.h | 7 +++++- 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/mysql-test/suite/innodb/r/truncate.result b/mysql-test/suite/innodb/r/truncate.result index e25cd6a0bea..a44d37da230 100644 --- a/mysql-test/suite/innodb/r/truncate.result +++ b/mysql-test/suite/innodb/r/truncate.result @@ -27,3 +27,18 @@ SHOW TABLE STATUS; Name Engine Version Row_format Rows Avg_row_length Data_length Max_data_length Index_length Data_free Auto_increment Create_time Update_time Check_time Collation Checksum Create_options Comment t1 InnoDB # Compressed # # # # # # 1 # # NULL latin1_swedish_ci NULL key_block_size=4 DROP TABLE t1; +# +# MDEV-17859 Operating system errors in file operations +# after failed CREATE +# +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +INSERT INTO t1 VALUES (1); +call mtr.add_suppression("InnoDB: (Operating system )?[Ee]rror number"); +call mtr.add_suppression("InnoDB: Cannot create file '.*t1\\.ibd"); +FLUSH TABLES; +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +ERROR HY000: Tablespace for table '`test`.`t1`' exists. Please DISCARD the tablespace before IMPORT +SELECT * FROM t1; +a +1 +DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/truncate.test b/mysql-test/suite/innodb/t/truncate.test index 16a3b06a8ae..5a80f7c49e3 100644 --- a/mysql-test/suite/innodb/t/truncate.test +++ b/mysql-test/suite/innodb/t/truncate.test @@ -36,3 +36,20 @@ TRUNCATE TABLE t1; --replace_column 3 # 5 # 6 # 7 # 8 # 9 # 10 # 12 # 13 # SHOW TABLE STATUS; DROP TABLE t1; + +--echo # +--echo # MDEV-17859 Operating system errors in file operations +--echo # after failed CREATE +--echo # +let $MYSQLD_DATADIR= `select @@datadir`; +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +INSERT INTO t1 VALUES (1); +call mtr.add_suppression("InnoDB: (Operating system )?[Ee]rror number"); +call mtr.add_suppression("InnoDB: Cannot create file '.*t1\\.ibd"); +FLUSH TABLES; +--move_file $MYSQLD_DATADIR/test/t1.frm $MYSQLD_DATADIR/test/hidden.frm +--error ER_TABLESPACE_EXISTS +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +--move_file $MYSQLD_DATADIR/test/hidden.frm $MYSQLD_DATADIR/test/t1.frm +SELECT * FROM t1; +DROP TABLE t1; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 00672bf91bd..fd970a069e3 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -10966,6 +10966,7 @@ create_table_info_t::create_table_def() DBUG_PRINT("enter", ("table_name: %s", m_table_name)); DBUG_ASSERT(m_trx->mysql_thd == m_thd); + DBUG_ASSERT(!m_drop_before_rollback); /* MySQL does the name length check. But we do additional check on the name length here */ @@ -11228,6 +11229,7 @@ err_col: table, m_trx, (fil_encryption_t)options->encryption, (uint32_t)options->encryption_key_id); + m_drop_before_rollback = (err == DB_SUCCESS); } DBUG_EXECUTE_IF("ib_crash_during_create_for_encryption", @@ -12531,6 +12533,9 @@ int create_table_info_t::create_table(bool create_fk) DBUG_RETURN(error); } + DBUG_ASSERT(m_drop_before_rollback + == !(m_flags2 & DICT_TF2_TEMPORARY)); + /* Create the keys */ if (m_form->s->keys == 0 || primary_key_no == -1) { @@ -12591,6 +12596,7 @@ int create_table_info_t::create_table(bool create_fk) dict_table_close(innobase_table, TRUE, FALSE); my_error(ER_WRONG_NAME_FOR_INDEX, MYF(0), FTS_DOC_ID_INDEX_NAME); + m_drop_before_rollback = false; error = -1; DBUG_RETURN(error); case FTS_EXIST_DOC_ID_INDEX: @@ -12607,16 +12613,6 @@ int create_table_info_t::create_table(bool create_fk) dict_table_close(innobase_table, TRUE, FALSE); if (error) { - /* Drop the being-created table before rollback, - so that rollback can possibly rename back a table - that could have been renamed before - the failed creation. */ - m_trx->error_state = DB_SUCCESS; - row_drop_table_for_mysql(m_table_name, m_trx, - SQLCOM_TRUNCATE); - trx_rollback_to_savepoint(m_trx, NULL); - - m_trx->error_state = DB_SUCCESS; DBUG_RETURN(error); } } @@ -12686,6 +12682,9 @@ int create_table_info_t::create_table(bool create_fk) error = convert_error_code_to_mysql(err, m_flags, NULL); if (error) { + /* row_table_add_foreign_constraints() dropped + the table */ + m_drop_before_rollback = false; DBUG_RETURN(error); } } @@ -12859,9 +12858,11 @@ ha_innobase::create( /* Drop the being-created table before rollback, so that rollback can possibly rename back a table that could have been renamed before the failed creation. */ - trx->error_state = DB_SUCCESS; - row_drop_table_for_mysql(info.table_name(), trx, - SQLCOM_TRUNCATE, true); + if (info.drop_before_rollback()) { + trx->error_state = DB_SUCCESS; + row_drop_table_for_mysql(info.table_name(), + trx, SQLCOM_TRUNCATE, true); + } trx_rollback_for_mysql(trx); row_mysql_unlock_data_dictionary(trx); if (own_trx) { diff --git a/storage/innobase/handler/ha_innodb.h b/storage/innobase/handler/ha_innodb.h index d7fd26612d7..b62045fd963 100644 --- a/storage/innobase/handler/ha_innodb.h +++ b/storage/innobase/handler/ha_innodb.h @@ -646,7 +646,7 @@ public: m_trx(trx), m_form(form), m_create_info(create_info), - m_table_name(table_name), + m_table_name(table_name), m_drop_before_rollback(false), m_remote_path(remote_path), m_innodb_file_per_table(file_per_table) {} @@ -719,6 +719,9 @@ public: const char* table_name() const { return(m_table_name); } + /** @return whether the table needs to be dropped on rollback */ + bool drop_before_rollback() const { return m_drop_before_rollback; } + THD* thd() const { return(m_thd); } @@ -760,6 +763,8 @@ private: /** Table name */ char* m_table_name; + /** Whether the table needs to be dropped before rollback */ + bool m_drop_before_rollback; /** Remote path (DATA DIRECTORY) or zero length-string */ char* m_remote_path;