Skip to content

Commit

Permalink
Bug#35183686: ALTER_STORED_COLUMN_ORDER with algorithm=INSTANT makes …
Browse files Browse the repository at this point in the history 
…wrong INSERT REDO log

REDO log is not logged (non instant add/del) column order change with instant DDL.
It might cause wrong REDO log replay when recovery, very danger.

The fix makes the column order change logged also correctly.

Change-Id: I7e617735bb9d327136b18b5165039e1155f1fe50
  • Loading branch information
Yasufumi Kinoshita committed Jan 29, 2024
1 parent a17cfed commit e6e13a8
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 9 deletions.
23 changes: 23 additions & 0 deletions mysql-test/suite/innodb/include/instant_ddl_recovery.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,26 @@ SELECT * FROM t1;
--echo # CLEANUP #
--echo ###########
DROP TABLE t1;

#
# Bug#35183686 ALTER_STORED_COLUMN_ORDER with algorithm=INSTANT makes wrong INSERT REDO log
#
--eval CREATE TABLE t1 (`c` text, `b` blob NOT NULL, `nc03242` longblob, `d` int DEFAULT NULL, UNIQUE KEY `b` (`b`(99))) ENGINE=InnoDB ROW_FORMAT=$row_format

alter table t1
change c c text after d,
add column nc05984 bool,
algorithm=instant;

SET GLOBAL innodb_log_checkpoint_now = ON;
SET GLOBAL innodb_page_cleaner_disabled_debug = 1;
SET GLOBAL innodb_dict_stats_disabled_debug = 1;
SET GLOBAL innodb_master_thread_disabled_debug = 1;

INSERT INTO t1 SET b='ulccclraaacaucrouorouoooolrlo', d=6;

--source include/kill_and_restart_mysqld.inc

SELECT * FROM t1;

DROP TABLE t1;
45 changes: 45 additions & 0 deletions mysql-test/suite/innodb/r/instant_ddl_recovery_debug.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,21 @@ id c1 c3
# CLEANUP #
###########
DROP TABLE t1;
CREATE TABLE t1 (`c` text, `b` blob NOT NULL, `nc03242` longblob, `d` int DEFAULT NULL, UNIQUE KEY `b` (`b`(99))) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
alter table t1
change c c text after d,
add column nc05984 bool,
algorithm=instant;
SET GLOBAL innodb_log_checkpoint_now = ON;
SET GLOBAL innodb_page_cleaner_disabled_debug = 1;
SET GLOBAL innodb_dict_stats_disabled_debug = 1;
SET GLOBAL innodb_master_thread_disabled_debug = 1;
INSERT INTO t1 SET b='ulccclraaacaucrouorouoooolrlo', d=6;
# Kill and restart
SELECT * FROM t1;
b nc03242 d c nc05984
ulccclraaacaucrouorouoooolrlo NULL 6 NULL NULL
DROP TABLE t1;
############################################
# Test instant ADD/DROP COLUMN for DYNAMIC format
############################################
Expand Down Expand Up @@ -252,6 +267,21 @@ id c1 c3
# CLEANUP #
###########
DROP TABLE t1;
CREATE TABLE t1 (`c` text, `b` blob NOT NULL, `nc03242` longblob, `d` int DEFAULT NULL, UNIQUE KEY `b` (`b`(99))) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
alter table t1
change c c text after d,
add column nc05984 bool,
algorithm=instant;
SET GLOBAL innodb_log_checkpoint_now = ON;
SET GLOBAL innodb_page_cleaner_disabled_debug = 1;
SET GLOBAL innodb_dict_stats_disabled_debug = 1;
SET GLOBAL innodb_master_thread_disabled_debug = 1;
INSERT INTO t1 SET b='ulccclraaacaucrouorouoooolrlo', d=6;
# Kill and restart
SELECT * FROM t1;
b nc03242 d c nc05984
ulccclraaacaucrouorouoooolrlo NULL 6 NULL NULL
DROP TABLE t1;
############################################
# Test instant ADD/DROP COLUMN for COMPACT format
############################################
Expand Down Expand Up @@ -379,3 +409,18 @@ id c1 c3
# CLEANUP #
###########
DROP TABLE t1;
CREATE TABLE t1 (`c` text, `b` blob NOT NULL, `nc03242` longblob, `d` int DEFAULT NULL, UNIQUE KEY `b` (`b`(99))) ENGINE=InnoDB ROW_FORMAT=COMPACT;
alter table t1
change c c text after d,
add column nc05984 bool,
algorithm=instant;
SET GLOBAL innodb_log_checkpoint_now = ON;
SET GLOBAL innodb_page_cleaner_disabled_debug = 1;
SET GLOBAL innodb_dict_stats_disabled_debug = 1;
SET GLOBAL innodb_master_thread_disabled_debug = 1;
INSERT INTO t1 SET b='ulccclraaacaucrouorouoooolrlo', d=6;
# Kill and restart
SELECT * FROM t1;
b nc03242 d c nc05984
ulccclraaacaucrouorouoooolrlo NULL 6 NULL NULL
DROP TABLE t1;
48 changes: 39 additions & 9 deletions storage/innobase/mtr/mtr0log.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -667,11 +667,12 @@ template <typename F>
@param[in] n number of fields
@param[in] is_versioned true if table has row versions
@param[in,out] f vector of fields with versions
@param[in] changed_order array indicating fields changed position
@param[in] log_ptr log buffer pointer
@param[in] func callback to check size reopen log buffer */
static bool log_index_fields(const dict_index_t *index, uint16_t n,
bool is_versioned, std::vector<dict_field_t *> &f,
byte *&log_ptr, F &func) {
bool *changed_order, byte *&log_ptr, F &func) {
/* Write metadata for each field. Log the fields in their logical order. */
for (size_t i = 0; i < n; i++) {
dict_field_t *field = index->get_field(i);
Expand All @@ -696,7 +697,8 @@ static bool log_index_fields(const dict_index_t *index, uint16_t n,
log_ptr += 2;

if (is_versioned) {
if (col->is_instant_added() || col->is_instant_dropped()) {
if (col->is_instant_added() || col->is_instant_dropped() ||
changed_order[i]) {
f.push_back(field);
}
}
Expand Down Expand Up @@ -738,7 +740,7 @@ static bool log_index_versioned_fields(const std::vector<dict_field_t *> &f,
| 16th bit indicates add version info follows. */
uint16_t phy_pos = field->get_phy_pos();

ut_ad(field->col->is_instant_added() || field->col->is_instant_dropped());
/* It also might be accompanying column order change (!added&&!dropped) */

if (field->col->is_instant_added()) {
/* Set 16th bit in phy_pos to indicate presence of version added */
Expand Down Expand Up @@ -843,22 +845,54 @@ bool mlog_open_and_write_index(mtr_t *mtr, const byte *rec,
return true;
};

/* Ordinal position of an existing field can't be changed with INSTANT
algorithm. But when it is combined with ADD/DROP COLUMN, ordinal position
of a filed can be changed. This bool array of size #fields in index,
represents if ordinal position of an existing filed is changed. */
bool *fields_with_changed_order = nullptr;
if (is_versioned) {
fields_with_changed_order = new bool[n];
memset(fields_with_changed_order, false, (sizeof(bool) * n));

uint16_t phy_pos = 0;
for (size_t i = 0; i < n; i++) {
dict_field_t *field = index->get_field(i);
const dict_col_t *col = field->col;

if (col->is_instant_added() || col->is_instant_dropped()) {
continue;
} else if (col->get_phy_pos() >= phy_pos) {
phy_pos = col->get_phy_pos();
} else {
fields_with_changed_order[i] = true;
}
}
}

if (is_comp) {
/* Write fields info. */
if (!log_index_fields(index, n, is_versioned, instant_fields_to_log,
log_ptr, f)) {
fields_with_changed_order, log_ptr, f)) {
if (is_versioned) {
delete[] fields_with_changed_order;
}
return false;
}
} else if (is_versioned) {
for (size_t i = 0; i < n; i++) {
dict_field_t *field = index->get_field(i);
const dict_col_t *col = field->col;
if (col->is_instant_added() || col->is_instant_dropped()) {
if (col->is_instant_added() || col->is_instant_dropped() ||
fields_with_changed_order[i]) {
instant_fields_to_log.push_back(field);
}
}
}

if (is_versioned) {
delete[] fields_with_changed_order;
}

if (!instant_fields_to_log.empty()) {
ut_ad(is_versioned);
/* Log INSTANT ADD/DROP fields */
Expand Down Expand Up @@ -1093,7 +1127,6 @@ static void update_instant_info(instant_fields_list_t f, dict_index_t *index) {
for (auto field : f) {
bool is_added = field.v_added != UINT8_UNDEFINED;
bool is_dropped = field.v_dropped != UINT8_UNDEFINED;
ut_ad(is_added || is_dropped);

dict_col_t *col = index->fields[field.logical_pos].col;

Expand Down Expand Up @@ -1277,9 +1310,6 @@ static byte *mlog_parse_index_v1(byte *ptr, const byte *end_ptr,
field->col->set_phy_pos(phy_pos);
phy_pos_bitmap[phy_pos] = true;
} else {
ut_ad(field->col->is_instant_added() ||
field->col->is_instant_dropped());

if (field->col->is_instant_added() &&
!field->col->is_instant_dropped()) {
shift_count--;
Expand Down

0 comments on commit e6e13a8

Please sign in to comment.