From 880baedcf6f2c1c342dc59e8a0e813c0ea728264 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 2 Apr 2021 18:13:46 +0300 Subject: [PATCH 1/9] MDEV-17913 Encrypted transactional Aria tables remain corrupt after crash recovery, automatic repairment does not work This was because of a wrong test in encryption code that wrote random numbers over the LSN for pages for transactional Aria tables during repair. The effect was that after an ALTER TABLE ENABLE KEYS of a encrypted recovery of the tables would not work. The test cases will be pushed into 10.5 as it requires of several changes to check table that safer not to backport. --- storage/maria/ma_crypt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/maria/ma_crypt.c b/storage/maria/ma_crypt.c index 95b84d38221..564edb4bbe8 100644 --- a/storage/maria/ma_crypt.c +++ b/storage/maria/ma_crypt.c @@ -268,7 +268,7 @@ static my_bool ma_crypt_data_pre_write_hook(PAGECACHE_IO_HOOK_ARGS *args) return 1; } - if (!share->now_transactional) + if (!share->base.born_transactional) { /* store a random number instead of LSN (for counter block) */ store_rand_lsn(args->page); @@ -392,7 +392,7 @@ static my_bool ma_crypt_index_pre_write_hook(PAGECACHE_IO_HOOK_ARGS *args) return 1; } - if (!share->now_transactional) + if (!share->base.born_transactional) { /* store a random number instead of LSN (for counter block) */ store_rand_lsn(args->page); From 915983e1cc3a0a356a0adfef38fc7ad87264bd9f Mon Sep 17 00:00:00 2001 From: Daniele Sciascia Date: Wed, 24 Mar 2021 10:55:27 +0100 Subject: [PATCH 2/9] MDEV-25226 Assertion when wsrep_on set OFF with SR transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch makes the following changes around variable wsrep_on: 1) Variable wsrep_on can no longer be updated from a session that has an active transaction running. The original behavior allowed cases like this: BEGIN; INSERT INTO t1 VALUES (1); SET SESSION wsrep_on = OFF; INSERT INTO t1 VALUES (2); COMMIT; With regular transactions this would result in no replication events (not even value 1). With streaming replication it would be unnecessarily complex to achieve the same behavior. In the above example, it would be possible for value 1 to be already replicated if it happened to fill a separate fragment, while value 2 wouldn't. 2) Global variable wsrep_on no longer affects current sessions, only subsequent ones. This is to avoid a similar case to the above, just using just by using global wsrep_on instead session wsrep_on: --connection conn_1 BEGIN; INSERT INTO t1 VALUES(1); --connection conn_2 SET GLOBAL wsrep_on = OFF; --connection conn_1 INSERT INTO t1 VALUES(2); COMMIT; The above example results in the transaction to be replicated, as global wsrep_on will only affect the session wsrep_on of new connections. Reviewed-by: Jan Lindström --- .../galera/r/galera_var_wsrep_on_off.result | 46 +++++++++++++++ .../galera/t/galera_var_wsrep_on_off.test | 57 +++++++++++++++++++ .../suite/galera_sr/r/MDEV-25226.result | 24 ++++++++ mysql-test/suite/galera_sr/t/MDEV-25226.test | 33 +++++++++++ sql/mysqld.cc | 13 +---- sql/wsrep_mysqld.cc | 12 ++++ sql/wsrep_mysqld.h | 7 ++- sql/wsrep_trans_observer.h | 2 +- sql/wsrep_var.cc | 14 ++++- sql/wsrep_var.h | 1 + 10 files changed, 190 insertions(+), 19 deletions(-) create mode 100644 mysql-test/suite/galera_sr/r/MDEV-25226.result create mode 100644 mysql-test/suite/galera_sr/t/MDEV-25226.test diff --git a/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result b/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result index 5323bc9bf60..b3096afd387 100644 --- a/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result +++ b/mysql-test/suite/galera/r/galera_var_wsrep_on_off.result @@ -22,3 +22,49 @@ SELECT COUNT(*) = 1 FROM t1 WHERE f1 = 3; COUNT(*) = 1 1 DROP TABLE t1; +START TRANSACTION; +SET SESSION wsrep_on=OFF; +ERROR 25000: You are not allowed to execute this command in a transaction +SET GLOBAL wsrep_on=OFF; +ERROR 25000: You are not allowed to execute this command in a transaction +COMMIT; +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY); +START TRANSACTION; +INSERT INTO t1 VALUES (1); +connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1;; +connection node_1a; +SET GLOBAL wsrep_on = OFF; +connection node_1; +SHOW SESSION VARIABLES LIKE 'wsrep_on'; +Variable_name Value +wsrep_on ON +SHOW GLOBAL VARIABLES LIKE 'wsrep_on'; +Variable_name Value +wsrep_on OFF +INSERT INTO t1 VALUES (2); +COMMIT; +connection node_2; +SET SESSION wsrep_sync_wait = 15; +SELECT * FROM t1; +f1 +1 +2 +connection node_1a; +SET GLOBAL wsrep_on = ON; +DROP TABLE t1; +connection node_1; +SET GLOBAL wsrep_on = OFF; +connect node_1b, 127.0.0.1, root, , test, $NODE_MYPORT_1;; +connection node_1b; +SHOW SESSION VARIABLES LIKE 'wsrep_on'; +Variable_name Value +wsrep_on OFF +SHOW GLOBAL VARIABLES LIKE 'wsrep_on'; +Variable_name Value +wsrep_on OFF +CREATE TABLE t2 (f1 INTEGER); +DROP TABLE t2; +SET GLOBAL wsrep_on = ON; +SHOW SESSION VARIABLES LIKE 'wsrep_on'; +Variable_name Value +wsrep_on ON diff --git a/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test b/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test index 783b78792e6..1a48abc25eb 100644 --- a/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test +++ b/mysql-test/suite/galera/t/galera_var_wsrep_on_off.test @@ -30,3 +30,60 @@ SELECT COUNT(*) = 1 FROM t1 WHERE f1 = 3; DROP TABLE t1; + +# +# Test that variable wsrep_on cannot be changed while in +# active transaction. +# + +START TRANSACTION; +--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION +SET SESSION wsrep_on=OFF; +--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION +SET GLOBAL wsrep_on=OFF; +COMMIT; + + +# +# Test that @@global.wsrep_on does not affect the value of +# @@session.wsrep_on of current sessions +# + +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY); +START TRANSACTION; +INSERT INTO t1 VALUES (1); + +--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1; +--connection node_1a +SET GLOBAL wsrep_on = OFF; + +--connection node_1 +SHOW SESSION VARIABLES LIKE 'wsrep_on'; +SHOW GLOBAL VARIABLES LIKE 'wsrep_on'; +INSERT INTO t1 VALUES (2); +COMMIT; + +--connection node_2 +SET SESSION wsrep_sync_wait = 15; +SELECT * FROM t1; + +--connection node_1a +SET GLOBAL wsrep_on = ON; +DROP TABLE t1; + + +# +# New connections inherit @@session.wsrep_on from @@global.wsrep_on +# +--connection node_1 +SET GLOBAL wsrep_on = OFF; + +--connect node_1b, 127.0.0.1, root, , test, $NODE_MYPORT_1; +--connection node_1b +SHOW SESSION VARIABLES LIKE 'wsrep_on'; +SHOW GLOBAL VARIABLES LIKE 'wsrep_on'; +CREATE TABLE t2 (f1 INTEGER); +DROP TABLE t2; + +SET GLOBAL wsrep_on = ON; +SHOW SESSION VARIABLES LIKE 'wsrep_on'; diff --git a/mysql-test/suite/galera_sr/r/MDEV-25226.result b/mysql-test/suite/galera_sr/r/MDEV-25226.result new file mode 100644 index 00000000000..4699023562d --- /dev/null +++ b/mysql-test/suite/galera_sr/r/MDEV-25226.result @@ -0,0 +1,24 @@ +connection node_2; +connection node_1; +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY); +SET SESSION wsrep_trx_fragment_size=1; +START TRANSACTION; +INSERT INTO t1 VALUES(1); +SET SESSION wsrep_on=OFF; +ERROR 25000: You are not allowed to execute this command in a transaction +SET GLOBAL wsrep_on=OFF; +ERROR 25000: You are not allowed to execute this command in a transaction +INSERT INTO t1 VALUES(2); +COMMIT; +connection node_1; +SELECT * FROM t1; +f1 +1 +2 +connection node_2; +SELECT * FROM t1; +f1 +1 +2 +connection node_1; +DROP TABLE t1; diff --git a/mysql-test/suite/galera_sr/t/MDEV-25226.test b/mysql-test/suite/galera_sr/t/MDEV-25226.test new file mode 100644 index 00000000000..3d19a0b9675 --- /dev/null +++ b/mysql-test/suite/galera_sr/t/MDEV-25226.test @@ -0,0 +1,33 @@ +# +# MDEV-25226 - Test the case the where wsrep_on is set OFF +# on a transaction that has already replicated a fragment. +# +# This would cause: Assertion `transaction_.active() == false || +# (transaction_.state() == wsrep::transaction::s_executing || +# transaction_.state() == wsrep::transaction::s_prepared || +# transaction_.state() == wsrep::transaction::s_aborted || +# transaction_.state() == wsrep::transaction::s_must_abort)' +# + +--source include/galera_cluster.inc + +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY); + +SET SESSION wsrep_trx_fragment_size=1; +START TRANSACTION; +INSERT INTO t1 VALUES(1); +--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION +SET SESSION wsrep_on=OFF; +--error ER_CANT_DO_THIS_DURING_AN_TRANSACTION +SET GLOBAL wsrep_on=OFF; +INSERT INTO t1 VALUES(2); +COMMIT; + +--connection node_1 +SELECT * FROM t1; + +--connection node_2 +SELECT * FROM t1; + +--connection node_1 +DROP TABLE t1; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index d0081034c0c..c536cdef504 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -1131,14 +1131,6 @@ PSI_file_key key_file_binlog_state; PSI_statement_info stmt_info_new_packet; #endif -#ifdef WITH_WSREP -/** Whether the Galera write-set replication is enabled. A cached copy of -global_system_variables.wsrep_on && wsrep_provider && - strcmp(wsrep_provider, WSREP_NONE) -*/ -bool WSREP_ON_; -#endif /* WITH_WSREP */ - #ifndef EMBEDDED_LIBRARY void net_before_header_psi(struct st_net *net, void *thd, size_t /* unused: count */) { @@ -5710,10 +5702,7 @@ int mysqld_main(int argc, char **argv) } #ifdef WITH_WSREP - WSREP_ON_= (global_system_variables.wsrep_on && - wsrep_provider && - strcmp(wsrep_provider, WSREP_NONE)); - + wsrep_set_wsrep_on(); if (WSREP_ON && wsrep_check_opts()) unireg_abort(1); #endif diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc index c3c4e4a0bf4..0338d7ad054 100644 --- a/sql/wsrep_mysqld.cc +++ b/sql/wsrep_mysqld.cc @@ -131,6 +131,18 @@ uint wsrep_ignore_apply_errors= 0; * End configuration options */ +/* + * Cached variables + */ + +// Whether the Galera write-set replication provider is set +// wsrep_provider && strcmp(wsrep_provider, WSREP_NONE) +bool WSREP_PROVIDER_EXISTS_; + +// Whether the Galera write-set replication is enabled +// global_system_variables.wsrep_on && WSREP_PROVIDER_EXISTS_ +bool WSREP_ON_; + /* * Other wsrep global variables. */ diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h index 02996012156..0b245ea11cb 100644 --- a/sql/wsrep_mysqld.h +++ b/sql/wsrep_mysqld.h @@ -20,6 +20,7 @@ #ifdef WITH_WSREP extern bool WSREP_ON_; +extern bool WSREP_PROVIDER_EXISTS_; #include #include "mysql/service_wsrep.h" @@ -221,7 +222,8 @@ extern wsrep_seqno_t wsrep_locked_seqno; /* use xxxxxx_NNULL macros when thd pointer is guaranteed to be non-null to * avoid compiler warnings (GCC 6 and later) */ -#define WSREP_NNULL(thd) (WSREP_ON && thd->variables.wsrep_on) +#define WSREP_NNULL(thd) \ + (WSREP_PROVIDER_EXISTS_ && thd->variables.wsrep_on) #define WSREP(thd) \ (thd && WSREP_NNULL(thd)) @@ -277,8 +279,7 @@ void WSREP_LOG(void (*fun)(const char* fmt, ...), const char* fmt, ...); WSREP_INFO("context: %s:%d", __FILE__, __LINE__); \ } -#define WSREP_PROVIDER_EXISTS \ - (wsrep_provider && strncasecmp(wsrep_provider, WSREP_NONE, FN_REFLEN)) +#define WSREP_PROVIDER_EXISTS (WSREP_PROVIDER_EXISTS_) static inline bool wsrep_cluster_address_exists() { diff --git a/sql/wsrep_trans_observer.h b/sql/wsrep_trans_observer.h index dbe710a0256..366cbef50d3 100644 --- a/sql/wsrep_trans_observer.h +++ b/sql/wsrep_trans_observer.h @@ -407,7 +407,7 @@ static inline void wsrep_after_apply(THD* thd) static inline void wsrep_open(THD* thd) { DBUG_ENTER("wsrep_open"); - if (WSREP(thd)) + if (WSREP_PROVIDER_EXISTS) { thd->wsrep_cs().open(wsrep::client_id(thd->thread_id)); thd->wsrep_cs().debug_log_level(wsrep_debug); diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc index 815a65ff240..01e9d022cfb 100644 --- a/sql/wsrep_var.cc +++ b/sql/wsrep_var.cc @@ -88,10 +88,11 @@ static bool refresh_provider_options() } } -static void wsrep_set_wsrep_on() +void wsrep_set_wsrep_on() { - WSREP_ON_= global_system_variables.wsrep_on && wsrep_provider && - strcmp(wsrep_provider, WSREP_NONE); + WSREP_PROVIDER_EXISTS_= wsrep_provider && + strncasecmp(wsrep_provider, WSREP_NONE, FN_REFLEN); + WSREP_ON_= global_system_variables.wsrep_on && WSREP_PROVIDER_EXISTS_; } /* This is intentionally declared as a weak global symbol, so that @@ -146,6 +147,13 @@ bool wsrep_on_check(sys_var *self, THD* thd, set_var* var) " innodb_lock_schedule_algorithm=FCFS and restart.", MYF(0)); return true; } + + if (thd->in_active_multi_stmt_transaction()) + { + my_error(ER_CANT_DO_THIS_DURING_AN_TRANSACTION, MYF(0)); + return true; + } + return false; } diff --git a/sql/wsrep_var.h b/sql/wsrep_var.h index 481df02f2d5..b1b2932cdfe 100644 --- a/sql/wsrep_var.h +++ b/sql/wsrep_var.h @@ -35,6 +35,7 @@ class set_var; class THD; int wsrep_init_vars(); +void wsrep_set_wsrep_on(); #define CHECK_ARGS (sys_var *self, THD* thd, set_var *var) #define UPDATE_ARGS (sys_var *self, THD* thd, enum_var_type type) From f8488370d6e4dc38643545260b17755bb8be8a9d Mon Sep 17 00:00:00 2001 From: mkaruza Date: Wed, 31 Mar 2021 14:59:50 +0200 Subject: [PATCH 3/9] MDEV-24956: ALTER TABLE not replicated with Galera in MariaDB 10.5.9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WSREP_CLIENT` is used as condition for starting ALTER/OPTIMIZE/REPAIR TOI. Using this condition async replicated affected DDL's will not be replicated. Fixed by removing this condition. Reviewed-by: Jan Lindström --- .../galera_3nodes/r/galera_2_cluster.result | 89 +++++++++++ .../galera_3nodes/t/galera_2_cluster.cnf | 25 +++ .../galera_3nodes/t/galera_2_cluster.test | 148 ++++++++++++++++++ sql/sql_admin.cc | 11 +- sql/sql_alter.cc | 2 +- 5 files changed, 269 insertions(+), 6 deletions(-) create mode 100644 mysql-test/suite/galera_3nodes/r/galera_2_cluster.result create mode 100644 mysql-test/suite/galera_3nodes/t/galera_2_cluster.cnf create mode 100644 mysql-test/suite/galera_3nodes/t/galera_2_cluster.test diff --git a/mysql-test/suite/galera_3nodes/r/galera_2_cluster.result b/mysql-test/suite/galera_3nodes/r/galera_2_cluster.result new file mode 100644 index 00000000000..87898891f9b --- /dev/null +++ b/mysql-test/suite/galera_3nodes/r/galera_2_cluster.result @@ -0,0 +1,89 @@ +connection node_2; +connection node_1; +connect node_5, 127.0.0.1, root, , test, $NODE_MYPORT_5; +connect node_4, 127.0.0.1, root, , test, $NODE_MYPORT_4; +connection node_4; +CHANGE MASTER TO master_host='127.0.0.1', master_user='root', master_port=NODE_MYPORT_1, master_use_gtid=current_pos;; +START SLAVE; +include/wait_for_slave_to_start.inc +connection node_1; +CREATE TABLE t1(c1 INTEGER NOT NULL AUTO_INCREMENT PRIMARY KEY, c2 INTEGER); +INSERT INTO t1(c2) VALUES(1); +connect node_3, 127.0.0.1, root, , test, $NODE_MYPORT_3; +connection node_3; +SELECT COUNT(*) = 1 FROM t1; +COUNT(*) = 1 +1 +connection node_1; +include/save_master_gtid.inc +connection node_4; +include/sync_with_master_gtid.inc +SELECT COUNT(*) = 1 FROM t1; +COUNT(*) = 1 +1 +connect node_6, 127.0.0.1, root, , test, $NODE_MYPORT_6; +connection node_6; +SELECT COUNT(*) = 1 FROM t1; +COUNT(*) = 1 +1 +connection node_2; +ALTER TABLE t1 ADD COLUMN t3 INTEGER; +Node 2 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; +COUNT(*) = 3 +1 +connection node_3; +Node 3 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; +COUNT(*) = 3 +1 +connection node_1; +include/save_master_gtid.inc +connection node_4; +include/sync_with_master_gtid.inc +Node 4 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; +COUNT(*) = 3 +1 +connection node_6; +Node 6 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; +COUNT(*) = 3 +1 +connection node_2; +OPTIMIZE TABLE t1; +Table Op Msg_type Msg_text +test.t1 optimize note Table does not support optimize, doing recreate + analyze instead +test.t1 optimize status OK +connection node_1; +connection node_4; +connection node_6; +connection node_1; +DROP TABLE t1; +connection node_4; +STOP SLAVE; +RESET SLAVE; +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; +SET GLOBAL GTID_SLAVE_POS=""; +connection node_1; +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; +connection node_2; +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; +connection node_3; +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; +connection node_5; +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; +connection node_6; +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; diff --git a/mysql-test/suite/galera_3nodes/t/galera_2_cluster.cnf b/mysql-test/suite/galera_3nodes/t/galera_2_cluster.cnf new file mode 100644 index 00000000000..3889a4f4fdd --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/galera_2_cluster.cnf @@ -0,0 +1,25 @@ +!include ../galera_2x3nodes.cnf + +[mysqld.1] +wsrep_gtid_domain_id=1 +server-id=11 + +[mysqld.2] +wsrep_gtid_domain_id=1 +server-id=12 + +[mysqld.3] +wsrep_gtid_domain_id=1 +server-id=13 + +[mysqld.4] +wsrep_gtid_domain_id=2 +server-id=21 + +[mysqld.5] +wsrep_gtid_domain_id=2 +server-id=22 + +[mysqld.6] +wsrep_gtid_domain_id=2 +server-id=23 \ No newline at end of file diff --git a/mysql-test/suite/galera_3nodes/t/galera_2_cluster.test b/mysql-test/suite/galera_3nodes/t/galera_2_cluster.test new file mode 100644 index 00000000000..8a9a74a7252 --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/galera_2_cluster.test @@ -0,0 +1,148 @@ +# +# This test creates 2x3 nodes galera cluster. +# +# A(1) <-> B(2) <-> C(3) {Galera cluster 1} +# | {Circular Async replication} +# D(4) <-> E(5) <-> F(6) {Galera cluster 2} +# + +--source include/big_test.inc +--source include/galera_cluster.inc +--source include/have_innodb.inc + +--connect node_5, 127.0.0.1, root, , test, $NODE_MYPORT_5 + +--connect node_4, 127.0.0.1, root, , test, $NODE_MYPORT_4 +--connection node_4 + +--replace_result $NODE_MYPORT_1 NODE_MYPORT_1 +--eval CHANGE MASTER TO master_host='127.0.0.1', master_user='root', master_port=$NODE_MYPORT_1, master_use_gtid=current_pos; +START SLAVE; +--source include/wait_for_slave_to_start.inc + +# +# CREATE TABLE & INSERT +# + +--connection node_1 + +CREATE TABLE t1(c1 INTEGER NOT NULL AUTO_INCREMENT PRIMARY KEY, c2 INTEGER); +INSERT INTO t1(c2) VALUES(1); + +--connect node_3, 127.0.0.1, root, , test, $NODE_MYPORT_3 +--connection node_3 + +SELECT COUNT(*) = 1 FROM t1; + +--connection node_1 +--source include/save_master_gtid.inc + +--connection node_4 +--source include/sync_with_master_gtid.inc + +SELECT COUNT(*) = 1 FROM t1; + +--connect node_6, 127.0.0.1, root, , test, $NODE_MYPORT_6 +--connection node_6 + +SELECT COUNT(*) = 1 FROM t1; + +# +# ALTER TABLE +# + +--connection node_2 + +ALTER TABLE t1 ADD COLUMN t3 INTEGER; +--echo Node 2 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; + +--connection node_3 + +--echo Node 3 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; + +--connection node_1 +--source include/save_master_gtid.inc + +--connection node_4 +--source include/sync_with_master_gtid.inc + +--echo Node 4 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; + +--connection node_6 + +--echo Node 6 column number AFTER ALTER +SELECT COUNT(*) = 3 FROM information_schema.columns WHERE table_name ='t1'; + +# +# OPTIMIZE TABLE +# + +--connection node_2 + +--let $wsrep_last_committed_before = `SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME = 'wsrep_last_committed'` +OPTIMIZE TABLE t1; + +--connection node_1 + +--let $wait_condition = SELECT VARIABLE_VALUE >= $wsrep_last_committed_before + 1 FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME = 'wsrep_last_committed' +--source include/wait_condition.inc + +--connection node_4 + +--let $wait_condition = SELECT VARIABLE_VALUE >= $wsrep_last_committed_before + 1 FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME = 'wsrep_last_committed' +--source include/wait_condition.inc + +--connection node_6 + +--let $wait_condition = SELECT VARIABLE_VALUE >= $wsrep_last_committed_before + 1 FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME = 'wsrep_last_committed' +--source include/wait_condition.inc + +# +# Cleanup +# + +--connection node_1 + +DROP TABLE t1; + +--connection node_4 + +STOP SLAVE; +RESET SLAVE; +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; +SET GLOBAL GTID_SLAVE_POS=""; + +--connection node_1 + +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; + +--connection node_2 + +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; + +--connection node_3 + +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; + +--connection node_5 + +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; + +--connection node_6 + +SET GLOBAL wsrep_on = OFF; +RESET MASTER; +SET GLOBAL wsrep_on = ON; diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index e59dffc10aa..a96eb58809b 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -447,8 +447,6 @@ dbug_err: */ static bool wsrep_toi_replication(THD *thd, TABLE_LIST *tables) { - if (!WSREP(thd) || !WSREP_CLIENT(thd)) return false; - LEX *lex= thd->lex; /* only handle OPTIMIZE and REPAIR here */ switch (lex->sql_command) @@ -549,10 +547,13 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, for (table= tables; table; table= table->next_local) table->table= NULL; #ifdef WITH_WSREP - if (wsrep_toi_replication(thd, tables)) + if (WSREP(thd)) { - WSREP_INFO("wsrep TOI replication of has failed, skipping OPTIMIZE"); - goto err; + if(wsrep_toi_replication(thd, tables)) + { + WSREP_INFO("wsrep TOI replication of has failed."); + goto err; + } } #endif /* WITH_WSREP */ diff --git a/sql/sql_alter.cc b/sql/sql_alter.cc index 2bbc8169df2..f1a67e7d968 100644 --- a/sql/sql_alter.cc +++ b/sql/sql_alter.cc @@ -471,7 +471,7 @@ bool Sql_cmd_alter_table::execute(THD *thd) if (check_grant(thd, priv_needed, first_table, FALSE, UINT_MAX, FALSE)) DBUG_RETURN(TRUE); /* purecov: inspected */ #ifdef WITH_WSREP - if (WSREP(thd) && WSREP_CLIENT(thd) && + if (WSREP(thd) && (!thd->is_current_stmt_binlog_format_row() || !thd->find_temporary_table(first_table))) { From 81258f14323e1d1ad0203bae93bc55a30d47c1b3 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 2 Apr 2021 22:00:36 +0300 Subject: [PATCH 4/9] MDEV-17913 Encrypted transactional Aria tables remain corrupt after crash recovery, automatic repairment does not work This was because of a wrong test in encryption code that wrote random numbers over the LSN for pages for transactional Aria tables during repair. The effect was that after an ALTER TABLE ENABLE KEYS of a encrypted recovery of the tables would not work. Fixed by changing testing of !share->now_transactional to !share->base.born_transactional. Other things: - Extended Aria check_table() to check for wrong (= too big) LSN numbers. - If check_table() failed just because of wrong LSN or TRN numbers, a following repair table will just do a zerofill which is much faster. - Limit number of LSN errors in one check table to MAX_LSN_ERROR (10). - Removed old obsolete test of 'if (error_count & 2)'. Changed error_count and warning_count from bits to numbers of errors/warnings as this is more useful. --- include/myisamchk.h | 3 +- .../suite/encryption/r/aria_tiny.result | 8 +++ mysql-test/suite/encryption/t/aria_tiny.test | 26 +++++++-- .../suite/maria/maria-autozerofill.result | 6 +- .../suite/maria/maria-autozerofill.test | 2 +- storage/maria/aria_chk.c | 36 ++++++++---- storage/maria/ha_maria.cc | 53 +++++++++++++---- storage/maria/ma_check.c | 57 +++++++++++++++---- storage/maria/ma_check_standalone.h | 4 +- storage/maria/ma_crypt.c | 4 +- storage/maria/ma_loghandler.c | 3 +- storage/maria/ma_loghandler_lsn.h | 3 + storage/maria/ma_open.c | 9 ++- 13 files changed, 166 insertions(+), 48 deletions(-) diff --git a/include/myisamchk.h b/include/myisamchk.h index f1a7348eea0..f9a55ba467f 100644 --- a/include/myisamchk.h +++ b/include/myisamchk.h @@ -70,7 +70,7 @@ typedef struct st_handler_check_param */ ulonglong unique_count[HA_MAX_KEY_SEG + 1]; ulonglong notnull_count[HA_MAX_KEY_SEG + 1]; - + ulonglong max_allowed_lsn; my_off_t search_after_block; my_off_t new_file_pos, key_file_blocks; my_off_t keydata, totaldata, key_blocks, start_check_pos; @@ -96,6 +96,7 @@ typedef struct st_handler_check_param uint out_flag, error_printed, verbose; uint opt_sort_key, total_files, max_level; uint key_cache_block_size, pagecache_block_size; + uint skip_lsn_error_count; int tmpfile_createflag, err_count; myf myf_rw; uint16 language; diff --git a/mysql-test/suite/encryption/r/aria_tiny.result b/mysql-test/suite/encryption/r/aria_tiny.result index b0f8bac7c32..7ad69878215 100644 --- a/mysql-test/suite/encryption/r/aria_tiny.result +++ b/mysql-test/suite/encryption/r/aria_tiny.result @@ -2,4 +2,12 @@ set global aria_encrypt_tables = 1; create table t1 (i int, key(i)) engine=aria; insert into t1 values (1); drop table t1; +create table t1 (a int primary key, b int, c int, key(b),key(c)) engine=aria; +alter table t1 disable keys; +insert into t1 select seq,seq,seq from seq_1_to_100; +alter table t1 enable keys; +check table t1; +Table Op Msg_type Msg_text +test.t1 check status OK +drop table t1; set global aria_encrypt_tables = 0; diff --git a/mysql-test/suite/encryption/t/aria_tiny.test b/mysql-test/suite/encryption/t/aria_tiny.test index cad63ed16f1..a7faba9633a 100644 --- a/mysql-test/suite/encryption/t/aria_tiny.test +++ b/mysql-test/suite/encryption/t/aria_tiny.test @@ -1,12 +1,30 @@ +--source include/have_file_key_management_plugin.inc +--source include/have_sequence.inc + # # MDEV-8022 Assertion `rc == 0' failed in ma_encrypt on dropping an encrypted Aria table # - ---source include/have_file_key_management_plugin.inc - set global aria_encrypt_tables = 1; create table t1 (i int, key(i)) engine=aria; insert into t1 values (1); drop table t1; -set global aria_encrypt_tables = 0; +# +# MDEV-17913 Encrypted transactional Aria tables remain corrupt after crash +# recovery, automatic repair does not work +# +# We are using a simplifed version of the test here. This works thanks to +# the extended check table code that also checks if LSN's are reasonable. +# + +create table t1 (a int primary key, b int, c int, key(b),key(c)) engine=aria; +alter table t1 disable keys; +insert into t1 select seq,seq,seq from seq_1_to_100; +alter table t1 enable keys; +check table t1; +drop table t1; + +# +# Cleanup +# +set global aria_encrypt_tables = 0; diff --git a/mysql-test/suite/maria/maria-autozerofill.result b/mysql-test/suite/maria/maria-autozerofill.result index 064ac9e6496..e135d3d7d58 100644 --- a/mysql-test/suite/maria/maria-autozerofill.result +++ b/mysql-test/suite/maria/maria-autozerofill.result @@ -38,14 +38,15 @@ create_rename_lsn has non-magic value # check table t2; Table Op Msg_type Msg_text -mysqltest.t2 check error Table is from another system and must be zerofilled or repaired to be usable on this system +mysqltest.t2 check error Table is probably from another system and must be zerofilled or repaired ('REPAIR TABLE table_name') to be usable on this system mysqltest.t2 check error Corrupt check table t2; Table Op Msg_type Msg_text -mysqltest.t2 check error Table is from another system and must be zerofilled or repaired to be usable on this system +mysqltest.t2 check error Table is probably from another system and must be zerofilled or repaired ('REPAIR TABLE table_name') to be usable on this system mysqltest.t2 check error Corrupt repair table t2; Table Op Msg_type Msg_text +mysqltest.t2 repair info Running zerofill on moved table mysqltest.t2 repair status OK check table t2; Table Op Msg_type Msg_text @@ -61,6 +62,7 @@ mysqltest.t4 analyze Note Zerofilling moved table ./mysqltest/t4 mysqltest.t4 analyze status OK repair table t5; Table Op Msg_type Msg_text +mysqltest.t5 repair info Running zerofill on moved table mysqltest.t5 repair status OK check table t5; Table Op Msg_type Msg_text diff --git a/mysql-test/suite/maria/maria-autozerofill.test b/mysql-test/suite/maria/maria-autozerofill.test index c24f89576f8..8459e27a8a8 100644 --- a/mysql-test/suite/maria/maria-autozerofill.test +++ b/mysql-test/suite/maria/maria-autozerofill.test @@ -71,7 +71,7 @@ perl; my $fname= "$ENV{'MYSQLTEST_VARDIR'}/tmp/autozerofill.txt"; open(FILE, "<", $fname) or die; my @content= ; - print grep(/Status:.*zerofilled/, @content); + print grep(/Status:.*/, @content); print "create_rename_lsn has magic value\n" if grep(/create_rename \(0,0x2\)/, @content); close FILE; EOF diff --git a/storage/maria/aria_chk.c b/storage/maria/aria_chk.c index cde87a04adc..7463adf34c1 100644 --- a/storage/maria/aria_chk.c +++ b/storage/maria/aria_chk.c @@ -37,6 +37,7 @@ static MY_TMPDIR maria_chk_tmpdir; static my_bool opt_transaction_logging, opt_debug; static my_bool opt_ignore_control_file, opt_require_control_file; static my_bool opt_warning_for_wrong_transid, opt_update_state; +static my_bool have_control_file= 0; static const char *type_names[]= { @@ -144,15 +145,19 @@ int main(int argc, char **argv) { if ((ma_control_file_open(FALSE, opt_require_control_file || !(check_param.testflag & T_SILENT), - TRUE) && - (opt_require_control_file || - (opt_transaction_logging && (check_param.testflag & T_REP_ANY))))) + TRUE))) { - error= 1; - goto end; + if (opt_require_control_file || + (opt_transaction_logging && (check_param.testflag & T_REP_ANY))) + { + error= 1; + goto end; + } } + else + have_control_file= 1; } - else + if (!have_control_file) opt_warning_for_wrong_transid= 0; /* @@ -456,7 +461,7 @@ static struct my_option my_long_options[] = (char**) &maria_stats_method_str, (char**) &maria_stats_method_str, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, { "zerofill", 'z', - "Fill empty space in data and index files with zeroes. This makes the data file movable between different servers.", + "Fill empty space in data and index files with zeroes. This makes the data file movable between different servers. It also fixes any wrong transaction or LSN numbers in the table after a crash or if someone removed the Aria log files.", 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, { "zerofill-keep-lsn", OPT_ZEROFILL_KEEP_LSN, "Like --zerofill but does not zero out LSN of data/index pages;" @@ -468,7 +473,7 @@ static struct my_option my_long_options[] = static void print_version(void) { - printf("%s Ver 1.2 for %s on %s\n", my_progname, SYSTEM_TYPE, + printf("%s Ver 1.3 for %s on %s\n", my_progname, SYSTEM_TYPE, MACHINE_TYPE); } @@ -614,7 +619,9 @@ Recover (repair)/ options (When using '--recover' or '--safe-recover'):\n\ Find a record, a block at given offset belongs to.\n\ -z, --zerofill Fill empty space in data and index files with zeroes.\n\ This makes the data file movable between different \n\ - servers.\n\ + servers. It also fixes any wrong transaction or LSN\n\ + numbers in the table after a crash or if someone\n\ + removed the Aria log files.\n\ --zerofill-keep-lsn Like --zerofill but does not zero out LSN of\n\ data/index pages."); @@ -1034,7 +1041,7 @@ static int maria_chk(HA_CHECK *param, char *filename) 0))) { /* Avoid twice printing of isam file name */ - param->error_printed=1; + param->error_printed++; switch (my_errno) { case HA_ERR_CRASHED: _ma_check_print_error(param,"'%s' doesn't have a correct index definition. You need to recreate it before you can do a repair",filename); @@ -1506,8 +1513,7 @@ end2: "the --force (-f) option or by not using the --quick (-q) " "flag\n"); } - else if (!(param->error_printed & 2) && - !(param->testflag & T_FORCE_CREATE)) + else if (!(param->testflag & T_FORCE_CREATE)) fprintf(stderr, "Aria table '%s' is corrupted\nFix it using switch " "\"-r\" or \"-o\"\n", filename); } @@ -1592,6 +1598,10 @@ static void descript(HA_CHECK *param, register MARIA_HA *info, char *name) if (share->state.changed & STATE_CRASHED) strmov(buff, share->state.changed & STATE_CRASHED_ON_REPAIR ? "crashed on repair" : "crashed"); + else if (have_control_file && + (share->state.changed & (STATE_MOVED | STATE_NOT_ZEROFILLED)) == + (STATE_MOVED | STATE_NOT_ZEROFILLED)) + strmov(buff, "moved from another system. Use --zerofill to fix it"); else { if (share->state.open_count) @@ -1610,6 +1620,8 @@ static void descript(HA_CHECK *param, register MARIA_HA *info, char *name) pos=strmov(pos,"zerofilled,"); if (!(share->state.changed & STATE_NOT_MOVABLE)) pos=strmov(pos,"movable,"); + if (have_control_file && (share->state.changed & STATE_MOVED)) + pos=strmov(pos,"moved,"); pos[-1]=0; /* Remove extra ',' */ } printf("Status: %s\n",buff); diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index c92f5f9eb5c..79d32886659 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -61,8 +61,7 @@ C_MODE_END ulong pagecache_division_limit, pagecache_age_threshold, pagecache_file_hash_size; ulonglong pagecache_buffer_size; const char *zerofill_error_msg= - "Table is from another system and must be zerofilled or repaired to be " - "usable on this system"; + "Table is probably from another system and must be zerofilled or repaired ('REPAIR TABLE table_name') to be usable on this system"; /** As the auto-repair is initiated when opened from the SQL layer @@ -906,7 +905,7 @@ void _ma_check_print_error(HA_CHECK *param, const char *fmt, ...) { va_list args; DBUG_ENTER("_ma_check_print_error"); - param->error_printed |= 1; + param->error_printed++; param->out_flag |= O_DATA_LOST; if (param->testflag & T_SUPPRESS_ERR_HANDLING) DBUG_VOID_RETURN; @@ -932,7 +931,7 @@ void _ma_check_print_warning(HA_CHECK *param, const char *fmt, ...) { va_list args; DBUG_ENTER("_ma_check_print_warning"); - param->warning_printed= 1; + param->warning_printed++; param->out_flag |= O_DATA_LOST; va_start(args, fmt); _ma_check_print_msg(param, MA_CHECK_WARNING, fmt, args); @@ -1262,7 +1261,7 @@ int ha_maria::write_row(const uchar * buf) int ha_maria::check(THD * thd, HA_CHECK_OPT * check_opt) { - int error; + int error, fatal_error; HA_CHECK *param= (HA_CHECK*) thd->alloc(sizeof *param); MARIA_SHARE *share= file->s; const char *old_proc_info; @@ -1294,6 +1293,7 @@ int ha_maria::check(THD * thd, HA_CHECK_OPT * check_opt) return HA_ADMIN_ALREADY_DONE; maria_chk_init_for_check(param, file); + param->max_allowed_lsn= translog_get_horizon(); if ((file->s->state.changed & (STATE_CRASHED_FLAGS | STATE_MOVED)) == STATE_MOVED) @@ -1336,9 +1336,21 @@ int ha_maria::check(THD * thd, HA_CHECK_OPT * check_opt) param->testflag= old_testflag; } } - if (!error) + fatal_error= error; + if (param->error_printed && + param->error_printed == (param->skip_lsn_error_count + + param->not_visible_rows_found) && + !(share->state.changed & (STATE_CRASHED_FLAGS | STATE_IN_REPAIR))) { - if ((share->state.changed & (STATE_CHANGED | + _ma_check_print_error(param, "%s", zerofill_error_msg); + /* This ensures that a future REPAIR TABLE will only do a zerofill */ + file->update|= STATE_MOVED; + share->state.changed|= STATE_MOVED; + fatal_error= 0; + } + if (!fatal_error) + { + if ((share->state.changed & (STATE_CHANGED | STATE_MOVED | STATE_CRASHED_FLAGS | STATE_IN_REPAIR | STATE_NOT_ANALYZED)) || (param->testflag & T_STATISTICS) || maria_is_crashed(file)) @@ -1349,9 +1361,13 @@ int ha_maria::check(THD * thd, HA_CHECK_OPT * check_opt) share->state.changed&= ~(STATE_CHANGED | STATE_CRASHED_FLAGS | STATE_IN_REPAIR); if (!(table->db_stat & HA_READ_ONLY)) - error= maria_update_state_info(param, file, - UPDATE_TIME | UPDATE_OPEN_COUNT | - UPDATE_STAT); + { + int tmp; + if ((tmp= maria_update_state_info(param, file, + UPDATE_TIME | UPDATE_OPEN_COUNT | + UPDATE_STAT))) + error= tmp; + } mysql_mutex_unlock(&share->intern_lock); info(HA_STATUS_NO_LOCK | HA_STATUS_TIME | HA_STATUS_VARIABLE | HA_STATUS_CONST); @@ -1443,6 +1459,20 @@ int ha_maria::repair(THD * thd, HA_CHECK_OPT *check_opt) maria_chk_init(param); param->thd= thd; param->op_name= "repair"; + + /* + The following can only be true if the table was marked as STATE_MOVED + during a CHECK TABLE and the table has not been used since then + */ + if ((file->s->state.changed & STATE_MOVED) && + !(file->s->state.changed & STATE_CRASHED_FLAGS)) + { + param->db_name= table->s->db.str; + param->table_name= table->alias.c_ptr(); + _ma_check_print_info(param, "Running zerofill on moved table"); + return zerofill(thd, check_opt); + } + param->testflag= ((check_opt->flags & ~(T_EXTEND)) | T_SILENT | T_FORCE_CREATE | T_CALC_CHECKSUM | (check_opt->flags & T_EXTEND ? T_REP : T_REP_BY_SORT)); @@ -1518,6 +1548,9 @@ int ha_maria::zerofill(THD * thd, HA_CHECK_OPT *check_opt) param->op_name= "zerofill"; param->testflag= check_opt->flags | T_SILENT | T_ZEROFILL; param->sort_buffer_length= THDVAR(thd, sort_buffer_size); + param->db_name= table->s->db.str; + param->table_name= table->alias.c_ptr(); + error=maria_zerofill(param, file, share->open_file_name.str); /* Reset trn, that may have been set by repair */ diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c index c6aa0662a9f..e1c85cd2d44 100644 --- a/storage/maria/ma_check.c +++ b/storage/maria/ma_check.c @@ -124,6 +124,7 @@ void maria_chk_init(HA_CHECK *param) param->stats_method= MI_STATS_METHOD_NULLS_NOT_EQUAL; param->max_stage= 1; param->stack_end_ptr= &my_thread_var->stack_ends_here; + param->max_allowed_lsn= (LSN) ~0ULL; } @@ -903,15 +904,33 @@ static int chk_index(HA_CHECK *param, MARIA_HA *info, MARIA_KEYDEF *keyinfo, _ma_check_print_error(param, "Page at %s is not marked for index %u", llstr(anc_page->pos, llbuff), (uint) keyinfo->key_nr); - if ((page_flag & KEYPAGE_FLAG_HAS_TRANSID) && - !share->base.born_transactional) + if (page_flag & KEYPAGE_FLAG_HAS_TRANSID) { - _ma_check_print_error(param, - "Page at %s is marked with HAS_TRANSID even if " - "table is not transactional", - llstr(anc_page->pos, llbuff)); + if (!share->base.born_transactional) + { + _ma_check_print_error(param, + "Page at %s is marked with HAS_TRANSID even if " + "table is not transactional", + llstr(anc_page->pos, llbuff)); + } + } + if (share->base.born_transactional) + { + LSN lsn= lsn_korr(anc_page->buff); + if ((ulonglong) lsn > param->max_allowed_lsn) + { + /* Avoid flooding of errors */ + if (param->skip_lsn_error_count++ < MAX_LSN_ERRORS) + { + _ma_check_print_error(param, + "Page at %s as wrong LSN " LSN_FMT ". Current " + "LSN is " LSN_FMT, + llstr(anc_page->pos, llbuff), + LSN_IN_PARTS(lsn), + LSN_IN_PARTS(param->max_allowed_lsn)); + } + } } - if (anc_page->size > share->max_index_block_size) { _ma_check_print_error(param, @@ -1850,6 +1869,7 @@ static int check_block_record(HA_CHECK *param, MARIA_HA *info, int extend, ha_rows full_page_count, tail_count; my_bool UNINIT_VAR(full_dir), now_transactional; uint offset_page, offset, free_count; + LSN lsn; if (_ma_scan_init_block_record(info)) { @@ -1994,6 +2014,23 @@ static int check_block_record(HA_CHECK *param, MARIA_HA *info, int extend, if (param->err_count++ > MAXERR || !(param->testflag & T_VERBOSE)) goto err; } + if (share->base.born_transactional) + { + lsn= lsn_korr(page_buff); + if ((ulonglong) lsn > param->max_allowed_lsn) + { + /* Avoid flooding of errors */ + if (param->skip_lsn_error_count++ < MAX_LSN_ERRORS) + { + _ma_check_print_error(param, + "Page %9s: Wrong LSN " LSN_FMT ". Current " + "LSN is " LSN_FMT, + llstr(page, llbuff), + LSN_IN_PARTS(lsn), + LSN_IN_PARTS(param->max_allowed_lsn)); + } + } + } if ((enum en_page_type) page_type == BLOB_PAGE) continue; param->empty+= empty_space; @@ -2778,7 +2815,7 @@ int maria_repair(HA_CHECK *param, register MARIA_HA *info, if ((param->testflag & (T_FORCE_UNIQUENESS|T_QUICK)) == T_QUICK) { param->testflag|=T_RETRY_WITHOUT_QUICK; - param->error_printed=1; + param->error_printed++; goto err; } /* purecov: begin tested */ @@ -5053,7 +5090,7 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param) } if (searching && ! sort_param->fix_datafile) { - param->error_printed=1; + param->error_printed++; param->retry_repair=1; param->testflag|=T_RETRY_WITHOUT_QUICK; my_errno= HA_ERR_WRONG_IN_RECORD; @@ -5327,7 +5364,7 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param) DBUG_RETURN(-1); if (searching && ! sort_param->fix_datafile) { - param->error_printed=1; + param->error_printed++; param->retry_repair=1; param->testflag|=T_RETRY_WITHOUT_QUICK; my_errno= HA_ERR_WRONG_IN_RECORD; diff --git a/storage/maria/ma_check_standalone.h b/storage/maria/ma_check_standalone.h index e2e651b43f3..9442800a0c7 100644 --- a/storage/maria/ma_check_standalone.h +++ b/storage/maria/ma_check_standalone.h @@ -124,7 +124,7 @@ void _ma_check_print_warning(HA_CHECK *param, const char *fmt,...) param->isam_file_name); param->out_flag|= O_DATA_LOST; } - param->warning_printed=1; + param->warning_printed++; va_start(args,fmt); fprintf(stderr,"%s: warning: ",my_progname_short); vfprintf(stderr, fmt, args); @@ -149,7 +149,7 @@ void _ma_check_print_error(HA_CHECK *param, const char *fmt,...) fprintf(stderr,"%s: Aria file %s\n",my_progname_short,param->isam_file_name); param->out_flag|= O_DATA_LOST; } - param->error_printed|=1; + param->error_printed++; va_start(args,fmt); fprintf(stderr,"%s: error: ",my_progname_short); vfprintf(stderr, fmt, args); diff --git a/storage/maria/ma_crypt.c b/storage/maria/ma_crypt.c index 48861485012..31f16a21841 100644 --- a/storage/maria/ma_crypt.c +++ b/storage/maria/ma_crypt.c @@ -268,7 +268,7 @@ static my_bool ma_crypt_data_pre_write_hook(PAGECACHE_IO_HOOK_ARGS *args) return 1; } - if (!share->now_transactional) + if (!share->base.born_transactional) { /* store a random number instead of LSN (for counter block) */ store_rand_lsn(args->page); @@ -392,7 +392,7 @@ static my_bool ma_crypt_index_pre_write_hook(PAGECACHE_IO_HOOK_ARGS *args) return 1; } - if (!share->now_transactional) + if (!share->base.born_transactional) { /* store a random number instead of LSN (for counter block) */ store_rand_lsn(args->page); diff --git a/storage/maria/ma_loghandler.c b/storage/maria/ma_loghandler.c index 1413b708bbc..5d3402ef866 100644 --- a/storage/maria/ma_loghandler.c +++ b/storage/maria/ma_loghandler.c @@ -7926,7 +7926,8 @@ void check_skipped_lsn(MARIA_HA *info, LSN lsn, my_bool index_file, else { /* Give error, but don't flood the log */ - if (skipped_lsn_err_count++ < 10 && ! info->s->redo_error_given++) + if (skipped_lsn_err_count++ < MAX_LSN_ERRORS && + ! info->s->redo_error_given++) { eprint(tracef, "Table %s has wrong LSN: " LSN_FMT " on page: %llu", (index_file ? info->s->data_file_name.str : diff --git a/storage/maria/ma_loghandler_lsn.h b/storage/maria/ma_loghandler_lsn.h index c99f0d0af97..c5bd76bb6b8 100644 --- a/storage/maria/ma_loghandler_lsn.h +++ b/storage/maria/ma_loghandler_lsn.h @@ -109,4 +109,7 @@ typedef LSN LSN_WITH_FLAGS; */ #define LSN_MAX (LSN)0x00FFFFFFFFFFFFFFULL +/* Max LSN error to print on check or recovery */ +#define MAX_LSN_ERRORS 10 + #endif diff --git a/storage/maria/ma_open.c b/storage/maria/ma_open.c index 9e7106ab5d3..d45907a44b9 100644 --- a/storage/maria/ma_open.c +++ b/storage/maria/ma_open.c @@ -565,13 +565,16 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags, !maria_in_recovery) || ((share->state.changed & STATE_NOT_MOVABLE) && ((!(open_flags & HA_OPEN_IGNORE_MOVED_STATE) && - memcmp(share->base.uuid, maria_uuid, MY_UUID_SIZE)))))) + memcmp(share->base.uuid, maria_uuid, MY_UUID_SIZE)))) || + ((share->state.changed & (STATE_MOVED | STATE_NOT_ZEROFILLED)) == + (STATE_MOVED | STATE_NOT_ZEROFILLED)))) { - DBUG_PRINT("warning", ("table is moved from another system. uuid_diff: %d create_trid: %lu max_trid: %lu", + DBUG_PRINT("warning", ("table is moved from another system. uuid_diff: %d create_trid: %lu max_trid: %lu moved: &d", memcmp(share->base.uuid, maria_uuid, MY_UUID_SIZE) != 0, (ulong) share->state.create_trid, - (ulong) trnman_get_max_trid())); + (ulong) trnman_get_max_trid(), + MY_TEST((share->state.changed & STATE_MOVED)))); if (open_flags & HA_OPEN_FOR_REPAIR) share->state.changed|= STATE_MOVED; else From 5b71e0424c0c647c39798fd1791e8b68d730d784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Tue, 6 Apr 2021 15:33:13 +0300 Subject: [PATCH 5/9] MDEV-21402 : sql_safe_updates breaks Galera 4 Added handling for sql_safe_updated i.e. we disable it while we do wsrep_schema operations. --- .../r/galera_wsrep_schema_init.result | 94 +++++++++++++++++++ .../t/galera_wsrep_schema_init.cnf | 5 + .../t/galera_wsrep_schema_init.test | 58 ++++++++++++ sql/wsrep_schema.cc | 36 ++++++- 4 files changed, 188 insertions(+), 5 deletions(-) create mode 100644 mysql-test/suite/galera_3nodes/r/galera_wsrep_schema_init.result create mode 100644 mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.cnf create mode 100644 mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.test diff --git a/mysql-test/suite/galera_3nodes/r/galera_wsrep_schema_init.result b/mysql-test/suite/galera_3nodes/r/galera_wsrep_schema_init.result new file mode 100644 index 00000000000..2a29afd62be --- /dev/null +++ b/mysql-test/suite/galera_3nodes/r/galera_wsrep_schema_init.result @@ -0,0 +1,94 @@ +connection node_2; +connection node_1; +connect node_3, 127.0.0.1, root, , test, $NODE_MYPORT_3; +connection node_1; +connection node_1; +connection node_2; +connection node_3; +SHOW CREATE TABLE mysql.wsrep_cluster; +Table Create Table +wsrep_cluster CREATE TABLE `wsrep_cluster` ( + `cluster_uuid` char(36) NOT NULL, + `view_id` bigint(20) NOT NULL, + `view_seqno` bigint(20) NOT NULL, + `protocol_version` int(11) NOT NULL, + `capabilities` int(11) NOT NULL, + PRIMARY KEY (`cluster_uuid`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +SHOW CREATE TABLE mysql.wsrep_cluster_members; +Table Create Table +wsrep_cluster_members CREATE TABLE `wsrep_cluster_members` ( + `node_uuid` char(36) NOT NULL, + `cluster_uuid` char(36) NOT NULL, + `node_name` char(32) NOT NULL, + `node_incoming_address` varchar(256) NOT NULL, + PRIMARY KEY (`node_uuid`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +SELECT @@sql_safe_updates; +@@sql_safe_updates +1 +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster; +COUNT(*) = 1 +1 +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster; +cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') +1 +SELECT COUNT(*) = 3 FROM mysql.wsrep_cluster_members; +COUNT(*) = 3 +1 +SELECT COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') FROM mysql.wsrep_cluster_members; +COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') +1 +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster_members WHERE node_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_gcomm_uuid'); +COUNT(*) = 1 +1 +SELECT node_incoming_address LIKE '127.0.0.1:%' from mysql.wsrep_cluster_members; +node_incoming_address LIKE '127.0.0.1:%' +1 +1 +1 +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster_members; +cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') +1 +1 +1 +connection node_2; +SELECT COUNT(*) = 3 FROM mysql.wsrep_cluster_members; +COUNT(*) = 3 +1 +SELECT COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') FROM mysql.wsrep_cluster_members; +COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') +1 +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster_members WHERE node_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_gcomm_uuid'); +COUNT(*) = 1 +1 +SELECT node_incoming_address LIKE '127.0.0.1:%' from mysql.wsrep_cluster_members; +node_incoming_address LIKE '127.0.0.1:%' +1 +1 +1 +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster_members; +cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') +1 +1 +1 +connection node_3; +SELECT COUNT(*) = 3 FROM mysql.wsrep_cluster_members; +COUNT(*) = 3 +1 +SELECT COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') FROM mysql.wsrep_cluster_members; +COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') +1 +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster_members WHERE node_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_gcomm_uuid'); +COUNT(*) = 1 +1 +SELECT node_incoming_address LIKE '127.0.0.1:%' from mysql.wsrep_cluster_members; +node_incoming_address LIKE '127.0.0.1:%' +1 +1 +1 +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster_members; +cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') +1 +1 +1 diff --git a/mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.cnf b/mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.cnf new file mode 100644 index 00000000000..317094cea72 --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.cnf @@ -0,0 +1,5 @@ +!include ../galera_3nodes.cnf + +[mysqld] +sql-safe-updates=1 +wsrep-debug=1 diff --git a/mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.test b/mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.test new file mode 100644 index 00000000000..7d8089a8ceb --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/galera_wsrep_schema_init.test @@ -0,0 +1,58 @@ +# +# This test performs basic checks on the contents of the wsrep_schema +# +# wsrep_members_history checks are temporarily disabled until it +# can be made configurable. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +--connect node_3, 127.0.0.1, root, , test, $NODE_MYPORT_3 +--connection node_1 +# Save original auto_increment_offset values. +--let $node_1=node_1 +--let $node_2=node_2 +--let $node_3=node_3 +--source ../galera/include/auto_increment_offset_save.inc + +# Make the test fail if table structure has changed + +SHOW CREATE TABLE mysql.wsrep_cluster; +SHOW CREATE TABLE mysql.wsrep_cluster_members; +#disabled SHOW CREATE TABLE mysql.wsrep_member_history; +SELECT @@sql_safe_updates; + +# Checks for the wsrep_cluster table + +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster; +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster; + +# Checks for the wsrep_cluster_members table + +SELECT COUNT(*) = 3 FROM mysql.wsrep_cluster_members; +SELECT COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') FROM mysql.wsrep_cluster_members; +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster_members WHERE node_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_gcomm_uuid'); + +SELECT node_incoming_address LIKE '127.0.0.1:%' from mysql.wsrep_cluster_members; +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster_members; + +--connection node_2 + +SELECT COUNT(*) = 3 FROM mysql.wsrep_cluster_members; +SELECT COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') FROM mysql.wsrep_cluster_members; +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster_members WHERE node_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_gcomm_uuid'); + +SELECT node_incoming_address LIKE '127.0.0.1:%' from mysql.wsrep_cluster_members; +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster_members; + +--connection node_3 +SELECT COUNT(*) = 3 FROM mysql.wsrep_cluster_members; +SELECT COUNT(*) = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size') FROM mysql.wsrep_cluster_members; +SELECT COUNT(*) = 1 FROM mysql.wsrep_cluster_members WHERE node_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_gcomm_uuid'); + +SELECT node_incoming_address LIKE '127.0.0.1:%' from mysql.wsrep_cluster_members; +SELECT cluster_uuid = (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_state_uuid') FROM mysql.wsrep_cluster_members; + +--source ../galera/include/auto_increment_offset_restore.inc + diff --git a/sql/wsrep_schema.cc b/sql/wsrep_schema.cc index e811d4e8317..5ee6468e9c1 100644 --- a/sql/wsrep_schema.cc +++ b/sql/wsrep_schema.cc @@ -159,6 +159,24 @@ private: THD *m_cur_thd; }; +class sql_safe_updates +{ +public: + sql_safe_updates(THD* thd) + : m_thd(thd) + , m_option_bits(thd->variables.option_bits) + { + thd->variables.option_bits&= ~OPTION_SAFE_UPDATES; + } + ~sql_safe_updates() + { + m_thd->variables.option_bits= m_option_bits; + } +private: + THD* m_thd; + ulonglong m_option_bits; +}; + static int execute_SQL(THD* thd, const char* sql, uint length) { DBUG_ENTER("Wsrep_schema::execute_SQL()"); int err= 0; @@ -603,13 +621,15 @@ static void wsrep_init_thd_for_schema(THD *thd) thd->prior_thr_create_utime= thd->start_utime= thd->thr_create_utime; - /* */ - thd->variables.wsrep_on = 0; + /* No Galera replication */ + thd->variables.wsrep_on= 0; /* No binlogging */ - thd->variables.sql_log_bin = 0; - thd->variables.option_bits &= ~OPTION_BIN_LOG; + thd->variables.sql_log_bin= 0; + thd->variables.option_bits&= ~OPTION_BIN_LOG; + /* No safe updates */ + thd->variables.option_bits&= ~OPTION_SAFE_UPDATES; /* No general log */ - thd->variables.option_bits |= OPTION_LOG_OFF; + thd->variables.option_bits|= OPTION_LOG_OFF; /* Read committed isolation to avoid gap locking */ thd->variables.tx_isolation= ISO_READ_COMMITTED; wsrep_assign_from_threadvars(thd); @@ -664,6 +684,7 @@ int Wsrep_schema::store_view(THD* thd, const Wsrep_view& view) Wsrep_schema_impl::wsrep_off wsrep_off(thd); Wsrep_schema_impl::binlog_off binlog_off(thd); + Wsrep_schema_impl::sql_safe_updates sql_safe_updates(thd); /* Clean up cluster table and members table. @@ -918,6 +939,7 @@ int Wsrep_schema::append_fragment(THD* thd, thd->lex->reset_n_backup_query_tables_list(&query_tables_list_backup); Wsrep_schema_impl::binlog_off binlog_off(thd); + Wsrep_schema_impl::sql_safe_updates sql_safe_updates(thd); Wsrep_schema_impl::init_stmt(thd); TABLE* frag_table= 0; @@ -967,6 +989,7 @@ int Wsrep_schema::update_fragment_meta(THD* thd, thd->lex->reset_n_backup_query_tables_list(&query_tables_list_backup); Wsrep_schema_impl::binlog_off binlog_off(thd); + Wsrep_schema_impl::sql_safe_updates sql_safe_updates(thd); int error; uchar *key=NULL; key_part_map key_map= 0; @@ -1089,6 +1112,7 @@ int Wsrep_schema::remove_fragments(THD* thd, WSREP_DEBUG("Removing %zu fragments", fragments.size()); Wsrep_schema_impl::wsrep_off wsrep_off(thd); Wsrep_schema_impl::binlog_off binlog_off(thd); + Wsrep_schema_impl::sql_safe_updates sql_safe_updates(thd); Query_tables_list query_tables_list_backup; Open_tables_backup open_tables_backup; @@ -1156,6 +1180,7 @@ int Wsrep_schema::replay_transaction(THD* orig_thd, Wsrep_schema_impl::wsrep_off wsrep_off(&thd); Wsrep_schema_impl::binlog_off binlog_off(&thd); + Wsrep_schema_impl::sql_safe_updates sql_safe_updates(&thd); Wsrep_schema_impl::thd_context_switch thd_context_switch(orig_thd, &thd); int ret= 1; @@ -1270,6 +1295,7 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd) Wsrep_storage_service storage_service(&storage_thd); Wsrep_schema_impl::binlog_off binlog_off(&storage_thd); Wsrep_schema_impl::wsrep_off wsrep_off(&storage_thd); + Wsrep_schema_impl::sql_safe_updates sql_safe_updates(&storage_thd); Wsrep_schema_impl::thd_context_switch thd_context_switch(orig_thd, &storage_thd); Wsrep_server_state& server_state(Wsrep_server_state::instance()); From f69c1c9dcb815d7597ec2035470a81ac3b6c9380 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Tue, 6 Apr 2021 16:57:38 +1000 Subject: [PATCH 6/9] MDEV-19508: SI_KERNEL is not on all implementations SI_USER is, however in FreeBSD there are a couple of non-kernel user signal infomations above SI_KERNEL. Put a fallback just in case there is nothing available. --- include/my_pthread.h | 1 - sql/mysqld.cc | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/include/my_pthread.h b/include/my_pthread.h index 81dd63ee331..bc47bb8bad0 100644 --- a/include/my_pthread.h +++ b/include/my_pthread.h @@ -197,7 +197,6 @@ static inline int my_sigwait(sigset_t *set, int *sig, int *code) *code= siginfo.si_code; return *sig < 0 ? errno : 0; #else -#define SI_KERNEL 128 *code= 0; return sigwait(set, sig); #endif diff --git a/sql/mysqld.cc b/sql/mysqld.cc index c536cdef504..7e3ce878cdc 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -3253,7 +3253,13 @@ pthread_handler_t signal_hand(void *arg __attribute__((unused))) } break; case SIGHUP: +#if defined(SI_KERNEL) if (!abort_loop && origin != SI_KERNEL) +#elif defined(SI_USER) + if (!abort_loop && origin <= SI_USER) +#else + if (!abort_loop) +#endif { int not_used; mysql_print_status(); // Print some debug info From 58780b5afbd00a87899c524735ec11b3534e51ee Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Thu, 25 Mar 2021 06:55:18 +0400 Subject: [PATCH 7/9] MDEV-22775 [HY000][1553] Changing name of primary key column with foreign key constraint fails. Problem: The problem happened because of a conceptual flaw in the server code: a. The table level CHARSET/COLLATE clause affected all data types, including numeric and temporal ones: CREATE TABLE t1 (a INT) CHARACTER SET utf8 [COLLATE utf8_general_ci]; In the above example, the Column_definition_attributes (and then the FRM record) for the column "a" erroneously inherited "utf8" as its character set. b. The "ALTER TABLE t1 CONVERT TO CHARACTER SET csname" statement also erroneously affected Column_definition_attributes::charset for numeric and temporal data types and wrote "csname" as their character set into FRM files. So now we have arbitrary non-relevant charset ID values for numeric and temporal data types in all FRM files in the world :) The code in the server and the other engines did not seem to be affected by this flaw. Only InnoDB inplace ALTER was affected. Solution: Fixing the code in the way that only character string data types (CHAR,VARCHAR,TEXT,ENUM,SET): - inherit the table level CHARSET/COLLATE clause - get the charset value according to "CONVERT TO CHARACTER SET csname". Numeric and temporal data types now always get &my_charset_numeric in Column_definition_attributes::charset and always write its ID into FRM files: - no matter what the table level CHARSET/COLLATE clause is, and - no matter what "CONVERT TO CHARACTER SET" says. Details: 1. Adding helper classes to pass small parts of HA_CREATE_INFO into Type_handler methods: - Column_derived_attributes - to pass table level CHARSET/COLLATE, so columns that do not have explicit CHARSET/COLLATE clauses can derive them from the table level, e.g. CREATE TABLE t1 (a VARCHAR(1), b CHAR(1)) CHARACTER SET utf8; - Column_bulk_alter_attributes - to pass bulk attribute changes generated by the ALTER related code. These bulk changes affect multiple columns at the same time: ALTER TABLE ... CONVERT TO CHARACTER SET csname; Note, passing the whole HA_CREATE_INFO directly to Type_handler would not be good: HA_CREATE_INFO is huge and would need not desired dependencies in sql_type.h and sql_type.cc. The Type_handler API should use smallest possible data types! 2. Type_handler::Column_definition_prepare_stage1() is now responsible to set Column_definition::charset properly, according to the data type, for example: - For string data types, Column_definition_attributes::charset is set from the table level CHARSET/COLLATE clause (if not specified explicitly in the column definition). - For numeric and temporal fields, Column_definition_attributes::charset is set to &my_charset_numeric, no matter what the table level CHARSET/COLLATE says. - For GEOMETRY, Column_definition_attributes::charset is set to &my_charset_bin, no matter what the table level CHARSET/COLLATE says. Previously this code (setting `charset`) was outside of of Column_definition_prepare_stage1(), namely in mysql_prepare_create_table(), and was erroneously called for all data types. 3. Adding Type_handler::Column_definition_bulk_alter(), to handle "ALTER TABLE .. CONVERT TO". Previously this code was inside get_sql_field_charset() and was erroneously called for all data types. 4. Removing the Schema_specification_st parameter from Type_handler::Column_definition_redefine_stage1(). Column_definition_attributes::charset is now fully properly initialized by Column_definition_prepare_stage1(). So we don't need access to the table level CHARSET/COLLATE clause in Column_definition_redefine_stage1() any more. 5. Other changes: - Removing global function get_sql_field_charset() - Moving the part of the former get_sql_field_charset(), which was responsible to inherit the table level CHARSET/COLLATE clause to new methods: -- Column_definition_attributes::explicit_or_derived_charset() and -- Column_definition::prepare_charset_for_string(). This code is only needed for string data types. Previously it was erroneously called for all data types. - Moving another part, which was responsible to apply the "CONVERT TO" clause, to Type_handler_general_purpose_string::Column_definition_bulk_alter(). - Replacing the call for get_sql_field_charset() in sql_partition.cc to sql_field->explicit_or_derived_charset() - it is perfectly enough. The old code was redundant: get_sql_field_charset() was called from sql_partition.cc only when there were no a "CONVERT TO CHARACTER SET" clause involved, so its purpose was only to inherit the table level CHARSET/COLLATE clause. - Moving the code handling the BINCMP_FLAG flag from mysql_prepare_create_table() to Column_definition::prepare_charset_for_string(): This code is responsible to resolve the BINARY comparison style into the corresponding _bin collation, to do the following transparent rewrite: CREATE TABLE t1 (a VARCHAR(10) BINARY) CHARSET utf8; -> CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET utf8 COLLATE utf8_bin); This code is only needed for string data types. Previously it was erroneously called for all data types. 6. Renaming Table_scope_and_contents_source_pod_st::table_charset to alter_table_convert_to_charset, because the only purpose it's used for is handlering "ALTER .. CONVERT". The new name is much more self-descriptive. --- .../innodb/r/instant_alter_charset.result | 11 ++ .../suite/innodb/t/instant_alter_charset.test | 14 ++ sql/field.cc | 18 ++- sql/field.h | 38 ++++- sql/handler.h | 4 +- sql/sql_partition.cc | 4 +- sql/sql_table.cc | 80 +++++----- sql/sql_table.h | 2 - sql/sql_type.cc | 140 ++++++++++++++---- sql/sql_type.h | 139 ++++++++++++++--- sql/table.cc | 2 +- storage/federatedx/ha_federatedx.cc | 4 +- 12 files changed, 338 insertions(+), 118 deletions(-) diff --git a/mysql-test/suite/innodb/r/instant_alter_charset.result b/mysql-test/suite/innodb/r/instant_alter_charset.result index 6b60c79b558..8b1171191fa 100644 --- a/mysql-test/suite/innodb/r/instant_alter_charset.result +++ b/mysql-test/suite/innodb/r/instant_alter_charset.result @@ -2032,3 +2032,14 @@ ALTER TABLE t1 MODIFY a VARCHAR(2) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; INSERT INTO t1 VALUES ('a'); DROP TABLE t1; +# +# MDEV-22775 [HY000][1553] Changing name of primary key column with foreign key constraint fails. +# +create table t1 (id int primary key) engine=innodb default charset=utf8; +create table t2 (input_id int primary key, id int not null, +key a (id), +constraint a foreign key (id) references t1 (id) +)engine=innodb default charset=utf8; +alter table t1 change id id2 int; +drop table t2; +drop table t1; diff --git a/mysql-test/suite/innodb/t/instant_alter_charset.test b/mysql-test/suite/innodb/t/instant_alter_charset.test index a5ddd49830c..1d444b88a7f 100644 --- a/mysql-test/suite/innodb/t/instant_alter_charset.test +++ b/mysql-test/suite/innodb/t/instant_alter_charset.test @@ -843,3 +843,17 @@ ALTER TABLE t1 MODIFY a VARCHAR(2) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; INSERT INTO t1 VALUES ('a'); DROP TABLE t1; + + +--echo # +--echo # MDEV-22775 [HY000][1553] Changing name of primary key column with foreign key constraint fails. +--echo # + +create table t1 (id int primary key) engine=innodb default charset=utf8; +create table t2 (input_id int primary key, id int not null, + key a (id), + constraint a foreign key (id) references t1 (id) +)engine=innodb default charset=utf8; +alter table t1 change id id2 int; +drop table t2; +drop table t1; diff --git a/sql/field.cc b/sql/field.cc index 7aea222ca07..89c51288de8 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -11076,16 +11076,26 @@ Column_definition::Column_definition(THD *thd, Field *old_field, CREATE TABLE t1 (a INT) AS SELECT a FROM t2; See Type_handler::Column_definition_redefine_stage1() for data type specific code. + + @param this - The field definition corresponding to the expression + in the "AS SELECT.." part. + + @param dup_field - The field definition from the "CREATE TABLE (...)" part. + It has already underwent prepare_stage1(), so + must be fully initialized: + -- dup_field->charset is set and BINARY + sorting style is applied, see find_bin_collation(). + + @param file - The table handler */ void Column_definition::redefine_stage1_common(const Column_definition *dup_field, - const handler *file, - const Schema_specification_st *schema) + const handler *file) { set_handler(dup_field->type_handler()); default_value= dup_field->default_value; - charset= dup_field->charset ? dup_field->charset : - schema->default_table_charset; + DBUG_ASSERT(dup_field->charset); // Set by prepare_stage1() + charset= dup_field->charset; length= dup_field->char_length; pack_length= dup_field->pack_length; key_length= dup_field->key_length; diff --git a/sql/field.h b/sql/field.h index b04e29ac6fb..6cb19d1e238 100644 --- a/sql/field.h +++ b/sql/field.h @@ -4631,6 +4631,11 @@ public: void frm_pack_charset(uchar *buff) const; void frm_unpack_basic(const uchar *buff); bool frm_unpack_charset(TABLE_SHARE *share, const uchar *buff); + CHARSET_INFO *explicit_or_derived_charset(const Column_derived_attributes + *derived_attr) const + { + return charset ? charset : derived_attr->charset(); + } }; @@ -4765,6 +4770,15 @@ public: void create_length_to_internal_length_bit(); void create_length_to_internal_length_newdecimal(); + /* + Prepare the "charset" member for string data types, + such as CHAR, VARCHAR, TEXT, ENUM, SET: + - derive the charset if not specified explicitly + - find a _bin collation if the BINARY comparison style was specified, e.g.: + CREATE TABLE t1 (a VARCHAR(10) BINARY) CHARSET utf8; + */ + bool prepare_charset_for_string(const Column_derived_attributes *dattr); + /** Prepare a SET/ENUM field. Create "interval" from "interval_list" if needed, and adjust "length". @@ -4800,7 +4814,13 @@ public: bool sp_prepare_create_field(THD *thd, MEM_ROOT *mem_root); bool prepare_stage1(THD *thd, MEM_ROOT *mem_root, - handler *file, ulonglong table_flags); + handler *file, ulonglong table_flags, + const Column_derived_attributes *derived_attr); + void prepare_stage1_simple(CHARSET_INFO *cs) + { + charset= cs; + create_length_to_internal_length_simple(); + } bool prepare_stage1_typelib(THD *thd, MEM_ROOT *mem_root, handler *file, ulonglong table_flags); bool prepare_stage1_string(THD *thd, MEM_ROOT *mem_root, @@ -4808,15 +4828,19 @@ public: bool prepare_stage1_bit(THD *thd, MEM_ROOT *mem_root, handler *file, ulonglong table_flags); + bool bulk_alter(const Column_derived_attributes *derived_attr, + const Column_bulk_alter_attributes *bulk_attr) + { + return type_handler()->Column_definition_bulk_alter(this, + derived_attr, + bulk_attr); + } void redefine_stage1_common(const Column_definition *dup_field, - const handler *file, - const Schema_specification_st *schema); - bool redefine_stage1(const Column_definition *dup_field, const handler *file, - const Schema_specification_st *schema) + const handler *file); + bool redefine_stage1(const Column_definition *dup_field, const handler *file) { const Type_handler *handler= dup_field->type_handler(); - return handler->Column_definition_redefine_stage1(this, dup_field, - file, schema); + return handler->Column_definition_redefine_stage1(this, dup_field, file); } bool prepare_stage2(handler *handler, ulonglong table_flags); bool prepare_stage2_blob(handler *handler, diff --git a/sql/handler.h b/sql/handler.h index 81a18137955..891187db171 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -2092,7 +2092,7 @@ public: struct Table_scope_and_contents_source_pod_st // For trivial members { - CHARSET_INFO *table_charset; + CHARSET_INFO *alter_table_convert_to_charset; LEX_CUSTRING tabledef_version; LEX_CSTRING connect_string; LEX_CSTRING comment; @@ -2237,7 +2237,7 @@ struct HA_CREATE_INFO: public Table_scope_and_contents_source_st, DBUG_ASSERT(cs); if (check_conflicting_charset_declarations(cs)) return true; - table_charset= default_table_charset= cs; + alter_table_convert_to_charset= default_table_charset= cs; used_fields|= (HA_CREATE_USED_CHARSET | HA_CREATE_USED_DEFAULT_CHARSET); return false; } diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 7ed1eb7aa52..373326c75c6 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -2388,6 +2388,8 @@ static int add_column_list_values(String *str, partition_info *part_info, */ if (create_info) { + const Column_derived_attributes + derived_attr(create_info->default_table_charset); Create_field *sql_field; if (!(sql_field= get_sql_field(field_name, @@ -2402,7 +2404,7 @@ static int add_column_list_values(String *str, partition_info *part_info, &need_cs_check)) return 1; if (need_cs_check) - field_cs= get_sql_field_charset(sql_field, create_info); + field_cs= sql_field->explicit_or_derived_charset(&derived_attr); else field_cs= NULL; } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index ac1984a074c..ea1a35a9ddd 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2926,6 +2926,15 @@ bool check_duplicates_in_interval(const char *set_or_name, } +bool Column_definition:: + prepare_charset_for_string(const Column_derived_attributes *dattr) +{ + if (!charset) + charset= dattr->charset(); + return (flags & BINCMP_FLAG) && !(charset= find_bin_collation(charset)); +} + + bool Column_definition::prepare_stage2_blob(handler *file, ulonglong table_flags, uint field_flags) @@ -3007,38 +3016,6 @@ bool Column_definition::prepare_stage2(handler *file, } -/* - Get character set from field object generated by parser using - default values when not set. - - SYNOPSIS - get_sql_field_charset() - sql_field The sql_field object - create_info Info generated by parser - - RETURN VALUES - cs Character set -*/ - -CHARSET_INFO* get_sql_field_charset(Column_definition *sql_field, - HA_CREATE_INFO *create_info) -{ - CHARSET_INFO *cs= sql_field->charset; - - if (!cs) - cs= create_info->default_table_charset; - /* - table_charset is set only in ALTER TABLE t1 CONVERT TO CHARACTER SET csname - if we want change character set for all varchar/char columns. - But the table charset must not affect the BLOB fields, so don't - allow to change my_charset_bin to somethig else. - */ - if (create_info->table_charset && cs != &my_charset_bin) - cs= create_info->table_charset; - return cs; -} - - /** Modifies the first column definition whose SQL type is TIMESTAMP by adding the features DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP. @@ -3211,11 +3188,14 @@ bool Column_definition::prepare_stage1_bit(THD *thd, bool Column_definition::prepare_stage1(THD *thd, MEM_ROOT *mem_root, handler *file, - ulonglong table_flags) + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) { return type_handler()->Column_definition_prepare_stage1(thd, mem_root, this, file, - table_flags); + table_flags, + derived_attr); } @@ -3428,6 +3408,9 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, int select_field_count= C_CREATE_SELECT(create_table_mode); bool tmp_table= create_table_mode == C_ALTER_TABLE; bool is_hash_field_needed= false; + const Column_derived_attributes dattr(create_info->default_table_charset); + const Column_bulk_alter_attributes + battr(create_info->alter_table_convert_to_charset); DBUG_ENTER("mysql_prepare_create_table"); DBUG_EXECUTE_IF("test_pseudo_invisible",{ @@ -3484,26 +3467,27 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, for (field_no=0; (sql_field=it++) ; field_no++) { + /* Virtual fields are always NULL */ + if (sql_field->vcol_info) + sql_field->flags&= ~NOT_NULL_FLAG; + /* Initialize length from its original value (number of characters), which was set in the parser. This is necessary if we're executing a prepared statement for the second time. */ sql_field->length= sql_field->char_length; - /* Set field charset. */ - sql_field->charset= get_sql_field_charset(sql_field, create_info); - if ((sql_field->flags & BINCMP_FLAG) && - !(sql_field->charset= find_bin_collation(sql_field->charset))) - DBUG_RETURN(true); - /* Virtual fields are always NULL */ - if (sql_field->vcol_info) - sql_field->flags&= ~NOT_NULL_FLAG; + if (sql_field->bulk_alter(&dattr, &battr)) + DBUG_RETURN(true); if (sql_field->prepare_stage1(thd, thd->mem_root, - file, file->ha_table_flags())) + file, file->ha_table_flags(), + &dattr)) DBUG_RETURN(true); + DBUG_ASSERT(sql_field->charset); + if (sql_field->real_field_type() == MYSQL_TYPE_BIT && file->ha_table_flags() & HA_CAN_BIT_FIELD) total_uneven_bit_length+= sql_field->length & 7; @@ -3554,7 +3538,7 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, if (!(sql_field->flags & NOT_NULL_FLAG)) null_fields--; - if (sql_field->redefine_stage1(dup_field, file, create_info)) + if (sql_field->redefine_stage1(dup_field, file)) DBUG_RETURN(true); it2.remove(); // Remove first (create) definition @@ -4569,7 +4553,9 @@ bool Column_definition::prepare_blob_field(THD *thd) bool Column_definition::sp_prepare_create_field(THD *thd, MEM_ROOT *mem_root) { - return prepare_stage1(thd, mem_root, NULL, HA_CAN_GEOMETRY) || + DBUG_ASSERT(charset); + const Column_derived_attributes dattr(&my_charset_bin); + return prepare_stage1(thd, mem_root, NULL, HA_CAN_GEOMETRY, &dattr) || prepare_stage2(NULL, HA_CAN_GEOMETRY); } @@ -11331,8 +11317,8 @@ bool Sql_cmd_create_table_like::execute(THD *thd) { create_info.used_fields&= ~HA_CREATE_USED_CHARSET; create_info.used_fields|= HA_CREATE_USED_DEFAULT_CHARSET; - create_info.default_table_charset= create_info.table_charset; - create_info.table_charset= 0; + create_info.default_table_charset= create_info.alter_table_convert_to_charset; + create_info.alter_table_convert_to_charset= 0; } /* diff --git a/sql/sql_table.h b/sql/sql_table.h index 35bff0873ea..62b61684286 100644 --- a/sql/sql_table.h +++ b/sql/sql_table.h @@ -252,8 +252,6 @@ bool quick_rm_table(THD *thd, handlerton *base, const LEX_CSTRING *db, const char *table_path=0); void close_cached_table(THD *thd, TABLE *table); void sp_prepare_create_field(THD *thd, Column_definition *sql_field); -CHARSET_INFO* get_sql_field_charset(Column_definition *sql_field, - HA_CREATE_INFO *create_info); bool mysql_write_frm(ALTER_PARTITION_PARAM_TYPE *lpt, uint flags); int write_bin_log(THD *thd, bool clear_error, char const *query, ulong query_length, diff --git a/sql/sql_type.cc b/sql/sql_type.cc index 7dea9cdea4a..e92dc97771e 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -2671,9 +2671,12 @@ bool Type_handler:: MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { - def->create_length_to_internal_length_simple(); + def->prepare_stage1_simple(&my_charset_bin); return false; } @@ -2682,8 +2685,12 @@ bool Type_handler_null:: MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { + def->prepare_charset_for_string(derived_attr); def->create_length_to_internal_length_null(); return false; } @@ -2693,19 +2700,56 @@ bool Type_handler_row:: MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { + def->charset= &my_charset_bin; def->create_length_to_internal_length_null(); return false; } +bool Type_handler_temporal_result:: + Column_definition_prepare_stage1(THD *thd, + MEM_ROOT *mem_root, + Column_definition *def, + handler *file, + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const +{ + def->prepare_stage1_simple(&my_charset_numeric); + return false; +} + + +bool Type_handler_numeric:: + Column_definition_prepare_stage1(THD *thd, + MEM_ROOT *mem_root, + Column_definition *def, + handler *file, + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const +{ + def->prepare_stage1_simple(&my_charset_numeric); + return false; +} + bool Type_handler_newdecimal:: Column_definition_prepare_stage1(THD *thd, MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { + def->charset= &my_charset_numeric; def->create_length_to_internal_length_newdecimal(); return false; } @@ -2715,8 +2759,12 @@ bool Type_handler_bit:: MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { + def->charset= &my_charset_numeric; return def->prepare_stage1_bit(thd, mem_root, file, table_flags); } @@ -2725,9 +2773,13 @@ bool Type_handler_typelib:: MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { - return def->prepare_stage1_typelib(thd, mem_root, file, table_flags); + return def->prepare_charset_for_string(derived_attr) || + def->prepare_stage1_typelib(thd, mem_root, file, table_flags); } @@ -2736,36 +2788,71 @@ bool Type_handler_string_result:: MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { - return def->prepare_stage1_string(thd, mem_root, file, table_flags); + return def->prepare_charset_for_string(derived_attr) || + def->prepare_stage1_string(thd, mem_root, file, table_flags); } #ifdef HAVE_SPATIAL +#if MYSQL_VERSION_ID > 100500 +#error The below method is in sql/sql_type_geom.cc starting from 10.5 +#endif bool Type_handler_geometry:: Column_definition_prepare_stage1(THD *thd, MEM_ROOT *mem_root, Column_definition *def, handler *file, - ulonglong table_flags) const + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const { + def->charset= &my_charset_bin; def->create_length_to_internal_length_string(); return def->prepare_blob_field(thd); } #endif +/*************************************************************************/ + +bool Type_handler_general_purpose_string:: + Column_definition_bulk_alter(Column_definition *def, + const Column_derived_attributes + *derived_attr, + const Column_bulk_alter_attributes + *bulk_alter_attr) + const +{ + if (!bulk_alter_attr->alter_table_convert_to_charset()) + return false; // No "CONVERT TO" clause. + CHARSET_INFO *defcs= def->explicit_or_derived_charset(derived_attr); + DBUG_ASSERT(defcs); + /* + Handle 'ALTER TABLE t1 CONVERT TO CHARACTER SET csname'. + Change character sets for all varchar/char/text columns, + but do not touch varbinary/binary/blob columns. + */ + if (defcs != &my_charset_bin) + def->charset= bulk_alter_attr->alter_table_convert_to_charset(); + return false; +}; + + /*************************************************************************/ bool Type_handler:: Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const { - def->redefine_stage1_common(dup, file, schema); + def->redefine_stage1_common(dup, file); def->create_length_to_internal_length_simple(); return false; } @@ -2774,11 +2861,10 @@ bool Type_handler:: bool Type_handler_null:: Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const { - def->redefine_stage1_common(dup, file, schema); + def->redefine_stage1_common(dup, file); def->create_length_to_internal_length_null(); return false; } @@ -2787,11 +2873,10 @@ bool Type_handler_null:: bool Type_handler_newdecimal:: Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const { - def->redefine_stage1_common(dup, file, schema); + def->redefine_stage1_common(dup, file); def->create_length_to_internal_length_newdecimal(); return false; } @@ -2800,11 +2885,10 @@ bool Type_handler_newdecimal:: bool Type_handler_string_result:: Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const { - def->redefine_stage1_common(dup, file, schema); + def->redefine_stage1_common(dup, file); def->set_compression_method(dup->compression_method()); def->create_length_to_internal_length_string(); return false; @@ -2814,11 +2898,10 @@ bool Type_handler_string_result:: bool Type_handler_typelib:: Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const { - def->redefine_stage1_common(dup, file, schema); + def->redefine_stage1_common(dup, file); def->create_length_to_internal_length_typelib(); return false; } @@ -2827,11 +2910,10 @@ bool Type_handler_typelib:: bool Type_handler_bit:: Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const { - def->redefine_stage1_common(dup, file, schema); + def->redefine_stage1_common(dup, file); /* If we are replacing a field with a BIT field, we need to initialize pack_flag. diff --git a/sql/sql_type.h b/sql/sql_type.h index 6958d4f970f..7a514643bf6 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -76,7 +76,6 @@ class Spvar_definition; struct st_value; class Protocol; class handler; -struct Schema_specification_st; struct TABLE; struct SORT_FIELD_ATTR; class Vers_history_point; @@ -110,6 +109,53 @@ enum scalar_comparison_op }; +/* + A helper class to store column attributes that are inherited + by columns (from the table level) when not specified explicitly. +*/ +class Column_derived_attributes +{ + /* + Table level CHARACTER SET and COLLATE value: + + CREATE TABLE t1 (a VARCHAR(1), b CHAR(2)) CHARACTER SET latin1; + + All character string columns (CHAR, VARCHAR, TEXT) + inherit CHARACTER SET from the table level. + */ + CHARSET_INFO *m_charset; +public: + explicit Column_derived_attributes(CHARSET_INFO *cs) + :m_charset(cs) + { } + CHARSET_INFO *charset() const { return m_charset; } +}; + + +/* + A helper class to store requests for changes + in multiple column data types during ALTER. +*/ +class Column_bulk_alter_attributes +{ + /* + Target CHARACTER SET specification in ALTER .. CONVERT, e.g. + + ALTER TABLE t1 CONVERT TO CHARACTER SET utf8; + + All character string columns (CHAR, VARCHAR, TEXT) + get converted to the "CONVERT TO CHARACTER SET". + */ + CHARSET_INFO *m_alter_table_convert_to_charset; +public: + explicit Column_bulk_alter_attributes(CHARSET_INFO *convert) + :m_alter_table_convert_to_charset(convert) + { } + CHARSET_INFO *alter_table_convert_to_charset() const + { return m_alter_table_convert_to_charset; } +}; + + class Native: public Binary_string { public: @@ -3597,7 +3643,17 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; + virtual bool Column_definition_bulk_alter(Column_definition *c, + const Column_derived_attributes + *derived_attr, + const Column_bulk_alter_attributes + *bulk_alter_attr) + const + { return false; } /* This method is called on queries like: CREATE TABLE t2 (a INT) AS SELECT a FROM t1; @@ -3616,9 +3672,7 @@ public: */ virtual bool Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st * - schema) + const handler *file) const; virtual bool Column_definition_prepare_stage2(Column_definition *c, handler *file, @@ -4008,11 +4062,13 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const { DBUG_ASSERT(0); @@ -4294,6 +4350,14 @@ class Type_handler_numeric: public Type_handler { public: String *print_item_value(THD *thd, Item *item, String *str) const; + bool Column_definition_prepare_stage1(THD *thd, + MEM_ROOT *mem_root, + Column_definition *c, + handler *file, + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; double Item_func_min_max_val_real(Item_func_min_max *) const; longlong Item_func_min_max_val_int(Item_func_min_max *) const; my_decimal *Item_func_min_max_val_decimal(Item_func_min_max *, @@ -4734,6 +4798,14 @@ public: void sortlength(THD *thd, const Type_std_attributes *item, SORT_FIELD_ATTR *attr) const; + bool Column_definition_prepare_stage1(THD *thd, + MEM_ROOT *mem_root, + Column_definition *c, + handler *file, + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Item_const_eq(const Item_const *a, const Item_const *b, bool binary_cmp) const; bool Item_param_set_from_value(THD *thd, @@ -4820,11 +4892,13 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const; uint32 max_display_length(const Item *item) const; /* @@ -4935,6 +5009,12 @@ class Type_handler_general_purpose_string: public Type_handler_string_result public: bool is_general_purpose_string_type() const { return true; } bool Vers_history_point_resolve_unit(THD *thd, Vers_history_point *p) const; + bool Column_definition_bulk_alter(Column_definition *c, + const Column_derived_attributes + *derived_attr, + const Column_bulk_alter_attributes + *bulk_alter_attr) + const; }; @@ -5291,11 +5371,13 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const; bool Column_definition_prepare_stage2(Column_definition *c, handler *file, @@ -5975,11 +6057,13 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const; bool Column_definition_prepare_stage2(Column_definition *c, handler *file, @@ -6021,11 +6105,13 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const; bool Column_definition_prepare_stage2(Column_definition *c, handler *file, @@ -6348,7 +6434,10 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Column_definition_prepare_stage2(Column_definition *c, handler *file, ulonglong table_flags) const; @@ -6424,11 +6513,13 @@ public: MEM_ROOT *mem_root, Column_definition *c, handler *file, - ulonglong table_flags) const; + ulonglong table_flags, + const Column_derived_attributes + *derived_attr) + const; bool Column_definition_redefine_stage1(Column_definition *def, const Column_definition *dup, - const handler *file, - const Schema_specification_st *schema) + const handler *file) const; void Item_param_set_param_func(Item_param *param, uchar **pos, ulong len) const; diff --git a/sql/table.cc b/sql/table.cc index b7c3a33aa40..12299271ab3 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -4479,7 +4479,7 @@ void update_create_info_from_table(HA_CREATE_INFO *create_info, TABLE *table) create_info->row_type= share->row_type; create_info->key_block_size= share->key_block_size; create_info->default_table_charset= share->table_charset; - create_info->table_charset= 0; + create_info->alter_table_convert_to_charset= 0; create_info->comment= share->comment; create_info->transactional= share->transactional; create_info->page_checksum= share->page_checksum; diff --git a/storage/federatedx/ha_federatedx.cc b/storage/federatedx/ha_federatedx.cc index de4e20ee112..19b56980714 100644 --- a/storage/federatedx/ha_federatedx.cc +++ b/storage/federatedx/ha_federatedx.cc @@ -3419,7 +3419,9 @@ int ha_federatedx::create(const char *name, TABLE *table_arg, { FEDERATEDX_SERVER server; - fill_server(thd->mem_root, &server, &tmp_share, create_info->table_charset); + // It's possibly wrong to use alter_table_convert_to_charset here. + fill_server(thd->mem_root, &server, &tmp_share, + create_info->alter_table_convert_to_charset); #ifndef DBUG_OFF mysql_mutex_init(fe_key_mutex_FEDERATEDX_SERVER_mutex, From 4e2ca42225311f73745c9eec309bc941183aba30 Mon Sep 17 00:00:00 2001 From: Monty Date: Tue, 6 Apr 2021 16:37:11 +0300 Subject: [PATCH 8/9] MDEV-25334 FTWRL/Backup blocks DDL on temporary tables with binlog enabled, assertion fails in Diagnostics_area::set_error_status Fixed by adding a MDL_BACKUP_COMMIT lock before altering temporary tables whose creation was logged to binary log (in which case the ALTER TABLE must also be logged) --- mysql-test/main/backup_lock_binlog.result | 40 ++++++++++++++++++ mysql-test/main/backup_lock_binlog.test | 49 +++++++++++++++++++++++ sql/sql_table.cc | 21 ++++++++-- 3 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 mysql-test/main/backup_lock_binlog.result create mode 100644 mysql-test/main/backup_lock_binlog.test diff --git a/mysql-test/main/backup_lock_binlog.result b/mysql-test/main/backup_lock_binlog.result new file mode 100644 index 00000000000..adf960a9cb1 --- /dev/null +++ b/mysql-test/main/backup_lock_binlog.result @@ -0,0 +1,40 @@ +# +# MDEV-25334 FTWRL/Backup blocks DDL on temporary tables with binlog +# enabled assertion fails in Diagnostics_area::set_error_status +# +select @@binlog_format; +@@binlog_format +MIXED +connect con1,localhost,root,,; +connection default; +# +# Test 1 +# +CREATE TEMPORARY TABLE tmp (a INT); +connection con1; +FLUSH TABLES WITH READ LOCK; +connection default; +SET lock_wait_timeout= 1; +ALTER TABLE tmp; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +connection con1; +unlock tables; +connection default; +drop table tmp; +# +# Test 2 (In statement format to ensure temporary table gets logged) +# +set @@binlog_format=statement; +CREATE TEMPORARY TABLE tmp (a INT); +connection con1; +BACKUP STAGE START; +BACKUP STAGE BLOCK_COMMIT; +connection default; +SET lock_wait_timeout= 1; +ALTER TABLE tmp; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +connection con1; +BACKUP STAGE end; +connection default; +drop table tmp; +disconnect con1; diff --git a/mysql-test/main/backup_lock_binlog.test b/mysql-test/main/backup_lock_binlog.test new file mode 100644 index 00000000000..45b3f1cfbd9 --- /dev/null +++ b/mysql-test/main/backup_lock_binlog.test @@ -0,0 +1,49 @@ +--source include/have_binlog_format_mixed_or_statement.inc + +# +# Tests involving locks and binlog +# + +--echo # +--echo # MDEV-25334 FTWRL/Backup blocks DDL on temporary tables with binlog +--echo # enabled assertion fails in Diagnostics_area::set_error_status +--echo # + +select @@binlog_format; +--connect (con1,localhost,root,,) +connection default; + +--echo # +--echo # Test 1 +--echo # + +CREATE TEMPORARY TABLE tmp (a INT); +--connection con1 +FLUSH TABLES WITH READ LOCK; +--connection default +SET lock_wait_timeout= 1; +--error ER_LOCK_WAIT_TIMEOUT +ALTER TABLE tmp; +--connection con1 +unlock tables; +--connection default +drop table tmp; + +--echo # +--echo # Test 2 (In statement format to ensure temporary table gets logged) +--echo # + +set @@binlog_format=statement; +CREATE TEMPORARY TABLE tmp (a INT); +--connection con1 +BACKUP STAGE START; +BACKUP STAGE BLOCK_COMMIT; +--connection default +SET lock_wait_timeout= 1; +--error ER_LOCK_WAIT_TIMEOUT +ALTER TABLE tmp; +--connection con1 +BACKUP STAGE end; +--connection default +drop table tmp; +--disconnect con1 diff --git a/sql/sql_table.cc b/sql/sql_table.cc index ea1a35a9ddd..680258141f2 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -9574,7 +9574,8 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db, If such table exists, there must be a corresponding TABLE_SHARE in THD::all_temp_tables list. */ - if (thd->find_tmp_table_share(alter_ctx.new_db.str, alter_ctx.new_name.str)) + if (thd->find_tmp_table_share(alter_ctx.new_db.str, + alter_ctx.new_name.str)) { my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alter_ctx.new_alias.str); DBUG_RETURN(true); @@ -9715,6 +9716,17 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db, if (table->s->tmp_table == NO_TMP_TABLE) mysql_audit_alter_table(thd, table_list); + else if (table->s->table_creation_was_logged && mysql_bin_log.is_open()) + { + /* Protect against MDL error in binary logging */ + MDL_request mdl_request; + DBUG_ASSERT(!mdl_ticket); + mdl_request.init(MDL_key::BACKUP, "", "", MDL_BACKUP_COMMIT, + MDL_TRANSACTION); + if (thd->mdl_context.acquire_lock(&mdl_request, + thd->variables.lock_wait_timeout)) + DBUG_RETURN(true); + } THD_STAGE_INFO(thd, stage_setup); @@ -9794,9 +9806,12 @@ do_continue:; thd->get_stmt_da()->current_statement_warn_count()); my_ok(thd, 0L, 0L, alter_ctx.tmp_buff); - /* We don't replicate alter table statement on temporary tables */ + /* + We don't replicate alter table statement on temporary tables + For which we did not log the CREATE TEMPORARY TABLE statement. + */ if (table->s->tmp_table == NO_TMP_TABLE || - !thd->is_current_stmt_binlog_format_row()) + table->s->table_creation_was_logged) { if (write_bin_log(thd, true, thd->query(), thd->query_length())) DBUG_RETURN(true); From 0ba845a8c7e59eec3f7cb84baf924bd5a7190a13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 8 Apr 2021 09:24:03 +0300 Subject: [PATCH 9/9] MDEV-17913 fixup: Correct a DBUG_PRINT format string --- storage/maria/ma_open.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/maria/ma_open.c b/storage/maria/ma_open.c index d45907a44b9..d346c606c4e 100644 --- a/storage/maria/ma_open.c +++ b/storage/maria/ma_open.c @@ -569,7 +569,7 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags, ((share->state.changed & (STATE_MOVED | STATE_NOT_ZEROFILLED)) == (STATE_MOVED | STATE_NOT_ZEROFILLED)))) { - DBUG_PRINT("warning", ("table is moved from another system. uuid_diff: %d create_trid: %lu max_trid: %lu moved: &d", + DBUG_PRINT("warning", ("table is moved from another system. uuid_diff: %d create_trid: %lu max_trid: %lu moved: %d", memcmp(share->base.uuid, maria_uuid, MY_UUID_SIZE) != 0, (ulong) share->state.create_trid,