From 5e044f78c0a9a8cd40dedff0e4bc857c0bd76b95 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Sat, 16 Mar 2019 15:08:17 -0700 Subject: [PATCH 1/8] MDEV-18945 Assertion `fixed == 1' failed in Item_cond_and::val_int In the function make_cond_for_table_from_pred a call of ix_fields() missed checking of the return code. As a result an extracted constant condition could be not well formed and this caused an assertion failure. --- mysql-test/r/update.result | 13 +++++++++++++ mysql-test/t/update.test | 19 +++++++++++++++++++ sql/sql_select.cc | 3 ++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/update.result b/mysql-test/r/update.result index 73ebb73e313..9e19abc4e9c 100644 --- a/mysql-test/r/update.result +++ b/mysql-test/r/update.result @@ -719,3 +719,16 @@ Handler_read_rnd_deleted 0 Handler_read_rnd_next 0 drop table t1, t2; # End of MariaDB 10.0 tests +# +# MDEV-18945: multi-table update with constant table and +# comparison of date field with integer field +# +CREATE TABLE t1 (i1 int, d1 date , i2 int , d2 date) engine=myisam; +INSERT INTO t1 VALUES (19,'0000-00-00',73,'2008-05-21'); +CREATE TABLE t2 (d1 date , i1 int, i2 int , d2 date) engine=myisam; +INSERT INTO t2 VALUES +('2006-01-12',-102,45,'2023-11-25'),('2034-12-19',-102,45,'2001-11-20'); +UPDATE t1,t2 SET t1.i1 = -39 WHERE t2.d1 <> t1.i1 AND t2.d1 = t1.d2; +ERROR 22007: Incorrect datetime value: '19' for column `test`.`t1`.`i1` at row 1 +DROP TABLE t1,t2; +# End of MariaDB 10.2 tests diff --git a/mysql-test/t/update.test b/mysql-test/t/update.test index e5ef0b11127..84709102f96 100644 --- a/mysql-test/t/update.test +++ b/mysql-test/t/update.test @@ -654,3 +654,22 @@ show status like 'Handler_read%'; drop table t1, t2; --echo # End of MariaDB 10.0 tests + +--echo # +--echo # MDEV-18945: multi-table update with constant table and +--echo # comparison of date field with integer field +--echo # + +CREATE TABLE t1 (i1 int, d1 date , i2 int , d2 date) engine=myisam; +INSERT INTO t1 VALUES (19,'0000-00-00',73,'2008-05-21'); + +CREATE TABLE t2 (d1 date , i1 int, i2 int , d2 date) engine=myisam; +INSERT INTO t2 VALUES + ('2006-01-12',-102,45,'2023-11-25'),('2034-12-19',-102,45,'2001-11-20'); + +--error ER_TRUNCATED_WRONG_VALUE +UPDATE t1,t2 SET t1.i1 = -39 WHERE t2.d1 <> t1.i1 AND t2.d1 = t1.d2; + +DROP TABLE t1,t2; + +--echo # End of MariaDB 10.2 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 1dfd6520e17..6253c39b777 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -20614,7 +20614,8 @@ make_cond_for_table_from_pred(THD *thd, Item *root_cond, Item *cond, the new parent Item. This should not be expensive because all children of Item_cond_and should be fixed by now. */ - new_cond->fix_fields(thd, 0); + if (new_cond->fix_fields(thd, 0)) + return (COND*) 0; new_cond->used_tables_cache= ((Item_cond_and*) cond)->used_tables_cache & tables; From 26432e49d37a37d09b862bb49a021e44bdf4789c Mon Sep 17 00:00:00 2001 From: sysprg Date: Mon, 18 Mar 2019 06:39:51 +0100 Subject: [PATCH 2/8] MDEV-17262: mysql crashed on galera while node rejoined cluster (#895) This patch contains a fix for the MDEV-17262/17243 issues and new mtr test. These issues (MDEV-17262/17243) have two reasons: 1) After an intermediate commit, a transaction loses its status of "transaction that registered in the MySQL for 2pc coordinator" (in the InnoDB) due to the fact that since version 10.2 the write_row() function (which located in the ha_innodb.cc) does not call trx_register_for_2pc(m_prebuilt->trx) during the processing of split transactions. It is necessary to restore this call inside the write_row() when an intermediate commit was made (for a split transaction). Similarly, we need to set the flag of the started transaction (m_prebuilt->sql_stat_start) after intermediate commit. The table->file->extra(HA_EXTRA_FAKE_START_STMT) called from the wsrep_load_data_split() function (which located in sql_load.cc) will also do this, but it will be too late. As a result, the call to the wsrep_append_keys() function from the InnoDB engine may be lost or function may be called with invalid transaction identifier. 2) If a transaction with the LOAD DATA statement is divided into logical mini-transactions (of the 10K rows) and binlog is rotated, then in rare cases due to the wsrep handler re-registration at the boundary of the split, the last portion of data may be lost. Since splitting of the LOAD DATA into mini-transactions is technical, I believe that we should not allow these mini-transactions to fall into separate binlogs. Therefore, it is necessary to prohibit the rotation of binlog in the middle of processing LOAD DATA statement. https://jira.mariadb.org/browse/MDEV-17262 and https://jira.mariadb.org/browse/MDEV-17243 --- include/mysql/service_wsrep.h | 6 + .../r/galera_load_data_ist.result | 36 +++++ .../galera_3nodes/t/galera_load_data_ist.cnf | 4 + .../galera_3nodes/t/galera_load_data_ist.test | 124 ++++++++++++++++++ sql/log.cc | 51 +++++++ sql/sql_class.cc | 2 + sql/sql_class.h | 8 ++ sql/sql_load.cc | 55 +++++--- sql/sql_plugin_services.ic | 2 + sql/wsrep_dummy.cc | 6 + sql/wsrep_hton.cc | 1 + sql/wsrep_thd.cc | 10 ++ storage/innobase/handler/ha_innodb.cc | 10 ++ 13 files changed, 295 insertions(+), 20 deletions(-) create mode 100644 mysql-test/suite/galera_3nodes/r/galera_load_data_ist.result create mode 100644 mysql-test/suite/galera_3nodes/t/galera_load_data_ist.cnf create mode 100644 mysql-test/suite/galera_3nodes/t/galera_load_data_ist.test diff --git a/include/mysql/service_wsrep.h b/include/mysql/service_wsrep.h index ee28856ac73..7237cccfaeb 100644 --- a/include/mysql/service_wsrep.h +++ b/include/mysql/service_wsrep.h @@ -108,6 +108,8 @@ extern struct wsrep_service_st { long long (*wsrep_thd_trx_seqno_func)(THD *thd); struct wsrep_ws_handle * (*wsrep_thd_ws_handle_func)(THD *thd); void (*wsrep_thd_auto_increment_variables_func)(THD *thd, unsigned long long *offset, unsigned long long *increment); + void (*wsrep_set_load_multi_commit_func)(THD *thd, bool split); + bool (*wsrep_is_load_multi_commit_func)(THD *thd); int (*wsrep_trx_is_aborting_func)(MYSQL_THD thd); int (*wsrep_trx_order_before_func)(MYSQL_THD, MYSQL_THD); void (*wsrep_unlock_rollback_func)(); @@ -152,6 +154,8 @@ extern struct wsrep_service_st { #define wsrep_thd_trx_seqno(T) wsrep_service->wsrep_thd_trx_seqno_func(T) #define wsrep_thd_ws_handle(T) wsrep_service->wsrep_thd_ws_handle_func(T) #define wsrep_thd_auto_increment_variables(T,O,I) wsrep_service->wsrep_thd_auto_increment_variables_func(T,O,I) +#define wsrep_set_load_multi_commit(T,S) wsrep_service->wsrep_set_load_multi_commit_func(T,S) +#define wsrep_is_load_multi_commit(T) wsrep_service->wsrep_is_load_multi_commit_func(T) #define wsrep_trx_is_aborting(T) wsrep_service->wsrep_trx_is_aborting_func(T) #define wsrep_trx_order_before(T1,T2) wsrep_service->wsrep_trx_order_before_func(T1,T2) #define wsrep_unlock_rollback() wsrep_service->wsrep_unlock_rollback_func() @@ -206,6 +210,8 @@ my_bool wsrep_thd_is_wsrep(MYSQL_THD thd); struct wsrep *get_wsrep(); struct wsrep_ws_handle *wsrep_thd_ws_handle(THD *thd); void wsrep_thd_auto_increment_variables(THD *thd, unsigned long long *offset, unsigned long long *increment); +void wsrep_set_load_multi_commit(THD *thd, bool split); +bool wsrep_is_load_multi_commit(THD *thd); void wsrep_aborting_thd_enqueue(THD *thd); void wsrep_lock_rollback(); void wsrep_post_commit(THD* thd, bool all); diff --git a/mysql-test/suite/galera_3nodes/r/galera_load_data_ist.result b/mysql-test/suite/galera_3nodes/r/galera_load_data_ist.result new file mode 100644 index 00000000000..cfb897e1076 --- /dev/null +++ b/mysql-test/suite/galera_3nodes/r/galera_load_data_ist.result @@ -0,0 +1,36 @@ +connection node_1; +connection node_2; +connection node_3; +connection node_1; +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY) ENGINE=InnoDB; +connection node_2; +connection node_3; +SET GLOBAL wsrep_provider_options = 'gmcast.isolate = 1'; +SET SESSION wsrep_on = OFF; +SET SESSION wsrep_on = ON; +SET SESSION wsrep_sync_wait = 0; +connection node_2a; +SET SESSION wsrep_sync_wait = 0; +connection node_2; +SET GLOBAL wsrep_load_data_splitting = TRUE; +SET DEBUG_SYNC='intermediate_transaction_commit SIGNAL commited WAIT_FOR ist'; +connection node_2a; +SET DEBUG_SYNC='now WAIT_FOR commited'; +connection node_3; +SET GLOBAL wsrep_provider_options = 'gmcast.isolate = 0'; +connection node_2a; +SET DEBUG_SYNC='now SIGNAL ist'; +connection node_1; +connection node_2; +SET DEBUG_SYNC='RESET'; +SELECT COUNT(*) = 95000 FROM t1; +COUNT(*) = 95000 +1 +wsrep_last_committed_diff +1 +connection node_1; +SET GLOBAL wsrep_load_data_splitting = 1;; +DROP TABLE t1; +disconnect node_3; +disconnect node_2; +disconnect node_1; diff --git a/mysql-test/suite/galera_3nodes/t/galera_load_data_ist.cnf b/mysql-test/suite/galera_3nodes/t/galera_load_data_ist.cnf new file mode 100644 index 00000000000..35ecb8b5937 --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/galera_load_data_ist.cnf @@ -0,0 +1,4 @@ +!include ../galera_3nodes.cnf + +[mysqld] +wsrep-causal-reads=OFF diff --git a/mysql-test/suite/galera_3nodes/t/galera_load_data_ist.test b/mysql-test/suite/galera_3nodes/t/galera_load_data_ist.test new file mode 100644 index 00000000000..e1140da229b --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/galera_load_data_ist.test @@ -0,0 +1,124 @@ +--source include/have_debug_sync.inc +--source include/galera_cluster.inc +--source include/have_innodb.inc +--source include/big_test.inc + +# Establish connection to the third node: +--let $galera_connection_name = node_3 +--let $galera_server_number = 3 +--source include/galera_connect.inc + +# Establish additional connection to the second node +# (which is used in the test for synchronization control): +--let $galera_connection_name = node_2a +--let $galera_server_number = 2 +--source include/galera_connect.inc + +# 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 + +# Create a file for LOAD DATA with 95K entries +--connection node_1 +--perl +open(FILE, ">", "$ENV{'MYSQLTEST_VARDIR'}/tmp/galera_var_load_data_splitting.csv") or die; +foreach my $i (1..95000) { + print FILE "$i\n"; +} +EOF + +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY) ENGINE=InnoDB; + +# Let's wait for the completion of the formation of a cluster +# of three nodes: +--let $wait_condition = SELECT VARIABLE_VALUE = 3 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; +--source include/wait_condition.inc +--connection node_2 +--source include/wait_until_ready.inc +--connection node_3 +--source include/wait_until_ready.inc + +# Disconnect the third node from the cluster: +SET GLOBAL wsrep_provider_options = 'gmcast.isolate = 1'; +SET SESSION wsrep_on = OFF; +--let $wait_condition = SELECT VARIABLE_VALUE = 'non-Primary' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_status'; +--source include/wait_condition.inc +SET SESSION wsrep_on = ON; +SET SESSION wsrep_sync_wait = 0; + +# Disable sync wait for control connection: +--connection node_2a +SET SESSION wsrep_sync_wait = 0; + +# Let's wait until the other nodes stop seeing the third +# node in the cluster: +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; +--source include/wait_condition.inc + +# Record wsrep_last_committed as it was before LOAD DATA: +--connection node_2 +--let $wsrep_last_committed_before = `SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME = 'wsrep_last_committed'` + +# Enable splitting for LOAD DATA: +--let $wsrep_load_data_splitting_orig = `SELECT @@wsrep_load_data_splitting` +SET GLOBAL wsrep_load_data_splitting = TRUE; + +# Stop after the first commit and wait for the IST signal: +SET DEBUG_SYNC='intermediate_transaction_commit SIGNAL commited WAIT_FOR ist'; + +# Perform the LOAD DATA statement: +--disable_query_log +let v1='$MYSQLTEST_VARDIR/tmp/galera_var_load_data_splitting.csv'; +--send_eval LOAD DATA INFILE $v1 INTO TABLE t1; +--enable_query_log + +# Wait for the first commit: +--connection node_2a +SET DEBUG_SYNC='now WAIT_FOR commited'; + +# Initiate the IST: +--connection node_3 +SET GLOBAL wsrep_provider_options = 'gmcast.isolate = 0'; + +# Continue the execution of LOAD DATA: +--connection node_2a +SET DEBUG_SYNC='now SIGNAL ist'; + +# Let's wait for the recovery of the cluster +# of three nodes: +--connection node_1 +--let $wait_condition = SELECT VARIABLE_VALUE = 3 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; +--source include/wait_condition.inc + +# Save the LOAD DATA results: +--connection node_2 +--reap + +# Reset all synchronization points and signals: +SET DEBUG_SYNC='RESET'; + +# Read the wsrep_last_commited after LOAD DATA: +--let $wsrep_last_committed_after = `SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME = 'wsrep_last_committed'` + +# Check the records: +SELECT COUNT(*) = 95000 FROM t1; + +# LOAD-ing 95K rows should causes 10 commits to be registered: +--disable_query_log +--eval SELECT $wsrep_last_committed_after = $wsrep_last_committed_before + 10 AS wsrep_last_committed_diff; +--enable_query_log + +# Restore the original splitting: +--connection node_1 +--eval SET GLOBAL wsrep_load_data_splitting = $wsrep_load_data_splitting_orig; + +# Drop test table: +DROP TABLE t1; + +# Restore original auto_increment_offset values: +--source ../galera/include/auto_increment_offset_restore.inc + +--let $galera_cluster_size=3 +--source include/galera_end.inc diff --git a/sql/log.cc b/sql/log.cc index 4d62c9783cd..df928e89390 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -6413,8 +6413,25 @@ err: update_binlog_end_pos(offset); signal_update(); + /* + If a transaction with the LOAD DATA statement is divided + into logical mini-transactions (of the 10K rows) and binlog + is rotated, then the last portion of data may be lost due to + wsrep handler re-registration at the boundary of the split. + Since splitting of the LOAD DATA into mini-transactions is + logical, we should not allow these mini-transactions to fall + into separate binlogs. Therefore, it is necessary to prohibit + the rotation of binlog in the middle of processing LOAD DATA: + */ +#ifdef WITH_WSREP + if (!thd->wsrep_split_flag) + { +#endif /* WITH_WSREP */ if ((error= rotate(false, &check_purge))) check_purge= false; +#ifdef WITH_WSREP + } +#endif /* WITH_WSREP */ } } } @@ -7139,8 +7156,25 @@ bool MYSQL_BIN_LOG::write_incident(THD *thd) !(error= flush_and_sync(0))) { signal_update(); + /* + If a transaction with the LOAD DATA statement is divided + into logical mini-transactions (of the 10K rows) and binlog + is rotated, then the last portion of data may be lost due to + wsrep handler re-registration at the boundary of the split. + Since splitting of the LOAD DATA into mini-transactions is + logical, we should not allow these mini-transactions to fall + into separate binlogs. Therefore, it is necessary to prohibit + the rotation of binlog in the middle of processing LOAD DATA: + */ +#ifdef WITH_WSREP + if (!thd->wsrep_split_flag) + { +#endif /* WITH_WSREP */ if ((error= rotate(false, &check_purge))) check_purge= false; +#ifdef WITH_WSREP + } +#endif /* WITH_WSREP */ } offset= my_b_tell(&log_file); @@ -7906,6 +7940,20 @@ MYSQL_BIN_LOG::trx_group_commit_leader(group_commit_entry *leader) mark_xids_active(binlog_id, xid_count); } + /* + If a transaction with the LOAD DATA statement is divided + into logical mini-transactions (of the 10K rows) and binlog + is rotated, then the last portion of data may be lost due to + wsrep handler re-registration at the boundary of the split. + Since splitting of the LOAD DATA into mini-transactions is + logical, we should not allow these mini-transactions to fall + into separate binlogs. Therefore, it is necessary to prohibit + the rotation of binlog in the middle of processing LOAD DATA: + */ +#ifdef WITH_WSREP + if (!leader->thd->wsrep_split_flag) + { +#endif /* WITH_WSREP */ if (rotate(false, &check_purge)) { /* @@ -7925,6 +7973,9 @@ MYSQL_BIN_LOG::trx_group_commit_leader(group_commit_entry *leader) my_error(ER_ERROR_ON_WRITE, MYF(ME_NOREFRESH), name, errno); check_purge= false; } +#ifdef WITH_WSREP + } +#endif /* WITH_WSREP */ /* In case of binlog rotate, update the correct current binlog offset. */ commit_offset= my_b_write_tell(&log_file); } diff --git a/sql/sql_class.cc b/sql/sql_class.cc index d6aa6456710..512f7fdfd56 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -784,6 +784,7 @@ THD::THD(my_thread_id id, bool is_wsrep_applier) wsrep_affected_rows = 0; wsrep_replicate_GTID = false; wsrep_skip_wsrep_GTID = false; + wsrep_split_flag = false; #endif /* Call to init() below requires fully initialized Open_tables_state. */ reset_open_tables_state(this); @@ -1218,6 +1219,7 @@ void THD::init(void) wsrep_affected_rows = 0; wsrep_replicate_GTID = false; wsrep_skip_wsrep_GTID = false; + wsrep_split_flag = false; #endif /* WITH_WSREP */ if (variables.sql_log_bin) diff --git a/sql/sql_class.h b/sql/sql_class.h index d701d4cb46c..cb182f55bf1 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -4431,6 +4431,14 @@ public: ulong wsrep_affected_rows; bool wsrep_replicate_GTID; bool wsrep_skip_wsrep_GTID; + /* This flag is set when innodb do an intermediate commit to + processing the LOAD DATA INFILE statement by splitting it into 10K + rows chunks. If flag is set, then binlog rotation is not performed + while intermediate transaction try to commit, because in this case + rotation causes unregistration of innodb handler. Later innodb handler + registered again, but replication of last chunk of rows is skipped + by the innodb engine: */ + bool wsrep_split_flag; #endif /* WITH_WSREP */ /* Handling of timeouts for commands */ diff --git a/sql/sql_load.cc b/sql/sql_load.cc index 8c2f17dac3f..8e0bdcb32b8 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -41,6 +41,7 @@ #include "sql_trigger.h" #include "sql_derived.h" #include "sql_show.h" +#include "debug_sync.h" extern "C" int _my_b_net_read(IO_CACHE *info, uchar *Buffer, size_t Count); @@ -119,21 +120,43 @@ static bool wsrep_load_data_split(THD *thd, const TABLE *table, if (hton->db_type != DB_TYPE_INNODB) DBUG_RETURN(false); WSREP_DEBUG("intermediate transaction commit in LOAD DATA"); + wsrep_set_load_multi_commit(thd, true); if (wsrep_run_wsrep_commit(thd, true) != WSREP_TRX_OK) DBUG_RETURN(true); if (binlog_hton->commit(binlog_hton, thd, true)) DBUG_RETURN(true); wsrep_post_commit(thd, true); hton->commit(hton, thd, true); + wsrep_set_load_multi_commit(thd, false); + DEBUG_SYNC(thd, "intermediate_transaction_commit"); table->file->extra(HA_EXTRA_FAKE_START_STMT); } DBUG_RETURN(false); } -# define WSREP_LOAD_DATA_SPLIT(thd,table,info) \ - if (wsrep_load_data_split(thd,table,info)) DBUG_RETURN(1) +/* + If the commit fails, then an early return from + the function occurs there and therefore we need + to reset the table->auto_increment_field_not_null + flag, which is usually reset after calling + the write_record(): +*/ +#define WSREP_LOAD_DATA_SPLIT(thd,table,info) \ + if (wsrep_load_data_split(thd,table,info)) \ + { \ + table->auto_increment_field_not_null= FALSE; \ + DBUG_RETURN(1); \ + } #else /* WITH_WSREP */ #define WSREP_LOAD_DATA_SPLIT(thd,table,info) /* empty */ #endif /* WITH_WSREP */ +#define WRITE_RECORD(thd,table,info) \ + do { \ + int err_= write_record(thd, table, &info); \ + table->auto_increment_field_not_null= FALSE; \ + if (err_) \ + DBUG_RETURN(1); \ + } while (0) + class READ_INFO: public Load_data_param { File file; @@ -911,7 +934,7 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, List_iterator_fast it(fields_vars); Item *item; TABLE *table= table_list->table; - bool err, progress_reports; + bool progress_reports; ulonglong counter, time_to_report_progress; DBUG_ENTER("read_fixed_length"); @@ -1003,11 +1026,8 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, } WSREP_LOAD_DATA_SPLIT(thd, table, info); - err= write_record(thd, table, &info); - table->auto_increment_field_not_null= FALSE; - if (err) - DBUG_RETURN(1); - + WRITE_RECORD(thd, table, info); + /* We don't need to reset auto-increment field since we are restoring its default value at the beginning of each loop iteration. @@ -1040,7 +1060,7 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, Item *item; TABLE *table= table_list->table; uint enclosed_length; - bool err, progress_reports; + bool progress_reports; ulonglong counter, time_to_report_progress; DBUG_ENTER("read_sep_field"); @@ -1124,7 +1144,7 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, { Load_data_outvar *dst= item->get_load_data_outvar_or_error(); DBUG_ASSERT(dst); - if (dst->load_data_set_no_data(thd, &read_info)) + if (unlikely(dst->load_data_set_no_data(thd, &read_info))) DBUG_RETURN(1); } } @@ -1146,10 +1166,8 @@ read_sep_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, } WSREP_LOAD_DATA_SPLIT(thd, table, info); - err= write_record(thd, table, &info); - table->auto_increment_field_not_null= FALSE; - if (err) - DBUG_RETURN(1); + WRITE_RECORD(thd, table, info); + /* We don't need to reset auto-increment field since we are restoring its default value at the beginning of each loop iteration. @@ -1267,13 +1285,10 @@ read_xml_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, case VIEW_CHECK_ERROR: DBUG_RETURN(-1); } - + WSREP_LOAD_DATA_SPLIT(thd, table, info); - err= write_record(thd, table, &info); - table->auto_increment_field_not_null= false; - if (err) - DBUG_RETURN(1); - + WRITE_RECORD(thd, table, info); + /* We don't need to reset auto-increment field since we are restoring its default value at the beginning of each loop iteration. diff --git a/sql/sql_plugin_services.ic b/sql/sql_plugin_services.ic index 3d6cf0a0723..e7de45b5ee2 100644 --- a/sql/sql_plugin_services.ic +++ b/sql/sql_plugin_services.ic @@ -178,6 +178,8 @@ static struct wsrep_service_st wsrep_handler = { wsrep_thd_trx_seqno, wsrep_thd_ws_handle, wsrep_thd_auto_increment_variables, + wsrep_set_load_multi_commit, + wsrep_is_load_multi_commit, wsrep_trx_is_aborting, wsrep_trx_order_before, wsrep_unlock_rollback, diff --git a/sql/wsrep_dummy.cc b/sql/wsrep_dummy.cc index 7297dbfe0fd..aff75cf7790 100644 --- a/sql/wsrep_dummy.cc +++ b/sql/wsrep_dummy.cc @@ -133,6 +133,12 @@ void wsrep_thd_auto_increment_variables(THD *thd, *increment= thd->variables.auto_increment_increment; } +void wsrep_set_load_multi_commit(THD *thd, bool split) +{ } + +bool wsrep_is_load_multi_commit(THD *thd) +{ return false; } + int wsrep_trx_is_aborting(THD *) { return 0; } diff --git a/sql/wsrep_hton.cc b/sql/wsrep_hton.cc index e3da6a79f26..3603e05fd5f 100644 --- a/sql/wsrep_hton.cc +++ b/sql/wsrep_hton.cc @@ -45,6 +45,7 @@ void wsrep_cleanup_transaction(THD *thd) thd->wsrep_exec_mode= LOCAL_STATE; thd->wsrep_affected_rows= 0; thd->wsrep_skip_wsrep_GTID= false; + thd->wsrep_split_flag= false; return; } diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc index dab9f91b381..00afbec290e 100644 --- a/sql/wsrep_thd.cc +++ b/sql/wsrep_thd.cc @@ -708,3 +708,13 @@ my_bool wsrep_thd_is_applier(MYSQL_THD thd) return (is_applier); } + +void wsrep_set_load_multi_commit(THD *thd, bool split) +{ + thd->wsrep_split_flag= split; +} + +bool wsrep_is_load_multi_commit(THD *thd) +{ + return thd->wsrep_split_flag; +} diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index fc1d88a0be2..5ea6648c4a4 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -8199,6 +8199,16 @@ ha_innobase::write_row( ++trx->will_lock; } +#ifdef WITH_WSREP + if (wsrep_is_load_multi_commit(m_user_thd)) + { + /* Note that this transaction is still active. */ + trx_register_for_2pc(m_prebuilt->trx); + /* We will need an IX lock on the destination table. */ + m_prebuilt->sql_stat_start = TRUE; + } +#endif /* WITH_WSREP */ + /* Handling of Auto-Increment Columns. */ if (table->next_number_field && record == table->record[0]) { From e3618a333ecaebd32e953aeb7929187af30b0f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Mar 2019 10:23:57 +0200 Subject: [PATCH 3/8] Post-merge fix after 0508d327aef520d3131ff8a85ed610337149fffc --- mysql-test/suite/galera/r/galera_kill_largechanges.result | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mysql-test/suite/galera/r/galera_kill_largechanges.result b/mysql-test/suite/galera/r/galera_kill_largechanges.result index b6393590629..d85d421c164 100644 --- a/mysql-test/suite/galera/r/galera_kill_largechanges.result +++ b/mysql-test/suite/galera/r/galera_kill_largechanges.result @@ -1,4 +1,6 @@ connection node_1; +connection node_2; +connection node_1; SET GLOBAL wsrep_provider_options = 'pc.ignore_sb=true'; CREATE TABLE ten (f1 INTEGER) ENGINE=InnoDB; INSERT INTO ten VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10),(11); @@ -15,5 +17,6 @@ COUNT(*) SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; VARIABLE_VALUE 2 +connection node_1; DROP TABLE t1; DROP TABLE ten; From 1d728a98d8c4626098db9876de101d8cf704c993 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Mar 2019 12:29:54 +0200 Subject: [PATCH 4/8] Fixup MDEV-17262: Remove a unused variable --- sql/sql_load.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/sql_load.cc b/sql/sql_load.cc index 8e0bdcb32b8..9006bf2b1d4 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -1211,7 +1211,6 @@ read_xml_field(THD *thd, COPY_INFO &info, TABLE_LIST *table_list, for ( ; ; it.rewind()) { - bool err; if (thd->killed) { thd->send_kill_message(); From 00572a0b0cc81c38f198adcc5582260a98f3eebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Mar 2019 12:32:10 +0200 Subject: [PATCH 5/8] MDEV-17482 InnoDB fails to say which fatal error fsync() returned os_file_fsync_posix(): If fsync() returns a fatal error, do include errno in the error message. In the future, we might handle fsync() or write or allocation failures on InnoDB data files a little more gracefully: flag the affected index or table as corrupted, and deny any subsequent writes to the table. If a write to the undo log or redo log fails, an alternative to killing the server could be to deny any writes to InnoDB tables until the server has been restarted. --- storage/innobase/os/os0file.cc | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index bb6a06240af..10a8f44b062 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -2495,18 +2495,10 @@ os_file_fsync_posix( ut_a(failures < 2000); break; - case EIO: - ib::error() << "fsync() returned EIO, aborting"; - /* fall through */ default: - ut_error; - break; + ib::fatal() << "fsync() returned " << errno; } } - - ut_error; - - return(-1); } /** Check the existence and type of the given file. From 9471dbafcecef384e60aca12e03aefcfdf6c04f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 19 Mar 2019 13:45:23 +0200 Subject: [PATCH 6/8] MDEV-18960: Assertion !omits_virtual_cols(*form->s) on TRUNCATE MariaDB before MDEV-5800 in version 10.2.2 did not support indexed virtual columns. Non-persistent virtual columns were hidden from storage engines. Only starting with MDEV-5800, InnoDB would create internal metadata on virtual columns. On TRUNCATE TABLE, an old .frm file from before MDEV-5800 may be used as the table schema. When the table is being re-created by InnoDB, the old schema must be used. That is, we may hide the existence of virtual columns from InnoDB. create_table_check_doc_id_col(): Remove the assertion that failed. This function can actually correctly deal with virtual columns that could have been created before MariaDB 10.2.2 introduced MDEV-5800. create_table_info_t::create_table_def(): Do not create metadata for virtual columns if the table definition was created before MariaDB 10.2.2. --- storage/innobase/handler/ha_innodb.cc | 57 +++++++++++---------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 5ea6648c4a4..f6c6be6ffa8 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -10847,8 +10847,6 @@ create_table_check_doc_id_col( ULINT_UNDEFINED if column is of the wrong type/name/size */ { - ut_ad(!omits_virtual_cols(*form->s)); - for (ulint i = 0; i < form->s->fields; i++) { const Field* field; ulint col_type; @@ -11022,7 +11020,6 @@ int create_table_info_t::create_table_def() { dict_table_t* table; - ulint n_cols; ulint col_type; ulint col_len; ulint nulls_allowed; @@ -11030,14 +11027,11 @@ create_table_info_t::create_table_def() ulint binary_type; ulint long_true_varchar; ulint charset_no; - ulint i; ulint j = 0; ulint doc_id_col = 0; ibool has_doc_id_col = FALSE; mem_heap_t* heap; - ulint num_v = 0; ulint space_id = 0; - ulint actual_n_cols; ha_table_option_struct *options= m_form->s->option_struct; dberr_t err = DB_SUCCESS; @@ -11068,14 +11062,15 @@ create_table_info_t::create_table_def() DBUG_RETURN(ER_WRONG_TABLE_NAME); } - n_cols = m_form->s->fields; + /* Find out the number of virtual columns. */ + ulint num_v = 0; + const bool omit_virtual = omits_virtual_cols(*m_form->s); + const ulint n_cols = omit_virtual + ? m_form->s->stored_fields : m_form->s->fields; - /* Find out any virtual column */ - for (i = 0; i < n_cols; i++) { - Field* field = m_form->field[i]; - - if (innobase_is_v_fld(field)) { - num_v++; + if (!omit_virtual) { + for (ulint i = 0; i < n_cols; i++) { + num_v += !m_form->field[i]->stored_in_db(); } } @@ -11086,7 +11081,9 @@ create_table_info_t::create_table_def() if (doc_id_col == ULINT_UNDEFINED) { err = DB_ERROR; - goto error_ret; +error_ret: + DBUG_RETURN(convert_error_code_to_mysql(err, m_flags, + m_thd)); } else { has_doc_id_col = TRUE; } @@ -11096,10 +11093,8 @@ create_table_info_t::create_table_def() determine the actual space id when the tablespace is created. */ /* Adjust the number of columns for the FTS hidden field */ - actual_n_cols = n_cols; - if (m_flags2 & DICT_TF2_FTS && !has_doc_id_col) { - actual_n_cols += 1; - } + const ulint actual_n_cols = n_cols + + (m_flags2 & DICT_TF2_FTS && !has_doc_id_col); table = dict_mem_table_create(m_table_name, space_id, actual_n_cols, num_v, m_flags, m_flags2); @@ -11122,10 +11117,7 @@ create_table_info_t::create_table_def() heap = mem_heap_create(1000); - for (i = 0; i < n_cols; i++) { - ulint is_virtual; - bool is_stored = false; - + for (ulint i = 0; i < n_cols; i++) { Field* field = m_form->field[i]; col_type = get_innobase_type_from_mysql_type( @@ -11191,9 +11183,6 @@ create_table_info_t::create_table_def() } } - is_virtual = (innobase_is_v_fld(field)) ? DATA_VIRTUAL : 0; - is_stored = innobase_is_s_fld(field); - /* First check whether the column to be added has a system reserved name. */ if (dict_col_name_is_reserved(field->field_name)){ @@ -11207,6 +11196,8 @@ err_col: goto error_ret; } + ulint is_virtual = !field->stored_in_db() ? DATA_VIRTUAL : 0; + if (!is_virtual) { dict_mem_table_add_col(table, heap, field->field_name, col_type, @@ -11216,7 +11207,7 @@ err_col: | binary_type | long_true_varchar, charset_no), col_len); - } else { + } else if (!omit_virtual) { dict_mem_table_add_v_col(table, heap, field->field_name, col_type, dtype_form_prtype( @@ -11228,7 +11219,7 @@ err_col: col_len, i, 0); } - if (is_stored) { + if (innobase_is_s_fld(field)) { ut_ad(!is_virtual); /* Added stored column in m_s_cols list. */ dict_mem_table_add_s_col( @@ -11237,12 +11228,12 @@ err_col: } if (num_v) { - for (i = 0; i < n_cols; i++) { + for (ulint i = 0; i < n_cols; i++) { dict_v_col_t* v_col; - Field* field = m_form->field[i]; + const Field* field = m_form->field[i]; - if (!innobase_is_v_fld(field)) { + if (field->stored_in_db()) { continue; } @@ -11256,8 +11247,7 @@ err_col: /** Fill base columns for the stored column present in the list. */ if (table->s_cols && table->s_cols->size()) { - - for (i = 0; i < n_cols; i++) { + for (ulint i = 0; i < n_cols; i++) { Field* field = m_form->field[i]; if (!innobase_is_s_fld(field)) { @@ -11334,8 +11324,7 @@ err_col: : ER_TABLESPACE_EXISTS, MYF(0), display_name); } -error_ret: - DBUG_RETURN(convert_error_code_to_mysql(err, m_flags, m_thd)); + goto error_ret; } /*****************************************************************//** From 1efda582ade2e0bb073cc267d0abe11922371d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 19 Mar 2019 14:30:11 +0200 Subject: [PATCH 7/8] Replace innobase_is_v_fld() with Field::stored_in_db() The macro innobase_is_v_fld() turns out to be equivalent with the opposite of Field::stored_in_db(). Remove the macro and invoke the member function directly. innodb_base_col_setup_for_stored(): Simplify a condition to only check Field::vcol_info. innobase_create_index_def(): Replace some redundant code with DBUG_ASSERT(). --- storage/innobase/handler/ha_innodb.cc | 53 +++++----- storage/innobase/handler/ha_innodb.h | 4 +- storage/innobase/handler/handler0alter.cc | 112 +++++++--------------- 3 files changed, 61 insertions(+), 108 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index f6c6be6ffa8..3189f154f15 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -5986,7 +5986,7 @@ innobase_build_v_templ( Field* field = table->field[i]; /* Build template for virtual columns */ - if (innobase_is_v_fld(field)) { + if (!field->stored_in_db()) { #ifdef UNIV_DEBUG const char* name; @@ -7475,7 +7475,7 @@ build_template_needs_field( { const Field* field = table->field[i]; - if (innobase_is_v_fld(field) && omits_virtual_cols(*table->s)) { + if (!field->stored_in_db() && omits_virtual_cols(*table->s)) { return NULL; } @@ -7555,7 +7555,7 @@ build_template_field( templ = prebuilt->mysql_template + prebuilt->n_template++; UNIV_MEM_INVALID(templ, sizeof *templ); - templ->is_virtual = innobase_is_v_fld(field); + templ->is_virtual = !field->stored_in_db(); if (!templ->is_virtual) { templ->col_no = i; @@ -7799,7 +7799,7 @@ ha_innobase::build_template( /* Push down an index condition or an end_range check. */ for (ulint i = 0; i < n_fields; i++) { const Field* field = table->field[i]; - const bool is_v = innobase_is_v_fld(field); + const bool is_v = !field->stored_in_db(); if (is_v && skip_virtual) { num_v++; continue; @@ -7939,7 +7939,7 @@ ha_innobase::build_template( for (ulint i = 0; i < n_fields; i++) { mysql_row_templ_t* templ; const Field* field = table->field[i]; - const bool is_v = innobase_is_v_fld(field); + const bool is_v = !field->stored_in_db(); if (is_v && skip_virtual) { num_v++; continue; @@ -7989,7 +7989,7 @@ no_icp: for (ulint i = 0; i < n_fields; i++) { const Field* field = table->field[i]; - const bool is_v = innobase_is_v_fld(field); + const bool is_v = !field->stored_in_db(); if (whole_row) { if (is_v && skip_virtual) { @@ -8533,7 +8533,7 @@ calc_row_difference( for (uint i = 0; i < table->s->fields; i++) { field = table->field[i]; - const bool is_virtual = innobase_is_v_fld(field); + const bool is_virtual = !field->stored_in_db(); if (is_virtual && skip_virtual) { continue; } @@ -8907,7 +8907,7 @@ wsrep_calc_row_hash( byte true_byte=1; const Field* field = table->field[i]; - if (innobase_is_v_fld(field)) { + if (!field->stored_in_db()) { continue; } @@ -10986,10 +10986,8 @@ innodb_base_col_setup_for_stored( for (uint i= 0; i < field->table->s->fields; ++i) { const Field* base_field = field->table->field[i]; - if (!innobase_is_s_fld(base_field) - && !innobase_is_v_fld(base_field) - && bitmap_is_set(&field->table->tmp_set, - i)) { + if (!base_field->vcol_info + && bitmap_is_set(&field->table->tmp_set, i)) { ulint z; for (z = 0; z < table->n_cols; z++) { const char* name = dict_table_get_col_name( @@ -11367,17 +11365,16 @@ create_index( key->user_defined_key_parts); for (ulint i = 0; i < key->user_defined_key_parts; i++) { - KEY_PART_INFO* key_part = key->key_part + i; + const Field* field = key->key_part[i].field; /* We do not support special (Fulltext or Spatial) index on virtual columns */ - if (innobase_is_v_fld(key_part->field)) { + if (!field->stored_in_db()) { ut_ad(0); DBUG_RETURN(HA_ERR_UNSUPPORTED); } - dict_mem_index_add_field( - index, key_part->field->field_name, 0); + dict_mem_index_add_field(index, field->field_name, 0); } DBUG_RETURN(convert_error_code_to_mysql( @@ -11465,7 +11462,7 @@ create_index( field_lengths[i] = key_part->length; - if (innobase_is_v_fld(key_part->field)) { + if (!key_part->field->stored_in_db()) { index->type |= DICT_VIRTUAL; } @@ -12502,19 +12499,17 @@ create_table_info_t::gcols_in_fulltext_or_spatial() { for (ulint i = 0; i < m_form->s->keys; i++) { const KEY* key = m_form->key_info + i; - if (key->flags & (HA_SPATIAL | HA_FULLTEXT)) { - for (ulint j = 0; j < key->user_defined_key_parts; j++) { - const KEY_PART_INFO* key_part = key->key_part + j; - - /* We do not support special (Fulltext or - Spatial) index on virtual columns */ - if (innobase_is_v_fld(key_part->field)) { - my_error(ER_UNSUPPORTED_ACTION_ON_GENERATED_COLUMN, MYF(0)); - return true; - } + if (!(key->flags & (HA_SPATIAL | HA_FULLTEXT))) { + continue; + } + for (ulint j = 0; j < key->user_defined_key_parts; j++) { + /* We do not support special (Fulltext or + Spatial) index on virtual columns */ + if (!key->key_part[j].field->stored_in_db()) { + my_error(ER_UNSUPPORTED_ACTION_ON_GENERATED_COLUMN, MYF(0)); + return true; } } - } return false; } @@ -12820,7 +12815,7 @@ create_table_info_t::create_table_update_dict() } if (const Field* ai = m_form->found_next_number_field) { - ut_ad(!innobase_is_v_fld(ai)); + ut_ad(ai->stored_in_db()); ib_uint64_t autoinc = m_create_info->auto_increment_value; diff --git a/storage/innobase/handler/ha_innodb.h b/storage/innobase/handler/ha_innodb.h index 88298411379..76a9b08c4c6 100644 --- a/storage/innobase/handler/ha_innodb.h +++ b/storage/innobase/handler/ha_innodb.h @@ -846,10 +846,8 @@ innodb_base_col_setup_for_stored( const Field* field, dict_s_col_t* s_col); -/** whether this is a stored column */ +/** whether this is a stored generated column */ #define innobase_is_s_fld(field) ((field)->vcol_info && (field)->stored_in_db()) -/** whether this is a computed virtual column */ -#define innobase_is_v_fld(field) ((field)->vcol_info && !(field)->stored_in_db()) /** Always normalize table name to lower case on Windows */ #ifdef _WIN32 diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 8bec4e20658..095fad367b3 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -485,7 +485,7 @@ check_v_col_in_order( cf_it.rewind(); while (const Create_field* new_field = cf_it++) { - if (!innobase_is_v_fld(new_field)) { + if (new_field->stored_in_db()) { continue; } @@ -517,8 +517,6 @@ check_v_col_in_order( continue; } - ut_ad(innobase_is_v_fld(field)); - if (field->flags & FIELD_IS_DROPPED) { continue; } @@ -859,7 +857,7 @@ ha_innobase::check_if_supported_inplace_alter( online = false; } - if (innobase_is_v_fld(key_part->field)) { + if (!key_part->field->stored_in_db()) { /* Do not support adding index on newly added virtual column, while there is also a drop virtual column in the same clause */ @@ -1288,7 +1286,7 @@ no_match: /* Any index on virtual columns cannot be used for reference constaint */ - if (innobase_is_v_fld(key_part.field)) { + if (!key_part.field->stored_in_db()) { goto no_match; } @@ -1864,21 +1862,15 @@ innobase_fields_to_mysql( for (uint i = 0; i < n_fields; i++) { Field* field = table->field[i]; ulint ipos; - ulint col_n; ulint prefix_col; field->reset(); - if (innobase_is_v_fld(field)) { - col_n = num_v; - num_v++; - } else { - col_n = i - num_v; - } + const bool is_v = !field->stored_in_db(); + const ulint col_n = is_v ? num_v++ : i - num_v; ipos = dict_index_get_nth_col_or_prefix_pos( - index, col_n, true, innobase_is_v_fld(field), - &prefix_col); + index, col_n, true, is_v, &prefix_col); if (ipos == ULINT_UNDEFINED || dfield_is_ext(&fields[ipos]) @@ -1924,7 +1916,7 @@ innobase_row_to_mysql( field->reset(); - if (innobase_is_v_fld(field)) { + if (!field->stored_in_db()) { /* Virtual column are not stored in InnoDB table, so skip it */ num_v++; @@ -2150,7 +2142,7 @@ innobase_create_index_field_def( : key_part->field; for (ulint i = 0; i < key_part->fieldnr; i++) { - if (innobase_is_v_fld(altered_table->field[i])) { + if (!altered_table->field[i]->stored_in_db()) { num_v++; } } @@ -2158,11 +2150,9 @@ innobase_create_index_field_def( col_type = get_innobase_type_from_mysql_type( &is_unsigned, field); - if (innobase_is_v_fld(field)) { - index_field->is_v_col = true; + if ((index_field->is_v_col = !field->stored_in_db())) { index_field->col_no = num_v; } else { - index_field->is_v_col = false; index_field->col_no = key_part->fieldnr - num_v; } @@ -2264,24 +2254,15 @@ innobase_create_index_def( /* Need to count the virtual fields before this spatial indexed field */ for (ulint i = 0; i < key->key_part->fieldnr; i++) { - if (innobase_is_v_fld(altered_table->field[i])) { - num_v++; - } + num_v += !altered_table->field[i]->stored_in_db(); } index->fields[0].col_no = key->key_part[0].fieldnr - num_v; index->fields[0].prefix_len = 0; index->fields[0].is_v_col = false; - if (innobase_is_v_fld(key->key_part[0].field)) { - - /* Currently, the spatial index cannot be created - on virtual columns. It is blocked in server - layer */ - ut_ad(0); - index->fields[0].is_v_col = true; - } else { - index->fields[0].is_v_col = false; - } + /* Currently, the spatial index cannot be created + on virtual columns. It is blocked in the SQL layer. */ + DBUG_ASSERT(key->key_part[0].field->stored_in_db()); } else { index->ind_type = (key->flags & HA_NOSAME) ? DICT_UNIQUE : 0; } @@ -2329,7 +2310,7 @@ innobase_fts_check_doc_id_col( for (i = 0; i < n_cols; i++) { const Field* field = altered_table->field[i]; - if (innobase_is_v_fld(field)) { + if (!field->stored_in_db()) { (*num_v)++; } @@ -2345,7 +2326,7 @@ innobase_fts_check_doc_id_col( || field->pack_length() != 8 || field->real_maybe_null() || !(field->flags & UNSIGNED_FLAG) - || innobase_is_v_fld(field)) { + || !field->stored_in_db()) { my_error(ER_INNODB_FT_WRONG_DOCID_COLUMN, MYF(0), field->field_name); } else { @@ -3082,17 +3063,12 @@ innobase_build_col_map( } while (const Create_field* new_field = cf_it++) { - bool is_v = false; - - if (innobase_is_v_fld(new_field)) { - is_v = true; - } - + bool is_v = !new_field->stored_in_db(); ulint num_old_v = 0; for (uint old_i = 0; table->field[old_i]; old_i++) { const Field* field = table->field[old_i]; - if (innobase_is_v_fld(field)) { + if (!field->stored_in_db()) { if (is_v && new_field->field == field) { col_map[old_table->n_cols + num_v] = num_old_v; @@ -3236,14 +3212,12 @@ innobase_get_col_names( ulint num_v = 0; DBUG_ASSERT(i < altered_table->s->fields); - if (innobase_is_v_fld(new_field)) { + if (!new_field->stored_in_db()) { continue; } for (uint old_i = 0; table->field[old_i]; old_i++) { - if (innobase_is_v_fld(table->field[old_i])) { - num_v++; - } + num_v += !table->field[old_i]->stored_in_db(); if (new_field->field == table->field[old_i]) { cols[old_i - num_v] = new_field->field_name; @@ -3528,7 +3502,7 @@ innobase_check_gis_columns( const KEY_PART_INFO& key_part = key.key_part[0]; /* Does not support spatial index on virtual columns */ - if (innobase_is_v_fld(key_part.field)) { + if (!key_part.field->stored_in_db()) { DBUG_RETURN(DB_UNSUPPORTED); } @@ -3629,7 +3603,7 @@ prepare_inplace_add_virtual( &is_unsigned, field); - if (!innobase_is_v_fld(field)) { + if (field->stored_in_db()) { continue; } @@ -4268,7 +4242,7 @@ innodb_v_adjust_idx_col( /* Found the field in the new table */ while (const Create_field* new_field = cf_it++) { - if (!innobase_is_v_fld(new_field)) { + if (new_field->stored_in_db()) { continue; } @@ -4288,7 +4262,7 @@ innodb_v_adjust_idx_col( ut_a(0); } - ut_ad(innobase_is_v_fld(field)); + ut_ad(!field->stored_in_db()); num_v = 0; @@ -4302,9 +4276,7 @@ innodb_v_adjust_idx_col( break; } - if (innobase_is_v_fld(old_table->field[old_i])) { - num_v++; - } + num_v += !old_table->field[old_i]->stored_in_db(); } ut_ad(col_found); @@ -4577,10 +4549,10 @@ prepare_inplace_alter_table_dict( for (uint i = 0; i < altered_table->s->fields; i++) { const Field* field = altered_table->field[i]; - if (innobase_is_v_fld(field)) { + if (!field->stored_in_db()) { n_v_cols++; } else { - n_cols++; + n_cols++; } } @@ -4632,7 +4604,7 @@ prepare_inplace_alter_table_dict( &is_unsigned, field); ulint charset_no; ulint col_len; - bool is_virtual = innobase_is_v_fld(field); + const bool is_virtual = !field->stored_in_db(); /* we assume in dtype_form_prtype() that this fits in two bytes */ @@ -4717,7 +4689,7 @@ prepare_inplace_alter_table_dict( dict_v_col_t* v_col; const Field* field = altered_table->field[i]; - if (!innobase_is_v_fld(field)) { + if (!!field->stored_in_db()) { continue; } v_col = dict_table_get_nth_v_col( @@ -5471,7 +5443,7 @@ alter_fill_stored_column( Field* field = altered_table->field[i]; dict_s_col_t s_col; - if (!innobase_is_v_fld(field)) { + if (field->stored_in_db()) { stored_col_no++; } @@ -5721,7 +5693,7 @@ check_if_ok_to_rename: MySQL should have checked these already. We want to allow renaming of c1 to c2, c2 to c1. */ for (j = 0; j < table->s->fields; j++) { - if (!innobase_is_v_fld(table->field[j])) { + if (table->field[j]->stored_in_db()) { s += strlen(s) + 1; } } @@ -6267,10 +6239,7 @@ err_exit: autoinc_col_max_value = innobase_get_int_col_max_value(field); } found_col: - if (innobase_is_v_fld(new_field)) { - ++num_v; - } - + num_v += !new_field->stored_in_db(); i++; } @@ -7172,13 +7141,13 @@ innobase_rename_columns_try( & Alter_inplace_info::ALTER_COLUMN_NAME); for (Field** fp = table->field; *fp; fp++, i++) { - bool is_virtual = innobase_is_v_fld(*fp); - + const bool is_virtual = !(*fp)->stored_in_db(); if (!((*fp)->flags & FIELD_IS_RENAMED)) { goto processed_field; } cf_it.rewind(); + while (Create_field* cf = cf_it++) { if (cf->field == *fp) { ulint col_n = is_virtual @@ -7331,19 +7300,10 @@ innobase_enlarge_columns_try( ha_alter_info->alter_info->create_list); ulint i = 0; ulint num_v = 0; - bool is_v; for (Field** fp = table->field; *fp; fp++, i++) { - ulint idx; - - if (innobase_is_v_fld(*fp)) { - is_v = true; - idx = num_v; - num_v++; - } else { - idx = i - num_v; - is_v = false; - } + const bool is_v = !(*fp)->stored_in_db(); + ulint idx = is_v ? num_v++ : i - num_v; cf_it.rewind(); while (Create_field* cf = cf_it++) { @@ -7389,7 +7349,7 @@ innobase_rename_or_enlarge_columns_cache( ulint num_v = 0; for (Field** fp = table->field; *fp; fp++, i++) { - bool is_virtual = innobase_is_v_fld(*fp); + const bool is_virtual = !(*fp)->stored_in_db(); cf_it.rewind(); while (Create_field* cf = cf_it++) { From a77e2668667e46d96c445a4a9a8325b7fe9461b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 19 Mar 2019 15:30:42 +0200 Subject: [PATCH 8/8] MDEV-18084: Crash on UPDATE after upgrade from 10.0 or 10.1 MariaDB Server 10.0 and 10.1 support non-indexed virtual columns, which are hidden from the storage engine. Starting with MDEV-5800 in MariaDB 10.2.2, the virtual columns are visible to storage engines. calc_row_difference(): Follow up the MDEV-17199 fix, which forgot to increment num_v when skipping virtual columns in tables that were created before MariaDB 10.2.2. This caused a corruption of the update vector when an updated persistent column is preceded by virtual columns. --- storage/innobase/handler/ha_innodb.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 3189f154f15..8d2af1b0e6d 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -8535,6 +8535,7 @@ calc_row_difference( field = table->field[i]; const bool is_virtual = !field->stored_in_db(); if (is_virtual && skip_virtual) { + num_v++; continue; } dict_col_t* col = is_virtual