From 007f68c37f6b77588866a04d7515aca084ab950d Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 8 May 2019 23:12:01 +0300 Subject: [PATCH] Replace ha_notify_table_changed() with notify_tabledef_changed() Reason for the change was that ha_notify_table_changed() was done after table open when .frm had been replaced, which caused failure in engines that checks on open if .frm matches the engines table definition. Other changes: - Remove not needed open/close call at end of inline alter table. Some test that depended on the table beeing in the table cache after ALTER TABLE had to be updated. --- mysql-test/main/alter_table_online.result | 20 ++++----- mysql-test/suite/gcol/r/gcol_bugfixes.result | 1 - .../suite/gcol/r/innodb_virtual_stats.result | 12 ++++++ .../suite/gcol/t/innodb_virtual_stats.test | 4 ++ .../suite/innodb/r/innodb-wl5522.result | 10 +++-- .../suite/innodb/r/instant_alter_crash.result | 5 ++- mysql-test/suite/innodb/r/monitor.result | 4 +- mysql-test/suite/innodb/t/innodb-wl5522.test | 5 ++- .../suite/innodb/t/instant_alter_crash.test | 5 ++- .../innodb_zip/r/wl5522_debug_zip.result | 2 - .../suite/innodb_zip/r/wl5522_zip.result | 4 +- sql/ha_partition.cc | 13 ------ sql/ha_partition.h | 2 - sql/handler.h | 28 ++++-------- sql/sql_table.cc | 43 ++++++++++--------- storage/mroonga/ha_mroonga.cpp | 28 ------------ storage/mroonga/ha_mroonga.hpp | 3 -- 17 files changed, 78 insertions(+), 111 deletions(-) diff --git a/mysql-test/main/alter_table_online.result b/mysql-test/main/alter_table_online.result index 2e3de2c0635..719d6fe1751 100644 --- a/mysql-test/main/alter_table_online.result +++ b/mysql-test/main/alter_table_online.result @@ -253,19 +253,19 @@ Feature_delay_key_write 0 alter online table t1 delay_key_write=1; show status like 'Feature_delay_key_write'; Variable_name Value +Feature_delay_key_write 0 +flush tables; +insert t1 values (1,2),(2,3),(3,4); +show status like 'Feature_delay_key_write'; +Variable_name Value +Feature_delay_key_write 1 +alter online table t1 delay_key_write=0; +show status like 'Feature_delay_key_write'; +Variable_name Value Feature_delay_key_write 1 flush tables; insert t1 values (1,2),(2,3),(3,4); show status like 'Feature_delay_key_write'; Variable_name Value -Feature_delay_key_write 2 -alter online table t1 delay_key_write=0; -show status like 'Feature_delay_key_write'; -Variable_name Value -Feature_delay_key_write 2 -flush tables; -insert t1 values (1,2),(2,3),(3,4); -show status like 'Feature_delay_key_write'; -Variable_name Value -Feature_delay_key_write 2 +Feature_delay_key_write 1 drop table t1; diff --git a/mysql-test/suite/gcol/r/gcol_bugfixes.result b/mysql-test/suite/gcol/r/gcol_bugfixes.result index 3d19f718287..3a859116cab 100644 --- a/mysql-test/suite/gcol/r/gcol_bugfixes.result +++ b/mysql-test/suite/gcol/r/gcol_bugfixes.result @@ -535,7 +535,6 @@ CREATE TABLE t (a INTEGER) engine=innodb; ALTER TABLE t ADD b INTEGER AS (SUBSTR('','a',1)); Warnings: Warning 1292 Truncated incorrect INTEGER value: 'a' -Warning 1292 Truncated incorrect INTEGER value: 'a' DROP TABLE t; set sql_mode= @save_old_sql_mode; # Bug#21875520 Problems with virtual column indexes diff --git a/mysql-test/suite/gcol/r/innodb_virtual_stats.result b/mysql-test/suite/gcol/r/innodb_virtual_stats.result index 4ef499f932f..c0f595263df 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_stats.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_stats.result @@ -24,6 +24,9 @@ vidxcd n_diff_pfx03 c,d,DB_ROW_ID vidxcd n_leaf_pages Number of leaf pages in the index vidxcd size Number of pages in the index ALTER TABLE t ADD COLUMN e INT GENERATED ALWAYS AS(a+a+b), ADD INDEX idxb (b), ALGORITHM=INPLACE; +select count(*) from t; +count(*) +1 SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; @@ -45,6 +48,9 @@ vidxcd n_diff_pfx03 c,d,DB_ROW_ID vidxcd n_leaf_pages Number of leaf pages in the index vidxcd size Number of pages in the index ALTER TABLE t DROP COLUMN c, DROP INDEX idxa, ALGORITHM=INPLACE; +select count(*) from t; +count(*) +1 SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; @@ -61,6 +67,9 @@ vidxcd n_diff_pfx02 d,DB_ROW_ID vidxcd n_leaf_pages Number of leaf pages in the index vidxcd size Number of pages in the index ALTER TABLE t ADD INDEX vidxe (e), ALGORITHM=INPLACE; +select count(*) from t; +count(*) +1 SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; @@ -81,6 +90,9 @@ vidxe n_diff_pfx02 e,DB_ROW_ID vidxe n_leaf_pages Number of leaf pages in the index vidxe size Number of pages in the index ALTER TABLE t ADD COLUMN f INT GENERATED ALWAYS AS(a + a), ADD INDEX vidxf (f), ALGORITHM=INPLACE; +select count(*) from t; +count(*) +1 SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; diff --git a/mysql-test/suite/gcol/t/innodb_virtual_stats.test b/mysql-test/suite/gcol/t/innodb_virtual_stats.test index 7e3c8f4e00e..69c67af8ed1 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_stats.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_stats.test @@ -20,24 +20,28 @@ FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; ALTER TABLE t ADD COLUMN e INT GENERATED ALWAYS AS(a+a+b), ADD INDEX idxb (b), ALGORITHM=INPLACE; +select count(*) from t; SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; ALTER TABLE t DROP COLUMN c, DROP INDEX idxa, ALGORITHM=INPLACE; +select count(*) from t; SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; ALTER TABLE t ADD INDEX vidxe (e), ALGORITHM=INPLACE; +select count(*) from t; SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats WHERE database_name = 'test' AND table_name = 't'; ALTER TABLE t ADD COLUMN f INT GENERATED ALWAYS AS(a + a), ADD INDEX vidxf (f), ALGORITHM=INPLACE; +select count(*) from t; SELECT index_name, stat_name, stat_description FROM mysql.innodb_index_stats diff --git a/mysql-test/suite/innodb/r/innodb-wl5522.result b/mysql-test/suite/innodb/r/innodb-wl5522.result index 50330b5b164..3cc31a5b831 100644 --- a/mysql-test/suite/innodb/r/innodb-wl5522.result +++ b/mysql-test/suite/innodb/r/innodb-wl5522.result @@ -285,14 +285,16 @@ ERROR HY000: Tablespace has been discarded for table `t1` restore: t1 .ibd and .cfg files ALTER TABLE t1 IMPORT TABLESPACE; ERROR HY000: Schema mismatch (Index x not found in tablespace meta-data file.) +select count(*) from t1; +ERROR HY000: Tablespace has been discarded for table `t1` ALTER TABLE t1 DROP INDEX x; -Warnings: -Warning 1814 Tablespace has been discarded for table `t1` +ALTER TABLE t1 DROP INDEX x, ALGORITHM=copy; +ERROR 42000: Can't DROP INDEX `x`; check that it exists ALTER TABLE t1 ADD INDEX idx(c2); -Warnings: -Warning 1814 Tablespace has been discarded for table `t1` restore: t1 .ibd and .cfg files ALTER TABLE t1 IMPORT TABLESPACE; +Warnings: +Warning 1814 Tablespace has been discarded for table `t1` CHECK TABLE t1; Table Op Msg_type Msg_text test.t1 check status OK diff --git a/mysql-test/suite/innodb/r/instant_alter_crash.result b/mysql-test/suite/innodb/r/instant_alter_crash.result index 528bd9a905a..cfcb24f8bb2 100644 --- a/mysql-test/suite/innodb/r/instant_alter_crash.result +++ b/mysql-test/suite/innodb/r/instant_alter_crash.result @@ -6,16 +6,17 @@ CREATE TABLE t1(id INT PRIMARY KEY, c2 INT UNIQUE) ENGINE=InnoDB ROW_FORMAT=REDUNDANT; CREATE TABLE t2 LIKE t1; INSERT INTO t1 VALUES(0,2); -BEGIN; INSERT INTO t2 VALUES(2,1); ALTER TABLE t2 ADD COLUMN (c3 TEXT NOT NULL DEFAULT 'De finibus bonorum'); +BEGIN; +INSERT INTO t2 VALUES(3,4,'accusantium doloremque laudantium'); connect ddl, localhost, root; SET DEBUG_SYNC='innodb_alter_inplace_before_commit SIGNAL ddl WAIT_FOR ever'; ALTER TABLE t1 ADD COLUMN (c3 TEXT NOT NULL DEFAULT ' et malorum'); connection default; SET DEBUG_SYNC='now WAIT_FOR ddl'; SET GLOBAL innodb_flush_log_at_trx_commit=1; -INSERT INTO t2 VALUES(3,4,'accusantium doloremque laudantium'); +COMMIT; # Kill the server disconnect ddl; # restart diff --git a/mysql-test/suite/innodb/r/monitor.result b/mysql-test/suite/innodb/r/monitor.result index 4aea9fb5b0b..9b2783e9d08 100644 --- a/mysql-test/suite/innodb/r/monitor.result +++ b/mysql-test/suite/innodb/r/monitor.result @@ -416,7 +416,7 @@ max_count_reset, min_count_reset, count_reset, status from information_schema.innodb_metrics where name like "metadata%"; name max_count min_count count max_count_reset min_count_reset count_reset status -metadata_table_handles_opened 2 NULL 2 2 NULL 2 enabled +metadata_table_handles_opened 1 NULL 1 1 NULL 1 enabled metadata_table_handles_closed 1 NULL 1 1 NULL 1 enabled metadata_table_reference_count NULL NULL 0 NULL NULL 0 disabled set global innodb_monitor_disable = module_metadata; @@ -426,7 +426,7 @@ max_count_reset, min_count_reset, count_reset, status from information_schema.innodb_metrics where name like "metadata%"; name max_count min_count count max_count_reset min_count_reset count_reset status -metadata_table_handles_opened 2 NULL 2 NULL NULL 0 disabled +metadata_table_handles_opened 1 NULL 1 NULL NULL 0 disabled metadata_table_handles_closed 1 NULL 1 NULL NULL 0 disabled metadata_table_reference_count NULL NULL 0 NULL NULL 0 disabled set global innodb_monitor_reset_all = module_metadata; diff --git a/mysql-test/suite/innodb/t/innodb-wl5522.test b/mysql-test/suite/innodb/t/innodb-wl5522.test index 744768a1d6c..e17ccf24fa6 100644 --- a/mysql-test/suite/innodb/t/innodb-wl5522.test +++ b/mysql-test/suite/innodb/t/innodb-wl5522.test @@ -312,8 +312,11 @@ EOF # This is really a name mismatch error, need better error codes. -- error ER_TABLE_SCHEMA_MISMATCH ALTER TABLE t1 IMPORT TABLESPACE; - +--error ER_TABLESPACE_DISCARDED +select count(*) from t1; ALTER TABLE t1 DROP INDEX x; +--error ER_CANT_DROP_FIELD_OR_KEY +ALTER TABLE t1 DROP INDEX x, ALGORITHM=copy; ALTER TABLE t1 ADD INDEX idx(c2); perl; diff --git a/mysql-test/suite/innodb/t/instant_alter_crash.test b/mysql-test/suite/innodb/t/instant_alter_crash.test index d16ee6c929a..13ff292d9ff 100644 --- a/mysql-test/suite/innodb/t/instant_alter_crash.test +++ b/mysql-test/suite/innodb/t/instant_alter_crash.test @@ -17,9 +17,10 @@ CREATE TABLE t1(id INT PRIMARY KEY, c2 INT UNIQUE) ENGINE=InnoDB ROW_FORMAT=REDUNDANT; CREATE TABLE t2 LIKE t1; INSERT INTO t1 VALUES(0,2); -BEGIN; INSERT INTO t2 VALUES(2,1); ALTER TABLE t2 ADD COLUMN (c3 TEXT NOT NULL DEFAULT 'De finibus bonorum'); +BEGIN; +INSERT INTO t2 VALUES(3,4,'accusantium doloremque laudantium'); connect ddl, localhost, root; SET DEBUG_SYNC='innodb_alter_inplace_before_commit SIGNAL ddl WAIT_FOR ever'; @@ -29,7 +30,7 @@ ALTER TABLE t1 ADD COLUMN (c3 TEXT NOT NULL DEFAULT ' et malorum'); connection default; SET DEBUG_SYNC='now WAIT_FOR ddl'; SET GLOBAL innodb_flush_log_at_trx_commit=1; -INSERT INTO t2 VALUES(3,4,'accusantium doloremque laudantium'); +COMMIT; --source include/kill_mysqld.inc disconnect ddl; diff --git a/mysql-test/suite/innodb_zip/r/wl5522_debug_zip.result b/mysql-test/suite/innodb_zip/r/wl5522_debug_zip.result index b3a4ad2b0ba..864ffba2117 100644 --- a/mysql-test/suite/innodb_zip/r/wl5522_debug_zip.result +++ b/mysql-test/suite/innodb_zip/r/wl5522_debug_zip.result @@ -503,8 +503,6 @@ SELECT COUNT(*) FROM test_wl5522.t1; ERROR HY000: Tablespace has been discarded for table `t1` SET SESSION debug_dbug="+d,ib_import_create_index_failure_1"; ALTER TABLE test_wl5522.t1 ADD INDEX idx(c1); -Warnings: -Warning 1814 Tablespace has been discarded for table `t1` SET SESSION debug_dbug=@saved_debug_dbug; DROP TABLE test_wl5522.t1; unlink: t1.ibd diff --git a/mysql-test/suite/innodb_zip/r/wl5522_zip.result b/mysql-test/suite/innodb_zip/r/wl5522_zip.result index 03bfd2cac7a..9d6bcb3629f 100644 --- a/mysql-test/suite/innodb_zip/r/wl5522_zip.result +++ b/mysql-test/suite/innodb_zip/r/wl5522_zip.result @@ -265,13 +265,13 @@ restore: t1 .ibd and .cfg files ALTER TABLE t1 IMPORT TABLESPACE; ERROR HY000: Schema mismatch (Index x not found in tablespace meta-data file.) ALTER TABLE t1 DROP INDEX x; -Warnings: -Warning 1814 Tablespace has been discarded for table `t1` ALTER TABLE t1 ADD INDEX idx(c2); Warnings: Warning 1814 Tablespace has been discarded for table `t1` restore: t1 .ibd and .cfg files ALTER TABLE t1 IMPORT TABLESPACE; +Warnings: +Warning 1814 Tablespace has been discarded for table `t1` CHECK TABLE t1; Table Op Msg_type Msg_text test.t1 check status OK diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 9dcd57a5f82..9ae2a7077c5 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -10202,19 +10202,6 @@ end: } -void ha_partition::notify_table_changed() -{ - handler **file; - - DBUG_ENTER("ha_partition::notify_table_changed"); - - for (file= m_file; *file; file++) - (*file)->ha_notify_table_changed(); - - DBUG_VOID_RETURN; -} - - uint ha_partition::min_of_the_max_uint( uint (handler::*operator_func)(void) const) const { diff --git a/sql/ha_partition.h b/sql/ha_partition.h index 5913b3d2aa8..caeff8a25aa 100644 --- a/sql/ha_partition.h +++ b/sql/ha_partition.h @@ -1388,8 +1388,6 @@ public: virtual bool commit_inplace_alter_table(TABLE *altered_table, Alter_inplace_info *ha_alter_info, bool commit); - virtual void notify_table_changed(); - /* ------------------------------------------------------------------------- MODULE tablespace support diff --git a/sql/handler.h b/sql/handler.h index 2d61249c58d..fb66814c594 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1642,6 +1642,14 @@ struct handlerton int (*discover_table_structure)(handlerton *hton, THD* thd, TABLE_SHARE *share, HA_CREATE_INFO *info); + /* + Notify the storage engine that the definition of the table (and the .frm + file) has changed. Returns 0 if ok. + */ + int (*notify_tabledef_changed)(handlerton *hton, LEX_CSTRING *db, + LEX_CSTRING *table_name, LEX_CUSTRING *frm, + LEX_CUSTRING *org_tabledef_version); + /* System Versioning */ @@ -4269,7 +4277,7 @@ public: *) Update SQL-layer data-dictionary by installing .FRM file for the new version of the table. *) Inform the storage engine about this change by calling the - handler::ha_notify_table_changed() method. + hton::notify_table_changed() *) Destroy the Alter_inplace_info and handler_ctx objects. */ @@ -4336,16 +4344,6 @@ public: bool commit); - /** - Public function wrapping the actual handler call. - @see notify_table_changed() - */ - void ha_notify_table_changed() - { - notify_table_changed(); - } - - protected: /** Allows the storage engine to update internal structures with concurrent @@ -4444,14 +4442,6 @@ protected: return false; } - - /** - Notify the storage engine that the table structure (.FRM) has been updated. - - @note No errors are allowed during notify_table_changed(). - */ - virtual void notify_table_changed() { } - public: /* End of On-line/in-place ALTER TABLE interface. */ diff --git a/sql/sql_table.cc b/sql/sql_table.cc index b5c6eb05f38..6de1116772c 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -7499,11 +7499,10 @@ static bool mysql_inplace_alter_table(THD *thd, { Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN | MYSQL_OPEN_IGNORE_KILLED); handlerton *db_type= table->s->db_type(); - MDL_ticket *mdl_ticket= table->mdl_ticket; Alter_info *alter_info= ha_alter_info->alter_info; bool reopen_tables= false; bool res; - + handlerton *hton; DBUG_ENTER("mysql_inplace_alter_table"); /* Downgrade DDL lock while we are waiting for exclusive lock below */ @@ -7709,6 +7708,28 @@ static bool mysql_inplace_alter_table(THD *thd, } } + /* Notify the engine that the table definition has changed */ + + hton= table->file->ht; + if (hton->notify_tabledef_changed) + { + char db_buff[FN_REFLEN], table_buff[FN_REFLEN]; + LEX_CSTRING tmp_db, tmp_table; + tmp_db.str= db_buff; + tmp_table.str= table_buff; + tmp_db.length= tablename_to_filename(table_list->db.str, + db_buff, sizeof(db_buff)); + tmp_table.length= tablename_to_filename(table_list->table_name.str, + table_buff, sizeof(table_buff)); + if ((hton->notify_tabledef_changed)(hton, &tmp_db, &tmp_table, + table->s->frm_image, + &table->s->tabledef_version)) + { + my_error(HA_ERR_INCOMPATIBLE_DEFINITION, MYF(0)); + DBUG_RETURN(true); + } + } + close_all_tables_for_name(thd, table->s, alter_ctx->is_table_renamed() ? HA_EXTRA_PREPARE_FOR_RENAME : @@ -7733,24 +7754,6 @@ static bool mysql_inplace_alter_table(THD *thd, DBUG_RETURN(true); } - table_list->mdl_request.ticket= mdl_ticket; - if (open_table(thd, table_list, &ot_ctx)) - DBUG_RETURN(true); - - /* - Tell the handler that the changed frm is on disk and table - has been re-opened - */ - table_list->table->file->ha_notify_table_changed(); - - /* - We might be going to reopen table down on the road, so we have to - restore state of the TABLE object which we used for obtaining of - handler object to make it usable for later reopening. - */ - close_thread_table(thd, &thd->open_tables); - table_list->table= NULL; - // Rename altered table if requested. if (alter_ctx->is_table_renamed()) { diff --git a/storage/mroonga/ha_mroonga.cpp b/storage/mroonga/ha_mroonga.cpp index ca474ede35b..b3e26245d6d 100644 --- a/storage/mroonga/ha_mroonga.cpp +++ b/storage/mroonga/ha_mroonga.cpp @@ -15514,34 +15514,6 @@ bool ha_mroonga::commit_inplace_alter_table( } DBUG_RETURN(result); } - -void ha_mroonga::wrapper_notify_table_changed() -{ - MRN_DBUG_ENTER_METHOD(); - MRN_SET_WRAP_SHARE_KEY(share, table->s); - MRN_SET_WRAP_TABLE_KEY(this, table); - wrap_handler->ha_notify_table_changed(); - MRN_SET_BASE_SHARE_KEY(share, table->s); - MRN_SET_BASE_TABLE_KEY(this, table); - DBUG_VOID_RETURN; -} - -void ha_mroonga::storage_notify_table_changed() -{ - MRN_DBUG_ENTER_METHOD(); - DBUG_VOID_RETURN; -} - -void ha_mroonga::notify_table_changed() -{ - MRN_DBUG_ENTER_METHOD(); - if (share->wrapper_mode) { - wrapper_notify_table_changed(); - } else { - storage_notify_table_changed(); - } - DBUG_VOID_RETURN; -} #else alter_table_operations ha_mroonga::wrapper_alter_table_flags(alter_table_operations flags) { diff --git a/storage/mroonga/ha_mroonga.hpp b/storage/mroonga/ha_mroonga.hpp index 68c79703903..6a66ca89976 100644 --- a/storage/mroonga/ha_mroonga.hpp +++ b/storage/mroonga/ha_mroonga.hpp @@ -641,7 +641,6 @@ protected: bool commit_inplace_alter_table(TABLE *altered_table, Alter_inplace_info *ha_alter_info, bool commit); - void notify_table_changed(); #endif private: @@ -1201,8 +1200,6 @@ private: bool storage_commit_inplace_alter_table(TABLE *altered_table, Alter_inplace_info *ha_alter_info, bool commit); - void wrapper_notify_table_changed(); - void storage_notify_table_changed(); #else alter_table_operations wrapper_alter_table_flags(alter_table_operations flags); alter_table_operations storage_alter_table_flags(alter_table_operations flags);