diff --git a/mysql-test/main/alter_table_online_debug.result b/mysql-test/main/alter_table_online_debug.result index 08a094db53a..d59167f5b6e 100644 --- a/mysql-test/main/alter_table_online_debug.result +++ b/mysql-test/main/alter_table_online_debug.result @@ -979,5 +979,142 @@ xa commit 'xid' one phase; drop table t; set debug_sync= reset; # +# MDEV-29069 ER_KEY_NOT_FOUND upon online autoinc addition and +# concurrent DELETE +# +set @old_dbug=@@debug_dbug; +set debug_dbug="+d,rpl_report_chosen_key"; +create table t (a int); +insert into t values (10),(20),(30); +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +alter table t add pk int auto_increment primary key, algorithm=copy, lock=none; +connection con2; +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +connection default; +Warnings: +Note 1105 Key chosen: -1 +Note 1105 Key chosen: -1 +select * from t; +a pk +11 1 +30 3 +# +# Add clumsy DEFAULT +# +create or replace table t (a int); +insert into t values (10),(20),(30); +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +alter table t add b int default(RAND() * 20), add key(b), +algorithm=copy, lock=none; +connection con2; +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +connection default; +Warnings: +Note 1105 Key chosen: -1 +Note 1105 Key chosen: -1 +select a from t; +a +11 +30 +# CURRENT_TIMESTAMP +create or replace table t (a int); +insert into t values (10),(20),(30); +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +alter table t add b timestamp default CURRENT_TIMESTAMP, add key(b), +algorithm=copy, lock=none; +connection con2; +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +connection default; +Warnings: +Note 1105 Key chosen: -1 +Note 1105 Key chosen: -1 +select a from t; +a +11 +30 +# CURRENT_TIMESTAMP, mixed key +create or replace table t (a int); +insert into t values (10),(20),(30); +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +alter table t add b timestamp default CURRENT_TIMESTAMP, add key(a, b), +algorithm=copy, lock=none; +connection con2; +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +connection default; +Warnings: +Note 1105 Key chosen: -1 +Note 1105 Key chosen: -1 +select a from t; +a +11 +30 +# Mixed primary key +create or replace table t (a int); +insert into t values (10),(20),(30); +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +alter table t add b int default (a+1), add primary key(b, a), +algorithm=copy, lock=none; +connection con2; +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +connection default; +Warnings: +Note 1105 Key chosen: -1 +Note 1105 Key chosen: -1 +select a from t; +a +11 +30 +# +# Normal row, could be used as a key +# +create or replace table t (a int); +insert into t values (10),(20),(30); +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +alter table t add b int as (a * 10) unique, algorithm=copy, lock=none; +connection con2; +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +connection default; +Warnings: +Note 1105 Key chosen: -1 +Note 1105 Key chosen: -1 +# +# Add key for old row +# +create or replace table t (a int); +insert into t values (10),(20),(30); +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +alter table t add unique(a), algorithm=copy, lock=none; +connection con2; +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +connection default; +Warnings: +Note 1105 Key chosen: 0 +Note 1105 Key chosen: 0 +drop table t; +set debug_sync= reset; +set debug_dbug= @old_debug; +connection default; +# # End of 11.2 tests # diff --git a/mysql-test/main/alter_table_online_debug.test b/mysql-test/main/alter_table_online_debug.test index 9d88c367a1d..4431c266ed4 100644 --- a/mysql-test/main/alter_table_online_debug.test +++ b/mysql-test/main/alter_table_online_debug.test @@ -1152,6 +1152,144 @@ xa commit 'xid' one phase; drop table t; set debug_sync= reset; +--echo # +--echo # MDEV-29069 ER_KEY_NOT_FOUND upon online autoinc addition and +--echo # concurrent DELETE +--echo # +set @old_dbug=@@debug_dbug; +set debug_dbug="+d,rpl_report_chosen_key"; + +create table t (a int); +insert into t values (10),(20),(30); + +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +--send +alter table t add pk int auto_increment primary key, algorithm=copy, lock=none; +--connection con2 +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; + +--connection default +--reap +select * from t; + +--echo # +--echo # Add clumsy DEFAULT +--echo # +create or replace table t (a int); +insert into t values (10),(20),(30); + +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +--send +alter table t add b int default(RAND() * 20), add key(b), + algorithm=copy, lock=none; +--connection con2 +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +--connection default +--reap +select a from t; + +--echo # CURRENT_TIMESTAMP +create or replace table t (a int); +insert into t values (10),(20),(30); + +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +--send +alter table t add b timestamp default CURRENT_TIMESTAMP, add key(b), + algorithm=copy, lock=none; +--connection con2 +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; +--connection default +--reap +select a from t; + +--echo # CURRENT_TIMESTAMP, mixed key +create or replace table t (a int); +insert into t values (10),(20),(30); + +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +--send +alter table t add b timestamp default CURRENT_TIMESTAMP, add key(a, b), + algorithm=copy, lock=none; +--connection con2 +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; + +--connection default +--reap +select a from t; + +--echo # Mixed primary key +create or replace table t (a int); +insert into t values (10),(20),(30); + +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +--send +alter table t add b int default (a+1), add primary key(b, a), + algorithm=copy, lock=none; +--connection con2 +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; + +--connection default +--reap +select a from t; + +--echo # +--echo # Normal row, could be used as a key +--echo # +create or replace table t (a int); +insert into t values (10),(20),(30); + +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +--send +alter table t add b int as (a * 10) unique, algorithm=copy, lock=none; +--connection con2 +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; + +--connection default +--reap + +--echo # +--echo # Add key for old row +--echo # +create or replace table t (a int); +insert into t values (10),(20),(30); + +set debug_sync= 'alter_table_online_downgraded signal downgraded wait_for goforit'; +--send +alter table t add unique(a), algorithm=copy, lock=none; +--connection con2 +set debug_sync= 'now wait_for downgraded'; +delete from t where a = 20; +update t set a = a + 1 where a = 10; +set debug_sync= 'now signal goforit'; + +--connection default +--reap + + +# Cleanup +drop table t; +set debug_sync= reset; +set debug_dbug= @old_debug; +--connection default + --echo # --echo # End of 11.2 tests --echo # diff --git a/sql/log_event.h b/sql/log_event.h index f622bd8bb61..61860a4e5ff 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -4798,7 +4798,9 @@ protected: friend class Rows_log_event; }; - int find_key(); // Find a best key to use in find_row() + int find_key(const rpl_group_info *); // Find a best key to use in find_row() + bool is_key_usable(const KEY *key) const; + bool use_pk_position() const; int find_row(rpl_group_info *); int write_row(rpl_group_info *, const bool); int update_sequence(); @@ -4864,8 +4866,8 @@ private: The member function will return 0 if all went OK, or a non-zero error code otherwise. */ - virtual - int do_before_row_operations(const Slave_reporting_capability *const log) = 0; + virtual + int do_before_row_operations(const rpl_group_info *) = 0; /* Primitive to clean up after a sequence of row executions. @@ -4954,7 +4956,7 @@ private: #endif #if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION) - virtual int do_before_row_operations(const Slave_reporting_capability *const); + virtual int do_before_row_operations(const rpl_group_info *); virtual int do_after_row_operations(const Slave_reporting_capability *const,int); virtual int do_exec_row(rpl_group_info *); #endif @@ -5044,7 +5046,7 @@ protected: #endif #if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION) - virtual int do_before_row_operations(const Slave_reporting_capability *const); + virtual int do_before_row_operations(const rpl_group_info *); virtual int do_after_row_operations(const Slave_reporting_capability *const,int); virtual int do_exec_row(rpl_group_info *); #endif /* defined(MYSQL_SERVER) && defined(HAVE_REPLICATION) */ @@ -5131,7 +5133,7 @@ protected: #endif #if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION) - virtual int do_before_row_operations(const Slave_reporting_capability *const); + virtual int do_before_row_operations(const rpl_group_info *const); virtual int do_after_row_operations(const Slave_reporting_capability *const,int); virtual int do_exec_row(rpl_group_info *); #endif diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 89be69eca96..26d00deb59f 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -5100,7 +5100,7 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi) this->slave_exec_mode= slave_exec_mode_options; // fix the mode // Do event specific preparations - error= do_before_row_operations(rli); + error= do_before_row_operations(rgi); /* Bug#56662 Assertion failed: next_insert_id == 0, file handler.cc @@ -6453,7 +6453,7 @@ bool Write_rows_compressed_log_event::write() #if defined(HAVE_REPLICATION) int -Write_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const) +Write_rows_log_event::do_before_row_operations(const rpl_group_info *) { int error= 0; @@ -7021,16 +7021,19 @@ uint8 Write_rows_log_event::get_trg_event_map() **************************************************************************/ #if defined(HAVE_REPLICATION) -/* - Compares table->record[0] and table->record[1] +/** + @brief Compares table->record[0] and table->record[1] - Returns TRUE if different. + @returns true if different. */ static bool record_compare(TABLE *table, bool vers_from_plain= false) { - bool result= FALSE; + bool result= false; + bool all_values_set= bitmap_is_set_all(&table->has_value_set); + /** Compare full record only if: + - all fields were given values - there are no blob fields (otherwise we would also need to compare blobs contents as well); - there are no varchar fields (otherwise we would also need @@ -7041,24 +7044,23 @@ static bool record_compare(TABLE *table, bool vers_from_plain= false) */ if ((table->s->blob_fields + table->s->varchar_fields + - table->s->null_fields) == 0) + table->s->null_fields) == 0 + && all_values_set) { - result= cmp_record(table,record[1]); + result= cmp_record(table, record[1]); goto record_compare_exit; } /* Compare null bits */ - if (memcmp(table->null_flags, - table->null_flags+table->s->rec_buff_length, - table->s->null_bytes)) - { - result= TRUE; // Diff in NULL value - goto record_compare_exit; - } + if (all_values_set && memcmp(table->null_flags, + table->null_flags + table->s->rec_buff_length, + table->s->null_bytes)) + goto record_compare_differ; // Diff in NULL value /* Compare fields */ for (Field **ptr=table->field ; *ptr ; ptr++) { + Field *f= *ptr; /* If the table is versioned, don't compare using the version if there is a primary key. If there isn't a primary key, we need the version to @@ -7068,27 +7070,67 @@ static bool record_compare(TABLE *table, bool vers_from_plain= false) because the implicit row_end value will be set to the maximum value for the latest row update (which is what we care about). */ - if (table->versioned() && (*ptr)->vers_sys_field() && + if (table->versioned() && f->vers_sys_field() && (table->s->primary_key < MAX_KEY || - (vers_from_plain && table->vers_start_field() == (*ptr)))) + (vers_from_plain && table->vers_start_field() == f))) continue; - /** - We only compare field contents that are not null. - NULL fields (i.e., their null bits) were compared - earlier. + + /* + We only compare fields that exist on the master (or in ONLINE + ALTER case, that were in the original table). */ - if (!(*(ptr))->is_null()) + if (!all_values_set) { - if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length)) - { - result= TRUE; - goto record_compare_exit; - } + if (!f->has_explicit_value()) + continue; + if (f->is_null() != f->is_null(table->s->rec_buff_length)) + goto record_compare_differ; } + + if (!f->is_null() && f->cmp_binary_offset(table->s->rec_buff_length)) + goto record_compare_differ; } record_compare_exit: return result; +record_compare_differ: + return true; +} + + +/** + Newly added fields with non-deterministic defaults (i.e. DEFAULT(RANDOM()), + CURRENT_TIMESTAMP, AUTO_INCREMENT) should be excluded from key search. + Basically we exclude all the default-filled fields based on + has_explicit_value bitmap. +*/ +bool Rows_log_event::is_key_usable(const KEY *key) const +{ + RPL_TABLE_LIST *tl= (RPL_TABLE_LIST*)m_table->pos_in_table_list; + + if (!m_table->s->keys_in_use.is_set(uint(key - m_table->key_info))) + return false; + + if (!tl->m_online_alter_copy_fields) + { + if (m_cols.n_bits >= m_table->s->fields) + return true; + if (m_table->s->virtual_fields + m_table->s->stored_fields == 0) + { + for (uint p= 0; p < key->user_defined_key_parts; p++) + if (key->key_part[p].fieldnr > m_cols.n_bits) + return false; + return true; + } + } + + for (uint p= 0; p < key->user_defined_key_parts; p++) + { + uint field_idx= key->key_part[p].fieldnr - 1; + if (!bitmap_is_set(&m_table->has_value_set, field_idx)) + return false; + } + return true; } @@ -7103,7 +7145,7 @@ record_compare_exit: @returns Error code on failure, 0 on success. */ -int Rows_log_event::find_key() +int Rows_log_event::find_key(const rpl_group_info *rgi) { DBUG_ASSERT(m_table); RPL_TABLE_LIST *tl= (RPL_TABLE_LIST*)m_table->pos_in_table_list; @@ -7118,6 +7160,35 @@ int Rows_log_event::find_key() { best_key_nr= MAX_KEY; + /* + if the source (in the row event) and destination (in m_table) records + don't have the same structure, some keys below might be unusable + for find_row(). + + If it's a replication and slave table (m_table) has less columns + than the master's - easy, all keys are usable. + + If slave's table has more columns, but none of them are generated - + then any column beyond m_cols.n_bits makes an index unusable. + + If slave's table has generated columns or it's the online alter table + where arbitrary structure conversion is possible (in the replication case + one table must be a prefix of the other, see table_def::compatible_with) + we cannot deduce what destination columns will be affected by m_cols, + we have to actually unpack one row and examine has_explicit_value() + */ + + if (tl->m_online_alter_copy_fields || + (m_cols.n_bits < m_table->s->fields && + m_table->s->virtual_fields + m_table->s->stored_fields)) + { + const uchar *curr_row_end= m_curr_row_end; + Check_level_instant_set clis(m_table->in_use, CHECK_FIELD_IGNORE); + if (int err= unpack_row(rgi, m_table, m_width, m_curr_row, &m_cols, + &curr_row_end, &m_master_reclength, m_rows_end)) + DBUG_RETURN(err); + } + /* Keys are sorted so that any primary key is first, followed by unique keys, followed by any other. So we will automatically pick the primary key if @@ -7125,7 +7196,7 @@ int Rows_log_event::find_key() */ for (i= 0, key= m_table->key_info; i < m_table->s->keys; i++, key++) { - if (!m_table->s->keys_in_use.is_set(i)) + if (!is_key_usable(key)) continue; /* We cannot use a unique key with NULL-able columns to uniquely identify @@ -7163,12 +7234,23 @@ int Rows_log_event::find_key() else { m_key_info= m_table->key_info + best_key_nr; - // Allocate buffer for key searches - m_key= (uchar *) my_malloc(PSI_INSTRUMENT_ME, m_key_info->key_length, MYF(MY_WME)); - if (m_key == NULL) - DBUG_RETURN(HA_ERR_OUT_OF_MEM); + + if (!use_pk_position()) + { + // Allocate buffer for key searches + m_key= (uchar *) my_malloc(PSI_INSTRUMENT_ME, m_key_info->key_length, MYF(MY_WME)); + if (m_key == NULL) + DBUG_RETURN(HA_ERR_OUT_OF_MEM); + } } + DBUG_EXECUTE_IF("rpl_report_chosen_key", + push_warning_printf(m_table->in_use, + Sql_condition::WARN_LEVEL_NOTE, + ER_UNKNOWN_ERROR, "Key chosen: %d", + m_key_nr == MAX_KEY ? + -1 : m_key_nr);); + DBUG_RETURN(0); } @@ -7230,6 +7312,13 @@ static int row_not_found_error(rpl_group_info *rgi) ? HA_ERR_KEY_NOT_FOUND : HA_ERR_RECORD_CHANGED; } +bool Rows_log_event::use_pk_position() const +{ + return m_table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION + && m_table->s->primary_key < MAX_KEY + && m_key_nr == m_table->s->primary_key; +} + /** Locate the current row in event's table. @@ -7302,8 +7391,7 @@ int Rows_log_event::find_row(rpl_group_info *rgi) DBUG_PRINT("info",("looking for the following record")); DBUG_DUMP("record[0]", table->record[0], table->s->reclength); - if ((table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION) && - table->s->primary_key < MAX_KEY) + if (use_pk_position()) { /* Use a more efficient method to fetch the record given by @@ -7562,7 +7650,7 @@ bool Delete_rows_compressed_log_event::write() #if defined(HAVE_REPLICATION) int -Delete_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const) +Delete_rows_log_event::do_before_row_operations(const rpl_group_info *rgi) { /* Increment the global status delete count variable @@ -7570,18 +7658,10 @@ Delete_rows_log_event::do_before_row_operations(const Slave_reporting_capability if (get_flags(STMT_END_F)) status_var_increment(thd->status_var.com_stat[SQLCOM_DELETE]); - if ((m_table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION) && - m_table->s->primary_key < MAX_KEY) - { - /* - We don't need to allocate any memory for m_key since it is not used. - */ - return 0; - } if (do_invoke_trigger()) m_table->prepare_triggers_for_delete_stmt_or_event(); - return find_key(); + return find_key(rgi); } int @@ -7726,7 +7806,7 @@ void Update_rows_log_event::init(MY_BITMAP const *cols) #if defined(HAVE_REPLICATION) int -Update_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const) +Update_rows_log_event::do_before_row_operations(const rpl_group_info *rgi) { /* Increment the global status update count variable @@ -7735,7 +7815,7 @@ Update_rows_log_event::do_before_row_operations(const Slave_reporting_capability status_var_increment(thd->status_var.com_stat[SQLCOM_UPDATE]); int err; - if ((err= find_key())) + if ((err= find_key(rgi))) return err; if (do_invoke_trigger()) diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc index 6440fb0eecb..ceb81dfa80d 100644 --- a/sql/rpl_record.cc +++ b/sql/rpl_record.cc @@ -188,7 +188,7 @@ pack_row(TABLE *table, MY_BITMAP const* cols, @retval HA_ERR_CORRUPT_EVENT Found error when trying to unpack fields. */ -int unpack_row(rpl_group_info *rgi, TABLE *table, uint const colcnt, +int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt, uchar const *const row_data, MY_BITMAP const *cols, uchar const **const current_row_end, ulong *const master_reclength, uchar const *const row_end) diff --git a/sql/rpl_record.h b/sql/rpl_record.h index e93cfb63193..0f486c51bda 100644 --- a/sql/rpl_record.h +++ b/sql/rpl_record.h @@ -29,7 +29,7 @@ size_t pack_row(TABLE* table, MY_BITMAP const* cols, #endif #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) -int unpack_row(rpl_group_info *rgi, +int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt, uchar const *const row_data, MY_BITMAP const *cols, uchar const **const curr_row_end, ulong *const master_reclength, diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc index bd55c14e447..da018d0e6fb 100644 --- a/sql/rpl_rli.cc +++ b/sql/rpl_rli.cc @@ -2473,7 +2473,7 @@ rpl_group_info::mark_start_commit() If no GTID is available, then NULL is returned. */ char * -rpl_group_info::gtid_info() +rpl_group_info::gtid_info() const { if (!gtid_sub_id || !current_gtid.seq_no) return NULL; diff --git a/sql/rpl_rli.h b/sql/rpl_rli.h index 06e2379df80..6d6c098847b 100644 --- a/sql/rpl_rli.h +++ b/sql/rpl_rli.h @@ -830,7 +830,7 @@ struct rpl_group_info longlong row_stmt_start_timestamp; bool long_find_row_note_printed; /* Needs room for "Gtid D-S-N\x00". */ - char gtid_info_buf[5+10+1+10+1+20+1]; + mutable char gtid_info_buf[5+10+1+10+1+20+1]; /* The timestamp, from the master, of the commit event. @@ -962,7 +962,7 @@ struct rpl_group_info void slave_close_thread_tables(THD *); void mark_start_commit_no_lock(); void mark_start_commit(); - char *gtid_info(); + char *gtid_info() const; void unmark_start_commit(); longlong get_row_stmt_start_timestamp()