From 6c5c1f0b2fc71ab7e90dc7798180135e7967a264 Mon Sep 17 00:00:00 2001 From: Eugene Kosov Date: Thu, 25 Apr 2019 16:29:55 +0300 Subject: [PATCH 01/13] MDEV-19231 make DB_SUCCESS equal to 0 It's a micro optimization. On most platforms CPUs has instructions to compare with 0 fast. DB_SUCCESS is the most popular outcome of functions and this patch optimized code like (err == DB_SUCCESS) BtrBulk::finish(): bogus assertion fixed fil_node_t::read_page0(): corrected usage of os_file_read() que_eval_sql(): bugus assertion removed. Apparently it checked that the field was assigned after having been zero-initialized at object creation. It turns out that the return type of os_file_read_func() was changed in mysql/mysql-server@98909cefbc37e54efc6452c7e95bccbf64ac9213 (MySQL 5.7) from ibool to dberr_t. The reviewer (if there was any) failed to point out that because of future merges, it could be a bad idea to change the return type of a function without changing the function name. This change was applied to MariaDB 10.2.2 in commit 2e814d4702d71a04388386a9f591d14a35980bfe but the MariaDB-specific code was not fully adjusted accordingly, e.g. in fil_node_open_file(). Essentially, code like !os_file_read(...) became dead code in MariaDB and later in Mariabackup 10.2, and we could be dealing with an uninitialized buffer after a failed page read. --- extra/mariabackup/backup_copy.cc | 4 ++-- extra/mariabackup/changed_page_bitmap.cc | 2 +- extra/mariabackup/fil_cur.cc | 6 +++--- extra/mariabackup/xtrabackup.cc | 12 ++++++------ storage/innobase/btr/btr0bulk.cc | 3 ++- storage/innobase/fil/fil0fil.cc | 2 +- storage/innobase/include/db0err.h | 5 +++-- storage/innobase/que/que0que.cc | 2 -- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/extra/mariabackup/backup_copy.cc b/extra/mariabackup/backup_copy.cc index 30ec5539c2d..fa665a798ac 100644 --- a/extra/mariabackup/backup_copy.cc +++ b/extra/mariabackup/backup_copy.cc @@ -559,9 +559,9 @@ datafile_read(datafile_cur_t *cursor) return(XB_FIL_CUR_EOF); } - if (!os_file_read(IORequestRead, + if (os_file_read(IORequestRead, cursor->file, cursor->buf, cursor->buf_offset, - to_read)) { + to_read) != DB_SUCCESS) { return(XB_FIL_CUR_ERROR); } diff --git a/extra/mariabackup/changed_page_bitmap.cc b/extra/mariabackup/changed_page_bitmap.cc index f6e9812b0c5..a21c498527a 100644 --- a/extra/mariabackup/changed_page_bitmap.cc +++ b/extra/mariabackup/changed_page_bitmap.cc @@ -195,7 +195,7 @@ log_online_read_bitmap_page( ut_a(bitmap_file->offset % MODIFIED_PAGE_BLOCK_SIZE == 0); success = os_file_read(IORequestRead, bitmap_file->file, page, bitmap_file->offset, - MODIFIED_PAGE_BLOCK_SIZE); + MODIFIED_PAGE_BLOCK_SIZE) == DB_SUCCESS; if (UNIV_UNLIKELY(!success)) { diff --git a/extra/mariabackup/fil_cur.cc b/extra/mariabackup/fil_cur.cc index fd5e1bc02dd..b5e7ba6a83e 100644 --- a/extra/mariabackup/fil_cur.cc +++ b/extra/mariabackup/fil_cur.cc @@ -253,7 +253,7 @@ xb_fil_cur_open( if (!node->space->crypt_data && os_file_read(IORequestRead, node->handle, cursor->buf, 0, - page_size.physical())) { + page_size.physical()) == DB_SUCCESS) { mutex_enter(&fil_system->mutex); if (!node->space->crypt_data) { node->space->crypt_data @@ -442,8 +442,8 @@ read_retry: cursor->buf_offset = offset; cursor->buf_page_no = (ulint)(offset / page_size); - if (!os_file_read(IORequestRead, cursor->file, cursor->buf, offset, - (ulint) to_read)) { + if (os_file_read(IORequestRead, cursor->file, cursor->buf, offset, + (ulint) to_read) != DB_SUCCESS) { ret = XB_FIL_CUR_ERROR; goto func_exit; } diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index 4b6ba960c05..07d21029733 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -3302,8 +3302,8 @@ static dberr_t xb_assign_undo_space_start() page = static_cast(ut_align(buf, UNIV_PAGE_SIZE)); retry: - if (!os_file_read(IORequestRead, file, page, TRX_SYS_PAGE_NO * UNIV_PAGE_SIZE, - UNIV_PAGE_SIZE)) { + if (os_file_read(IORequestRead, file, page, TRX_SYS_PAGE_NO * UNIV_PAGE_SIZE, + UNIV_PAGE_SIZE) != DB_SUCCESS) { msg("Reading TRX_SYS page failed."); error = DB_ERROR; goto func_exit; @@ -4655,7 +4655,7 @@ xb_space_create_file( free(buf); - if (!ret) { + if (ret != DB_SUCCESS) { msg("mariabackup: could not write the first page to %s", path); os_file_close(*file); @@ -4947,7 +4947,7 @@ xtrabackup_apply_delta( << page_size_shift); success = os_file_read(IORequestRead, src_file, incremental_buffer, offset, page_size); - if (!success) { + if (success != DB_SUCCESS) { goto error; } @@ -4980,7 +4980,7 @@ xtrabackup_apply_delta( success = os_file_read(IORequestRead, src_file, incremental_buffer, offset, page_in_buffer * page_size); - if (!success) { + if (success != DB_SUCCESS) { goto error; } @@ -5029,7 +5029,7 @@ xtrabackup_apply_delta( success = os_file_write(IORequestWrite, dst_path, dst_file, buf, off, page_size); - if (!success) { + if (success != DB_SUCCESS) { goto error; } } diff --git a/storage/innobase/btr/btr0bulk.cc b/storage/innobase/btr/btr0bulk.cc index 998ff3f1e45..2655ce2a2ce 100644 --- a/storage/innobase/btr/btr0bulk.cc +++ b/storage/innobase/btr/btr0bulk.cc @@ -1050,6 +1050,7 @@ BtrBulk::finish(dberr_t err) ut_ad(!sync_check_iterate(dict_sync_check())); - ut_ad(err != DB_SUCCESS || btr_validate_index(m_index, NULL, false)); + ut_ad(err == DB_SUCCESS + || btr_validate_index(m_index, NULL, false) == DB_SUCCESS); return(err); } diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index 0e6f1b05cd6..ccf1ac0ecef 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -539,7 +539,7 @@ bool fil_node_t::read_page0(bool first) /* Align the memory for file i/o if we might have O_DIRECT set */ byte* page = static_cast(ut_align(buf2, psize)); IORequest request(IORequest::READ); - if (!os_file_read(request, handle, page, 0, psize)) { + if (os_file_read(request, handle, page, 0, psize) != DB_SUCCESS) { ib::error() << "Unable to read first page of file " << name; ut_free(buf2); return false; diff --git a/storage/innobase/include/db0err.h b/storage/innobase/include/db0err.h index ec8e29d458c..0d7c7b28ea7 100644 --- a/storage/innobase/include/db0err.h +++ b/storage/innobase/include/db0err.h @@ -30,12 +30,13 @@ Created 5/24/1996 Heikki Tuuri /* Do not include univ.i because univ.i includes this. */ enum dberr_t { + DB_SUCCESS, + DB_SUCCESS_LOCKED_REC = 9, /*!< like DB_SUCCESS, but a new explicit record lock was created */ - DB_SUCCESS = 10, /* The following are error codes */ - DB_ERROR, + DB_ERROR = 11, DB_INTERRUPTED, DB_OUT_OF_MEMORY, DB_OUT_OF_FILE_SPACE, diff --git a/storage/innobase/que/que0que.cc b/storage/innobase/que/que0que.cc index 05964403543..57c7a6216de 100644 --- a/storage/innobase/que/que0que.cc +++ b/storage/innobase/que/que0que.cc @@ -1233,7 +1233,5 @@ que_eval_sql( mutex_exit(&dict_sys->mutex); } - ut_a(trx->error_state != 0); - DBUG_RETURN(trx->error_state); } From b2dbc781c7c7d5638c5e7b3640656cb63543deb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 25 Apr 2019 17:26:23 +0300 Subject: [PATCH 02/13] Implement --debug=d,ib_log_checkpoint_avoid Normally, the InnoDB master thread executes InnoDB log checkpoints so frequently that bugs in crash recovery or redo logging can be hard to reproduce. This is because crash recovery would start replaying the log only from the latest checkpoint. Because the InnoDB redo log format only allows saving information for at most 2 latest checkpoints, and because the log files are written in a circular fashion, it would be challenging to implement a debug option that would start the redo log apply from the very start of the redo log file. --- storage/innobase/srv/srv0srv.cc | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc index 1adf1f137d6..58596175681 100644 --- a/storage/innobase/srv/srv0srv.cc +++ b/storage/innobase/srv/srv0srv.cc @@ -2244,6 +2244,16 @@ srv_master_do_active_tasks(void) MONITOR_SRV_DICT_LRU_MICROSECOND, counter_time); } + /* The periodic log_checkpoint() call here makes it harder to + reproduce bugs in crash recovery or mariabackup --prepare, or + in code that writes the redo log records. Omitting the call + here should not affect correctness, because log_free_check() + should still be invoking checkpoints when needed. In a + production server, those calls could cause "furious flushing" + and stall the server. Normally we want to perform checkpoints + early and often to avoid those situations. */ + DBUG_EXECUTE_IF("ib_log_checkpoint_avoid", return;); + if (srv_shutdown_state != SRV_SHUTDOWN_NONE) { return; } @@ -2323,6 +2333,16 @@ srv_master_do_idle_tasks(void) MONITOR_INC_TIME_IN_MICRO_SECS( MONITOR_SRV_LOG_FLUSH_MICROSECOND, counter_time); + /* The periodic log_checkpoint() call here makes it harder to + reproduce bugs in crash recovery or mariabackup --prepare, or + in code that writes the redo log records. Omitting the call + here should not affect correctness, because log_free_check() + should still be invoking checkpoints when needed. In a + production server, those calls could cause "furious flushing" + and stall the server. Normally we want to perform checkpoints + early and often to avoid those situations. */ + DBUG_EXECUTE_IF("ib_log_checkpoint_avoid", return;); + if (srv_shutdown_state != SRV_SHUTDOWN_NONE) { return; } From 3dffdee667666df9ade9f2c458bf1ea495ffba02 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Thu, 25 Apr 2019 13:43:31 +0200 Subject: [PATCH 03/13] MDEV-17036: BULK with replace doesn't take the first parameter in account INSERT and REPLACE served by the same function, so flags (and processing) should be the same. --- sql/sql_parse.cc | 3 +- tests/mysql_client_test.c | 61 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 146c4d2d02e..68a08dc2b3f 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -609,7 +609,8 @@ void init_update_queries(void) CF_CAN_GENERATE_ROW_EVENTS | CF_OPTIMIZER_TRACE | CF_CAN_BE_EXPLAINED | - CF_INSERTS_DATA | CF_SP_BULK_SAFE; + CF_INSERTS_DATA | CF_SP_BULK_SAFE | + CF_SP_BULK_OPTIMIZED; sql_command_flags[SQLCOM_REPLACE_SELECT]= CF_CHANGES_DATA | CF_REEXECUTION_FRAGILE | CF_CAN_GENERATE_ROW_EVENTS | CF_OPTIMIZER_TRACE | diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index be16db0fc9e..365229e126f 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -19743,6 +19743,66 @@ static void test_bulk_delete() rc= mysql_query(mysql, "DROP TABLE t1"); myquery(rc); } + +static void test_bulk_replace() +{ + int rc; + MYSQL_STMT *stmt; + MYSQL_BIND bind[2]; + MYSQL_ROW row; + int i, + id[]= {1, 2, 3, 4}, + val[]= {1, 1, 1, 1}, + count= sizeof(id)/sizeof(id[0]); + MYSQL_RES *result; + + rc= mysql_query(mysql, "DROP TABLE IF EXISTS t1"); + myquery(rc); + rc= mysql_query(mysql, "CREATE TABLE t1 (id int not null primary key, active int)"); + myquery(rc); + rc= mysql_query(mysql, "insert into t1 values (1, 0), (2, 0), (3, 0)"); + myquery(rc); + verify_affected_rows(3); + + stmt= mysql_stmt_init(mysql); + rc= mysql_stmt_prepare(stmt, "replace into t1 (id, active) values (?, ?)", -1); + check_execute(stmt, rc); + + memset(bind, 0, sizeof(bind)); + bind[0].buffer_type = MYSQL_TYPE_LONG; + bind[0].buffer = (void *)id; + bind[0].buffer_length = 0; + bind[1].buffer_type = MYSQL_TYPE_LONG; + bind[1].buffer = (void *)val; + bind[1].buffer_length = 0; + + mysql_stmt_attr_set(stmt, STMT_ATTR_ARRAY_SIZE, (void*)&count); + rc= mysql_stmt_bind_param(stmt, bind); + check_execute(stmt, rc); + + rc= mysql_stmt_execute(stmt); + check_execute(stmt, rc); + + mysql_stmt_close(stmt); + + rc= mysql_query(mysql, "SELECT active FROM t1"); + myquery(rc); + + result= mysql_store_result(mysql); + mytest(result); + + i= 0; + while ((row= mysql_fetch_row(result))) + { + i++; + DIE_IF(atoi(row[0]) != 1); + } + DIE_IF(i != 4); + mysql_free_result(result); + + rc= mysql_query(mysql, "DROP TABLE t1"); + myquery(rc); +} #endif static struct my_tests_st my_tests[]= { @@ -20026,6 +20086,7 @@ static struct my_tests_st my_tests[]= { { "test_mdev12579", test_mdev12579 }, #ifndef EMBEDDED_LIBRARY { "test_bulk_delete", test_bulk_delete }, + { "test_bulk_replace", test_bulk_replace }, #endif { 0, 0 } }; From 4e01bc8c963d9513625dd984cd1aca24b8a7b516 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Thu, 20 Dec 2018 09:52:34 +0100 Subject: [PATCH 04/13] MDEV-16240: Assertion `0' failed in row_sel_convert_mysql_key_to_innobase Set table in row ID position mode before using this function. --- mysql-test/r/multi_update_innodb.result | 40 +++++++++++++++++ mysql-test/t/multi_update_innodb.test | 49 ++++++++++++++++++++ sql/handler.cc | 15 ++++++- sql/item_subselect.cc | 9 +++- sql/item_subselect.h | 2 +- sql/log_event.cc | 7 +++ sql/sql_insert.cc | 60 ++++++++++++++++++++----- sql/sql_load.cc | 7 +++ sql/sql_select.cc | 16 +++++++ sql/sql_update.cc | 8 ++++ 10 files changed, 197 insertions(+), 16 deletions(-) diff --git a/mysql-test/r/multi_update_innodb.result b/mysql-test/r/multi_update_innodb.result index 5890fd24f5f..294ebfcebdf 100644 --- a/mysql-test/r/multi_update_innodb.result +++ b/mysql-test/r/multi_update_innodb.result @@ -151,3 +151,43 @@ create table t2 like t1; insert into t2 select * from t1; delete t1,t2 from t2,t1 where t1.a<'B' and t2.b=t1.b; drop table t1,t2; +# +# MDEV-16240: Assertion `0' failed in +# row_sel_convert_mysql_key_to_innobase +# +SET @save_sql_mode=@@sql_mode; +set sql_mode=''; +CREATE TABLE `t3` ( +`f1` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE current_timestamp(), +`f2` datetime DEFAULT '2000-01-01 00:00:00' ON UPDATE current_timestamp(), +`f3` TIMESTAMP NULL DEFAULT '2000-01-01 00:00:00', +`pk` datetime NOT NULL DEFAULT '0000-00-00 00:00:00', +`f4` datetime DEFAULT current_timestamp(), +PRIMARY KEY (`pk`), +UNIQUE KEY `f2k` (`f2`), +KEY `f4k` (`f4`) +) ENGINE=InnoDB; +INSERT INTO `t3` VALUES ('2018-05-18 15:08:07','2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00','0000-00-00 00:00:00'),('0000-00-00 00:00:00','0000-00-00 00:00:00','1999-12-31 23:00:00','2002-07-03 23:04:40','0000-00-00 00:00:00'); +CREATE VIEW `v1` AS +SELECT `t3`.`pk` AS `pk`, +`t3`.`f3` AS `f3`, +`t3`.`f4` AS `f4`, +`t3`.`f2` AS `f2`, +`t3`.`f1` AS `f1` +FROM `t3`; +CREATE TABLE `t4` ( +`f1` datetime DEFAULT current_timestamp() ON UPDATE current_timestamp(), +`f3` timestamp NULL DEFAULT NULL, +`f2` timestamp NULL DEFAULT '1999-12-31 23:00:00' ON UPDATE current_timestamp(), +`pk` int(11) NOT NULL, +`f4` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), +PRIMARY KEY (`pk`) +) ENGINE=InnoDB; +INSERT INTO `t4` VALUES ('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,1,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,3,'2018-05-18 15:08:06'),('0000-00-00 00:00:00','0000-00-00 00:00:00',NULL,1976,'0000-00-00 00:00:00'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2000,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2001,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2002,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2003,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2004,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2005,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2018,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2019,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2024,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','1999-12-31 23:00:00',2025,'2018-05-18 15:08:06'),('0000-00-00 00:00:00',NULL,'2018-05-18 15:08:06',2026,'2018-05-18 15:08:06'),('2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00',2027,'0000-00-00 00:00:00'); +UPDATE `v1` t1, `t4` t2 +SET t1.`f2` = 6452736 WHERE t1.`f4` = 6272000; +ERROR 23000: Duplicate entry '0000-00-00 00:00:00' for key 'f2k' +DROP VIEW v1; +DROP TABLE t3,t4; +SET @@sql_mode=@save_sql_mode; +# End of 10.2 tests diff --git a/mysql-test/t/multi_update_innodb.test b/mysql-test/t/multi_update_innodb.test index f5f8f91edb8..2e46ee06d4d 100644 --- a/mysql-test/t/multi_update_innodb.test +++ b/mysql-test/t/multi_update_innodb.test @@ -173,3 +173,52 @@ delete t1,t2 from t2,t1 where t1.a<'B' and t2.b=t1.b; drop table t1,t2; +--echo # +--echo # MDEV-16240: Assertion `0' failed in +--echo # row_sel_convert_mysql_key_to_innobase +--echo # + +SET @save_sql_mode=@@sql_mode; +set sql_mode=''; + +CREATE TABLE `t3` ( + `f1` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE current_timestamp(), + `f2` datetime DEFAULT '2000-01-01 00:00:00' ON UPDATE current_timestamp(), + `f3` TIMESTAMP NULL DEFAULT '2000-01-01 00:00:00', + `pk` datetime NOT NULL DEFAULT '0000-00-00 00:00:00', + `f4` datetime DEFAULT current_timestamp(), + PRIMARY KEY (`pk`), + UNIQUE KEY `f2k` (`f2`), + KEY `f4k` (`f4`) + ) ENGINE=InnoDB; + +INSERT INTO `t3` VALUES ('2018-05-18 15:08:07','2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00','0000-00-00 00:00:00'),('0000-00-00 00:00:00','0000-00-00 00:00:00','1999-12-31 23:00:00','2002-07-03 23:04:40','0000-00-00 00:00:00'); + +CREATE VIEW `v1` AS +SELECT `t3`.`pk` AS `pk`, + `t3`.`f3` AS `f3`, + `t3`.`f4` AS `f4`, + `t3`.`f2` AS `f2`, + `t3`.`f1` AS `f1` +FROM `t3`; + +CREATE TABLE `t4` ( + `f1` datetime DEFAULT current_timestamp() ON UPDATE current_timestamp(), + `f3` timestamp NULL DEFAULT NULL, + `f2` timestamp NULL DEFAULT '1999-12-31 23:00:00' ON UPDATE current_timestamp(), + `pk` int(11) NOT NULL, + `f4` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), + PRIMARY KEY (`pk`) +) ENGINE=InnoDB; + +INSERT INTO `t4` VALUES ('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,1,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,3,'2018-05-18 15:08:06'),('0000-00-00 00:00:00','0000-00-00 00:00:00',NULL,1976,'0000-00-00 00:00:00'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2000,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2001,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2002,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2003,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2004,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2005,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2018,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2019,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2024,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','1999-12-31 23:00:00',2025,'2018-05-18 15:08:06'),('0000-00-00 00:00:00',NULL,'2018-05-18 15:08:06',2026,'2018-05-18 15:08:06'),('2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00',2027,'0000-00-00 00:00:00'); + +--error ER_DUP_ENTRY +UPDATE `v1` t1, `t4` t2 +SET t1.`f2` = 6452736 WHERE t1.`f4` = 6272000; + +DROP VIEW v1; +DROP TABLE t3,t4; +SET @@sql_mode=@save_sql_mode; + +--echo # End of 10.2 tests diff --git a/sql/handler.cc b/sql/handler.cc index ad1bad59efa..e5b452f9649 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -2649,8 +2649,7 @@ int handler::ha_rnd_pos(uchar *buf, uchar *pos) DBUG_ENTER("handler::ha_rnd_pos"); DBUG_ASSERT(table_share->tmp_table != NO_TMP_TABLE || m_lock_type != F_UNLCK); - /* TODO: Find out how to solve ha_rnd_pos when finding duplicate update. */ - /* DBUG_ASSERT(inited == RND); */ + DBUG_ASSERT(inited == RND); TABLE_IO_WAIT(tracker, m_psi, PSI_TABLE_FETCH_ROW, MAX_KEY, 0, { result= rnd_pos(buf, pos); }) @@ -3308,6 +3307,10 @@ void handler::get_auto_increment(ulonglong offset, ulonglong increment, ulonglong nr; int error; MY_BITMAP *old_read_set; + bool rnd_inited= (inited == RND); + + if (rnd_inited && ha_rnd_end()) + return; old_read_set= table->prepare_for_keyread(table->s->next_number_index); @@ -3317,6 +3320,10 @@ void handler::get_auto_increment(ulonglong offset, ulonglong increment, DBUG_ASSERT(0); (void) extra(HA_EXTRA_NO_KEYREAD); *first_value= ULONGLONG_MAX; + if (rnd_inited && ha_rnd_init_with_error(0)) + { + //TODO: it would be nice to return here an error + } return; } @@ -3363,6 +3370,10 @@ void handler::get_auto_increment(ulonglong offset, ulonglong increment, ha_index_end(); table->restore_column_maps_after_keyread(old_read_set); *first_value= nr; + if (rnd_inited && ha_rnd_init_with_error(0)) + { + //TODO: it would be nice to return here an error + } return; } diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 8a9dd083911..8cff8f3a5c4 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -5855,12 +5855,16 @@ Ordered_key::cmp_keys_by_row_data_and_rownum(Ordered_key *key, } -void Ordered_key::sort_keys() +bool Ordered_key::sort_keys() { + if (tbl->file->ha_rnd_init_with_error(0)) + return TRUE; my_qsort2(key_buff, (size_t) key_buff_elements, sizeof(rownum_t), (qsort2_cmp) &cmp_keys_by_row_data_and_rownum, (void*) this); /* Invalidate the current row position. */ cur_key_idx= HA_POS_ERROR; + tbl->file->ha_rnd_end(); + return FALSE; } @@ -6313,7 +6317,8 @@ subselect_rowid_merge_engine::init(MY_BITMAP *non_null_key_parts, /* Sort the keys in each of the indexes. */ for (uint i= 0; i < merge_keys_count; i++) - merge_keys[i]->sort_keys(); + if (merge_keys[i]->sort_keys()) + return TRUE; if (init_queue(&pq, merge_keys_count, 0, FALSE, subselect_rowid_merge_engine::cmp_keys_by_cur_rownum, NULL, diff --git a/sql/item_subselect.h b/sql/item_subselect.h index bd6a1bdc498..5366c759795 100644 --- a/sql/item_subselect.h +++ b/sql/item_subselect.h @@ -1287,7 +1287,7 @@ public: ++cur_key_idx; } - void sort_keys(); + bool sort_keys(); double null_selectivity(); /* diff --git a/sql/log_event.cc b/sql/log_event.cc index 8990e1953b6..7ebc75dd1bf 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -12845,6 +12845,12 @@ Rows_log_event::write_row(rpl_group_info *rgi, if (table->file->ha_table_flags() & HA_DUPLICATE_POS) { DBUG_PRINT("info",("Locating offending record using rnd_pos()")); + + if ((error= table->file->ha_rnd_init_with_error(0))) + { + DBUG_RETURN(error); + } + error= table->file->ha_rnd_pos(table->record[1], table->file->dup_ref); if (error) { @@ -12854,6 +12860,7 @@ Rows_log_event::write_row(rpl_group_info *rgi, table->file->print_error(error, MYF(0)); DBUG_RETURN(error); } + table->file->ha_rnd_end(); } else { diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index c623336fdba..4005153cb64 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -881,7 +881,12 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, #endif /* EMBEDDED_LIBRARY */ { if (duplic != DUP_ERROR || ignore) + { table->file->extra(HA_EXTRA_IGNORE_DUP_KEY); + if (table->file->ha_table_flags() & HA_DUPLICATE_POS && + table->file->ha_rnd_init_with_error(0)) + goto abort; + } /** This is a simple check for the case when the table has a trigger that reads from it, or when the statement invokes a stored function @@ -942,7 +947,10 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, { DBUG_PRINT("info", ("iteration %llu", iteration)); if (iteration && bulk_parameters_set(thd)) - goto abort; + { + error= 1; + goto values_loop_end; + } while ((values= its++)) { @@ -1094,7 +1102,11 @@ values_loop_end: error=1; } if (duplic != DUP_ERROR || ignore) + { table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); + if (table->file->ha_table_flags() & HA_DUPLICATE_POS) + table->file->ha_rnd_end(); + } transactional_table= table->file->has_transactions(); @@ -1705,6 +1717,7 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info) goto err; if (table->file->ha_table_flags() & HA_DUPLICATE_POS) { + DBUG_ASSERT(table->file->inited == handler::RND); if (table->file->ha_rnd_pos(table->record[1],table->file->dup_ref)) goto err; } @@ -3177,6 +3190,9 @@ bool Delayed_insert::handle_inserts(void) max_rows= ULONG_MAX; // Do as much as possible } + if (table->file->ha_rnd_init_with_error(0)) + goto err; + /* We can't use row caching when using the binary log because if we get a crash, then binary log will contain rows that are not yet @@ -3352,6 +3368,8 @@ bool Delayed_insert::handle_inserts(void) } } + table->file->ha_rnd_end(); + if (WSREP((&thd))) thd_proc_info(&thd, "insert done"); else @@ -3649,7 +3667,12 @@ select_insert::prepare(List &values, SELECT_LEX_UNIT *u) thd->cuted_fields=0; if (info.ignore || info.handle_duplicates != DUP_ERROR) + { table->file->extra(HA_EXTRA_IGNORE_DUP_KEY); + if (table->file->ha_table_flags() & HA_DUPLICATE_POS && + table->file->ha_rnd_init_with_error(0)) + DBUG_RETURN(1); + } if (info.handle_duplicates == DUP_REPLACE && (!table->triggers || !table->triggers->has_delete_triggers())) table->file->extra(HA_EXTRA_WRITE_CAN_REPLACE); @@ -3818,6 +3841,9 @@ bool select_insert::prepare_eof() if (!error && thd->is_error()) error= thd->get_stmt_da()->sql_errno(); + if (info.ignore || info.handle_duplicates != DUP_ERROR) + if (table->file->ha_table_flags() & HA_DUPLICATE_POS) + table->file->ha_rnd_end(); table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); @@ -3929,6 +3955,11 @@ void select_insert::abort_result_set() { if (thd->locked_tables_mode <= LTM_LOCK_TABLES) table->file->ha_end_bulk_insert(); + if (table->file->inited) + table->file->ha_rnd_end(); + table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); + table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); + /* If at least one row has been inserted/modified and will stay in the table (the table doesn't have transactions) we must write to @@ -4341,7 +4372,12 @@ select_create::prepare(List &values, SELECT_LEX_UNIT *u) restore_record(table,s->default_values); // Get empty record thd->cuted_fields=0; if (info.ignore || info.handle_duplicates != DUP_ERROR) + { table->file->extra(HA_EXTRA_IGNORE_DUP_KEY); + if (table->file->ha_table_flags() & HA_DUPLICATE_POS && + table->file->ha_rnd_init_with_error(0)) + DBUG_RETURN(1); + } if (info.handle_duplicates == DUP_REPLACE && (!table->triggers || !table->triggers->has_delete_triggers())) table->file->extra(HA_EXTRA_WRITE_CAN_REPLACE); @@ -4522,9 +4558,6 @@ bool select_create::send_eof() */ exit_done= 1; // Avoid double calls - table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); - table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); - send_ok_packet(); if (m_plock) @@ -4600,13 +4633,6 @@ void select_create::abort_result_set() thd->locked_tables_list.unlock_locked_table(thd, create_info->mdl_ticket); } - if (m_plock) - { - mysql_unlock_tables(thd, *m_plock); - *m_plock= NULL; - m_plock= NULL; - } - if (table) { bool tmp_table= table->s->tmp_table; @@ -4617,9 +4643,21 @@ void select_create::abort_result_set() thd->restore_tmp_table_share(saved_tmp_table_share); } + if (table->file->inited && + (info.ignore || info.handle_duplicates != DUP_ERROR) && + (table->file->ha_table_flags() & HA_DUPLICATE_POS)) + table->file->ha_rnd_end(); table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); table->auto_increment_field_not_null= FALSE; + + if (m_plock) + { + mysql_unlock_tables(thd, *m_plock); + *m_plock= NULL; + m_plock= NULL; + } + drop_open_table(thd, table, create_table->db, create_table->table_name); table=0; // Safety if (thd->log_current_statement && mysql_bin_log.is_open()) diff --git a/sql/sql_load.cc b/sql/sql_load.cc index 49558f8b694..0fcc4efbccd 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -660,6 +660,10 @@ int mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list, thd->abort_on_warning= !ignore && thd->is_strict_mode(); + if ((table_list->table->file->ha_table_flags() & HA_DUPLICATE_POS) && + (error= table_list->table->file->ha_rnd_init_with_error(0))) + goto err; + thd_progress_init(thd, 2); if (table_list->table->validate_default_values_of_unset_fields(thd)) { @@ -679,6 +683,9 @@ int mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list, set_fields, set_values, read_info, *ex->enclosed, skip_lines, ignore); + if (table_list->table->file->ha_table_flags() & HA_DUPLICATE_POS) + table_list->table->file->ha_rnd_end(); + thd_proc_info(thd, "End bulk insert"); if (!error) thd_progress_next_stage(thd); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 3c1cea6be51..291560dc098 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -20318,6 +20318,15 @@ end_unique_update(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), table->file->print_error(error,MYF(0)); /* purecov: inspected */ DBUG_RETURN(NESTED_LOOP_ERROR); /* purecov: inspected */ } + /* Prepare table for random positioning */ + bool rnd_inited= (table->file->inited == handler::RND); + if (!rnd_inited && + ((error= table->file->ha_index_end()) || + (error= table->file->ha_rnd_init(0)))) + { + table->file->print_error(error, MYF(0)); + DBUG_RETURN(NESTED_LOOP_ERROR); + } if (table->file->ha_rnd_pos(table->record[1],table->file->dup_ref)) { table->file->print_error(error,MYF(0)); /* purecov: inspected */ @@ -20331,6 +20340,13 @@ end_unique_update(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), table->file->print_error(error,MYF(0)); /* purecov: inspected */ DBUG_RETURN(NESTED_LOOP_ERROR); /* purecov: inspected */ } + if (!rnd_inited && + ((error= table->file->ha_rnd_end()) || + (error= table->file->ha_index_init(0, 0)))) + { + table->file->print_error(error, MYF(0)); + DBUG_RETURN(NESTED_LOOP_ERROR); + } } if (join->thd->check_killed()) { diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 80ecd820046..1da265d7c16 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -211,6 +211,14 @@ static void prepare_record_for_error_message(int error, TABLE *table) bitmap_union(table->read_set, &unique_map); /* Tell the engine about the new set. */ table->file->column_bitmaps_signal(); + + if ((error= table->file->ha_index_or_rnd_end()) || + (error= table->file->ha_rnd_init(0))) + { + table->file->print_error(error, MYF(0)); + DBUG_VOID_RETURN; + } + /* Read record that is identified by table->file->ref. */ (void) table->file->ha_rnd_pos(table->record[1], table->file->ref); /* Copy the newly read columns into the new record. */ From 9a5a86f293b6fe40ad606de43c04a2d8ba6b60b1 Mon Sep 17 00:00:00 2001 From: Sujatha Sivakumar Date: Thu, 25 Apr 2019 13:25:28 +0530 Subject: [PATCH 05/13] MDEV-17260: Memory leaks in mysqlbinlog Problem: ======== The mysqlbinlog tool is leaking memory, causing failures in various tests when compiling and testing with AddressSanitizer or LeakSanitizer like this: cmake -DCMAKE_BUILD_TYPE=Debug -DWITH_ASAN:BOOL=ON /path/to/source make -j$(nproc) cd mysql-test ASAN_OPTIONS=abort_on_error=1 ./mtr --parallel=auto Analysis: ========= Two types of leaks were observed during above execution. 1) Leak in Log_event::read_log_event(char const*, unsigned int, char const**, Format_description_log_event const*, char) File: sql/log_event.cc:2150 For all row based replication events the memory which is allocated during read_log_event is not freed after the event is processed. The event specific memory has to be retained only when flashback option is enabled with mysqlbinlog tool. In this case all the events are retained till the end statement is received and they are processed in reverse order and they are destroyed. But in the existing code all events are retained irrespective of flashback mode. Hence the memory leaks are observed. 2) read_remote_annotate_event(unsigned char*, unsigned long, char const**) File: client/mysqlbinlog.cc:194 In general the Annotate event is not printed immediately because all subsequent rbr-events can be filtered away. Instead it will be printed together with the first not filtered away Table map or the last rbr will be processed. While reading remote annotate events memory is allocated for event buffer and event's temp_buf is made to point to the allocated buffer as shown below. The TRUE flag is used for doing proper cleanup using free_temp_buf(). i.e at the time of deletion of annotate event its destructor takes care of clearing the temp_buf. /* Ensure the event->temp_buf is pointing to the allocated buffer. (TRUE = free temp_buf on the event deletion) */ event->register_temp_buf((char*)event_buf, TRUE); But existing code does the following when it receives a remote annotate_event. if (remote_opt) ev->temp_buf= 0; That is code immediately sets temp_buf=0, because of which free_temp_buf() call will return empty handed as it has lost the reference to the allocated temporary buffer. This results in memory leak Fix: ==== 1) If not in flashback mode, destroy the memory for events once they are processed. 2) Remove the ev->temp_buf=0 code for remote option. Let the proper cleanup to be done as part of free_temp_buf(). --- client/mysqlbinlog.cc | 9 +++---- mysql-test/suite/binlog/r/flashback.result | 18 +++++++++++++ mysql-test/suite/binlog/t/flashback.test | 30 +++++++++++++++++++++- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/client/mysqlbinlog.cc b/client/mysqlbinlog.cc index cfc05cbf794..3dad4fef67b 100644 --- a/client/mysqlbinlog.cc +++ b/client/mysqlbinlog.cc @@ -227,8 +227,7 @@ void print_annotate_event(PRINT_EVENT_INFO *print_event_info) if (annotate_event) { annotate_event->print(result_file, print_event_info); - delete annotate_event; // the event should not be printed more than once - annotate_event= 0; + free_annotate_event(); } } @@ -1465,7 +1464,7 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev, if (print_row_event(print_event_info, ev, e->get_table_id(), e->get_flags(Rows_log_event::STMT_END_F))) goto err; - if (!is_stmt_end) + if (opt_flashback && !is_stmt_end) destroy_evt= FALSE; break; } @@ -1478,7 +1477,7 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev, if (print_row_event(print_event_info, ev, e->get_table_id(), e->get_flags(Old_rows_log_event::STMT_END_F))) goto err; - if (!is_stmt_end) + if (opt_flashback && !is_stmt_end) destroy_evt= FALSE; break; } @@ -1539,8 +1538,6 @@ end: } } - if (remote_opt) - ev->temp_buf= 0; if (destroy_evt) /* destroy it later if not set (ignored table map) */ delete ev; } diff --git a/mysql-test/suite/binlog/r/flashback.result b/mysql-test/suite/binlog/r/flashback.result index c96eaebe838..0d189b735a3 100644 --- a/mysql-test/suite/binlog/r/flashback.result +++ b/mysql-test/suite/binlog/r/flashback.result @@ -674,6 +674,24 @@ world.city 563256876 DROP TABLE test.test; DROP TABLE world.city; DROP DATABASE world; +# < CASE 7 > +# Test Case for MDEV-17260 +# +RESET MASTER; +CREATE TABLE t1 ( f INT PRIMARY KEY ) ENGINE=innodb; +INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6); +# 6- Rows must be present +SELECT COUNT(*) FROM t1; +COUNT(*) +6 +FLUSH LOGS; +DELETE FROM t1; +FLUSH LOGS; +# 0- Rows must be present +include/assert.inc [Table t1 should have 0 rows.] +# 6- Rows must be present upon restoring from flashback +include/assert.inc [Table t1 should have six rows.] +DROP TABLE t1; SET binlog_format=statement; Warnings: Warning 1105 MariaDB Galera and flashback do not support binlog format: STATEMENT diff --git a/mysql-test/suite/binlog/t/flashback.test b/mysql-test/suite/binlog/t/flashback.test index 3fc8c44c60c..9782fa4ec83 100644 --- a/mysql-test/suite/binlog/t/flashback.test +++ b/mysql-test/suite/binlog/t/flashback.test @@ -335,8 +335,36 @@ DROP TABLE test.test; DROP TABLE world.city; DROP DATABASE world; -## Clear +--echo # < CASE 7 > +--echo # Test Case for MDEV-17260 +--echo # +RESET MASTER; + +CREATE TABLE t1 ( f INT PRIMARY KEY ) ENGINE=innodb; +INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6); +--echo # 6- Rows must be present +SELECT COUNT(*) FROM t1; +FLUSH LOGS; +DELETE FROM t1; +FLUSH LOGS; + +--echo # 0- Rows must be present +--let $assert_cond= COUNT(*) = 0 FROM t1 +--let $assert_text= Table t1 should have 0 rows. +--source include/assert.inc + +--exec $MYSQL_BINLOG -vv -B --read-from-remote-server --user=root --host=127.0.0.1 --port=$MASTER_MYPORT master-bin.000002> $MYSQLTEST_VARDIR/tmp/mysqlbinlog_row_flashback_7.sql +--exec $MYSQL -e "SET binlog_format= ROW; source $MYSQLTEST_VARDIR/tmp/mysqlbinlog_row_flashback_7.sql;" + +--echo # 6- Rows must be present upon restoring from flashback +--let $assert_cond= COUNT(*) = 6 FROM t1 +--let $assert_text= Table t1 should have six rows. +--source include/assert.inc + +DROP TABLE t1; + +## Clear SET binlog_format=statement; --error ER_FLASHBACK_NOT_SUPPORTED SET GLOBAL binlog_format=statement; From 5cfc7799a33576bb638652bac6e341570c80a7e1 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 26 Apr 2019 14:01:21 +0400 Subject: [PATCH 06/13] MDEV-16518 MYSQL57_GENERATED_FIELD: The code in TABLE_SHARE::init_from_binary_frm_image() is not safe --- mysql-test/std_data/frm/mdev16518.frm | Bin 0 -> 8619 bytes mysql-test/suite/vcol/r/vcol_misc.result | 16 ++++++++++++++++ mysql-test/suite/vcol/t/vcol_misc.test | 23 +++++++++++++++++++++++ sql/table.cc | 3 ++- 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 mysql-test/std_data/frm/mdev16518.frm diff --git a/mysql-test/std_data/frm/mdev16518.frm b/mysql-test/std_data/frm/mdev16518.frm new file mode 100644 index 0000000000000000000000000000000000000000..72a4c41f0b4f3c4de8471b474ce4c4c813cdd36e GIT binary patch literal 8619 zcmeI&K?;IE6b9hm85ND7h!EVl39=S;ga}$n(XQPbV`kS)Izad6SWQn%bJLfiK87s literal 0 HcmV?d00001 diff --git a/mysql-test/suite/vcol/r/vcol_misc.result b/mysql-test/suite/vcol/r/vcol_misc.result index bedc44e7fc4..291a954baf6 100644 --- a/mysql-test/suite/vcol/r/vcol_misc.result +++ b/mysql-test/suite/vcol/r/vcol_misc.result @@ -409,3 +409,19 @@ Warning 1918 Encountered illegal value '\xF0\xF1\xF2\xF3\xF4\xF5\xF6\xF7' when c # # End of 10.1 tests # +# +# Start of 10.2 tests +# +# +# MDEV-16518 MYSQL57_GENERATED_FIELD: The code in TABLE_SHARE::init_from_binary_frm_image() is not safe +# +SHOW TABLES; +Tables_in_test +t1 +SHOW CREATE TABLE t1; +ERROR HY000: Incorrect information in file: './test/t1.frm' +ALTER TABLE t1; +ERROR HY000: Incorrect information in file: './test/t1.frm' +# +# End of 10.2 tests +# diff --git a/mysql-test/suite/vcol/t/vcol_misc.test b/mysql-test/suite/vcol/t/vcol_misc.test index b351e1eb4a6..255621845fb 100644 --- a/mysql-test/suite/vcol/t/vcol_misc.test +++ b/mysql-test/suite/vcol/t/vcol_misc.test @@ -371,3 +371,26 @@ SELECT COLUMN_GET(@aaa, 'price' AS DOUBLE) aaa; --echo # --echo # End of 10.1 tests --echo # + + +--echo # +--echo # Start of 10.2 tests +--echo # + +--echo # +--echo # MDEV-16518 MYSQL57_GENERATED_FIELD: The code in TABLE_SHARE::init_from_binary_frm_image() is not safe +--echo # + +--copy_file std_data/frm/mdev16518.frm $MYSQLD_DATADIR/test/t1.frm +SHOW TABLES; +--replace_result $MYSQLD_DATADIR ./ +--error ER_NOT_FORM_FILE +SHOW CREATE TABLE t1; +--replace_result $MYSQLD_DATADIR ./ +--error ER_NOT_FORM_FILE +ALTER TABLE t1; +--remove_file $MYSQLD_DATADIR/test/t1.frm + +--echo # +--echo # End of 10.2 tests +--echo # diff --git a/sql/table.cc b/sql/table.cc index 2cf21c889e2..5337a506215 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1839,7 +1839,8 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, goto err; vcol_info= new (&share->mem_root) Virtual_column_info(); vcol_info_length= uint2korr(vcol_screen_pos + 1); - DBUG_ASSERT(vcol_info_length); + if (!vcol_info_length) // Expect non-empty expression + goto err; vcol_info->stored_in_db= vcol_screen_pos[3]; vcol_info->utf8= 0; vcol_screen_pos+= vcol_info_length + MYSQL57_GCOL_HEADER_SIZE;; From 1c4d1f3d057428fb9f30a16a1b47db576ec75916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 26 Apr 2019 12:53:37 +0300 Subject: [PATCH 07/13] innobase_col_check_fk(): Remove copying --- storage/innobase/handler/handler0alter.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index a3b03a45eb3..05bb8ac61cd 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -1376,14 +1376,10 @@ innobase_col_check_fk( { dict_s_col_list::const_iterator it; - for (it = s_cols->begin(); - it != s_cols->end(); ++it) { - dict_s_col_t s_col = *it; - - for (ulint j = 0; j < s_col.num_base; j++) { - if (strcmp(col_name, dict_table_get_col_name( - table, - s_col.base_col[j]->ind)) == 0) { + for (it = s_cols->begin(); it != s_cols->end(); ++it) { + for (ulint j = it->num_base; j--; ) { + if (!strcmp(col_name, dict_table_get_col_name( + table, it->base_col[j]->ind))) { return(true); } } From 06ec56f579e6de647fd38854da31b6324100faf9 Mon Sep 17 00:00:00 2001 From: Sachin Agarwal Date: Mon, 26 Nov 2018 16:17:40 +0530 Subject: [PATCH 08/13] Bug #27850600 INNODB ASYNC IO ERROR HANDLING IN IO_EVENT Problem: io_getevents() - read asynchronous I/O events from the completion queue. For each IO event, the res field in io_event tells whether IO event is succeeded or not. To see if the IO actually succeeded we always need to check event.res (negative=error, positive=bytesread/written). LinuxAIOHandler::collect() doesn't check event.res value for each event. which leads to incorrect value in n_bytes for IO context (or IO Slot). Fix: Added a check for event.res negative value. RB: 20871 Reviewed by : annamalai.gurusami@oracle.com --- storage/innobase/os/os0file.cc | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index 10a8f44b062..98a328c7f47 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -1962,10 +1962,24 @@ LinuxAIOHandler::collect() will be done in the calling function. */ m_array->acquire(); - slot->ret = events[i].res2; + /* events[i].res2 should always be ZERO */ + ut_ad(events[i].res2 == 0); slot->io_already_done = true; - slot->n_bytes = events[i].res; + /*Even though events[i].res is an unsigned number + in libaio, it is used to return a negative value + (negated errno value) to indicate error and a positive + value to indicate number of bytes read or written. */ + + if (events[i].res > slot->len) { + /* failure */ + slot->n_bytes = 0; + slot->ret = events[i].res; + } else { + /* success */ + slot->n_bytes = events[i].res; + slot->ret = 0; + } m_array->release(); } From c795a9f3fea60d303645c6729834cdd389aaed0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 26 Apr 2019 10:13:29 +0300 Subject: [PATCH 09/13] MDEV-12004: Add the Bug#28825718 test case Adapt the test case from mysql/mysql-server@2bbbcddd903626c84c610dd2ed82cc22ee4d6cde (MySQL 5.7.26). --- .../suite/gcol/r/innodb_virtual_debug.result | 30 ++++++++++++++++ .../suite/gcol/t/innodb_virtual_debug.test | 34 ++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/gcol/r/innodb_virtual_debug.result b/mysql-test/suite/gcol/r/innodb_virtual_debug.result index 50b714566d9..806cf1a98c8 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_debug.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_debug.result @@ -94,3 +94,33 @@ NULL 29 DROP TABLE t; SET DEBUG_SYNC = 'RESET'; +# +# Bug#28825718 - ASSERTION FAILURE: TRX0REC.CC:NNN:N_IDX > 0 WHILE DOING REPLACE/INSERT +# +CREATE TABLE t1(a INT PRIMARY KEY, b INT, c INT GENERATED ALWAYS AS(b+1) VIRTUAL) ENGINE=InnoDB; +INSERT INTO t1(a, b) VALUES(1, 1); +connect con1,localhost,root,,; +SET DEBUG_SYNC = 'row_log_apply_after SIGNAL s1 WAIT_FOR s2'; +SET lock_wait_timeout = 1; +ALTER TABLE t1 ADD UNIQUE INDEX(c, b); +connection default; +SET DEBUG_SYNC = 'now WAIT_FOR s1'; +SET DEBUG_SYNC = 'row_ins_sec_index_enter SIGNAL s2 WAIT_FOR s3'; +INSERT INTO t1(a, b) VALUES(2, 2); +connection con1; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +SET DEBUG_SYNC = 'now SIGNAL s3'; +disconnect con1; +connection default; +SET DEBUG_SYNC = 'RESET'; +ALTER TABLE t1 ADD KEY(b); +INSERT INTO t1(a, b) VALUES(3, 3); +SELECT * FROM t1; +a b c +1 1 2 +2 2 3 +3 3 4 +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +DROP TABLE t1; diff --git a/mysql-test/suite/gcol/t/innodb_virtual_debug.test b/mysql-test/suite/gcol/t/innodb_virtual_debug.test index ccdd16c9ebe..40446b991cd 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_debug.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_debug.test @@ -60,7 +60,6 @@ SET DEBUG_SYNC = 'innodb_inplace_alter_table_enter SIGNAL start_create WAIT_FOR --send ALTER TABLE t FORCE connection con1; - SET DEBUG_SYNC = 'now WAIT_FOR start_create'; start transaction; update t set a=1 where a = 0; @@ -280,4 +279,37 @@ disconnect con1; } SET DEBUG_SYNC = 'RESET'; + +--echo # +--echo # Bug#28825718 - ASSERTION FAILURE: TRX0REC.CC:NNN:N_IDX > 0 WHILE DOING REPLACE/INSERT +--echo # + +CREATE TABLE t1(a INT PRIMARY KEY, b INT, c INT GENERATED ALWAYS AS(b+1) VIRTUAL) ENGINE=InnoDB; + +INSERT INTO t1(a, b) VALUES(1, 1); + +connect (con1,localhost,root,,); +SET DEBUG_SYNC = 'row_log_apply_after SIGNAL s1 WAIT_FOR s2'; +SET lock_wait_timeout = 1; +--send ALTER TABLE t1 ADD UNIQUE INDEX(c, b) + +connection default; +SET DEBUG_SYNC = 'now WAIT_FOR s1'; +SET DEBUG_SYNC = 'row_ins_sec_index_enter SIGNAL s2 WAIT_FOR s3'; +--send INSERT INTO t1(a, b) VALUES(2, 2) + +connection con1; +--error ER_LOCK_WAIT_TIMEOUT +reap; +SET DEBUG_SYNC = 'now SIGNAL s3'; +disconnect con1; +connection default; +reap; +SET DEBUG_SYNC = 'RESET'; +ALTER TABLE t1 ADD KEY(b); +INSERT INTO t1(a, b) VALUES(3, 3); +SELECT * FROM t1; +CHECK TABLE t1; +DROP TABLE t1; + --source include/wait_until_count_sessions.inc From 793bd3ee13972896e576a67528f6d940ad8261c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 26 Apr 2019 17:36:39 +0300 Subject: [PATCH 10/13] lock_rec_convert_impl_to_expl_for_trx(): Remove unused parameter --- storage/innobase/lock/lock0lock.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index f43694b6926..7f6171241ac 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -6030,7 +6030,6 @@ lock_rec_convert_impl_to_expl_for_trx( const buf_block_t* block, /*!< in: buffer block of rec */ const rec_t* rec, /*!< in: user record on page */ dict_index_t* index, /*!< in: index of record */ - const ulint* offsets,/*!< in: rec_get_offsets(rec, index) */ trx_t* trx, /*!< in/out: active transaction */ ulint heap_no)/*!< in: rec heap number to lock */ { @@ -6105,7 +6104,7 @@ lock_rec_convert_impl_to_expl( trx cannot be committed until the ref count is zero. */ lock_rec_convert_impl_to_expl_for_trx( - block, rec, index, offsets, trx, heap_no); + block, rec, index, trx, heap_no); } } From f3a9f12bc327d9568488999a20120eabcd1e6d68 Mon Sep 17 00:00:00 2001 From: Aditya A Date: Fri, 25 Jan 2019 19:51:57 +0530 Subject: [PATCH 11/13] Bug #29021730 CRASHING INNOBASE_COL_CHECK_FK WITH FOREIGN KEYS PROBLEM ------- Function innodb_base_col_setup_for_stored() was skipping to store the base column information for a generated column if the base column was a "STORED" generated column. This later causes a crash in function innoabse_col_check_fk() where it says that a generated columns depends upon two base columns ,but there is information on only one of them. There was a explicit check barring the stored columns being stored, which is wrong because the documentation says that a generated stored column can be a part of a generated column. FIX ---- Store the information of base column if it is a stored generated column. #RB21247 Reviewed by: Debarun Banerjee --- storage/innobase/handler/ha_innodb.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index d9ee942324a..953a4d89037 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 2000, 2018, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2000, 2019, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, 2009 Google Inc. Copyright (c) 2009, Percona Inc. Copyright (c) 2012, Facebook Inc. @@ -10977,7 +10977,7 @@ 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 (!base_field->vcol_info + if (base_field->stored_in_db() && bitmap_is_set(&field->table->tmp_set, i)) { ulint z; for (z = 0; z < table->n_cols; z++) { From 4e9f8c9cc4932db53d5bae456bcdb9f33f822254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 26 Apr 2019 17:37:19 +0300 Subject: [PATCH 12/13] Remove roll_node_t::partial The field roll_node_t::partial holds if and only if savept has been set. Make savept a pointer. trx_rollback_start(): Use the semantic type undo_no_t for roll_limit. --- storage/innobase/include/trx0roll.h | 6 ++---- storage/innobase/trx/trx0roll.cc | 7 +++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/storage/innobase/include/trx0roll.h b/storage/innobase/include/trx0roll.h index d6fe576da90..70eaf48b83b 100644 --- a/storage/innobase/include/trx0roll.h +++ b/storage/innobase/include/trx0roll.h @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2015, 2018, MariaDB Corporation. +Copyright (c) 2015, 2019, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -200,9 +200,7 @@ enum roll_node_state { struct roll_node_t{ que_common_t common; /*!< node type: QUE_NODE_ROLLBACK */ enum roll_node_state state; /*!< node execution state */ - bool partial;/*!< TRUE if we want a partial - rollback */ - trx_savept_t savept; /*!< savepoint to which to + const trx_savept_t* savept; /*!< savepoint to which to roll back, in the case of a partial rollback */ que_thr_t* undo_thr;/*!< undo query graph */ diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc index a592fb47d21..474b627a11e 100644 --- a/storage/innobase/trx/trx0roll.cc +++ b/storage/innobase/trx/trx0roll.cc @@ -83,8 +83,7 @@ trx_rollback_to_savepoint_low( roll_node = roll_node_create(heap); if (savept != NULL) { - roll_node->partial = TRUE; - roll_node->savept = *savept; + roll_node->savept = savept; check_trx_state(trx); } else { assert_trx_nonlocking_or_in_list(trx); @@ -1122,7 +1121,7 @@ que_thr_t* trx_rollback_start( /*===============*/ trx_t* trx, /*!< in: transaction */ - ib_id_t roll_limit) /*!< in: rollback to undo no (for + undo_no_t roll_limit) /*!< in: rollback to undo no (for partial undo), 0 if we are rolling back the entire transaction */ { @@ -1215,7 +1214,7 @@ trx_rollback_step( ut_a(node->undo_thr == NULL); - roll_limit = node->partial ? node->savept.least_undo_no : 0; + roll_limit = node->savept ? node->savept->least_undo_no : 0; trx_commit_or_rollback_prepare(trx); From 00377147e3029b982cbc29d3f4477362c6e6fdb4 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 26 Apr 2019 23:06:07 +0400 Subject: [PATCH 13/13] Tests for MDEV-15881 Assertion `is_valid_value_slow()' failed in Datetime::Datetime or corrupt data after ALTER with indexed persistent column The patch for "MDEV-18486 Database crash on a table with indexed virtual column" fixed MDEV-15881 as well. So adding tests only. --- .../suite/vcol/r/vcol_keys_myisam.result | 25 +++++++++++++++++ mysql-test/suite/vcol/t/vcol_keys_myisam.test | 28 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/mysql-test/suite/vcol/r/vcol_keys_myisam.result b/mysql-test/suite/vcol/r/vcol_keys_myisam.result index cde93889848..b7086600ab1 100644 --- a/mysql-test/suite/vcol/r/vcol_keys_myisam.result +++ b/mysql-test/suite/vcol/r/vcol_keys_myisam.result @@ -383,3 +383,28 @@ select * from t1; id hexid 100 64 drop table t1; +# +# MDEV-15881 Assertion `is_valid_value_slow()' failed in Datetime::Datetime or corrupt data after ALTER with indexed persistent column +# +CREATE TABLE t1 (i INT, d1 DATE, d2 DATE NOT NULL, t TIMESTAMP, KEY(t)) ENGINE=MyISAM; +INSERT INTO t1 VALUES (1,'2023-03-16','2023-03-15','2012-12-12 12:12:12'); +ALTER TABLE t1 MODIFY t FLOAT AS (i) PERSISTENT; +SELECT i, d1, d2 FROM t1 INTO OUTFILE 'load_t1'; +DELETE FROM t1; +LOAD DATA INFILE 'load_t1' INTO TABLE t1 (i,d1,d2); +SELECT * FROM t1 WHERE d2 < d1; +i d1 d2 t +1 2023-03-16 2023-03-15 1 +DROP TABLE t1; +CREATE TABLE t1 ( +i INT DEFAULT NULL, +d1 DATE DEFAULT NULL, +d2 DATE NOT NULL, +t FLOAT GENERATED ALWAYS AS (i) STORED, +KEY (t) +) ENGINE=MyISAM; +LOAD DATA INFILE 'load_t1' INTO TABLE t1 (i,d1,d2); +SELECT * FROM t1 WHERE d2 < d1; +i d1 d2 t +1 2023-03-16 2023-03-15 1 +DROP TABLE t1; diff --git a/mysql-test/suite/vcol/t/vcol_keys_myisam.test b/mysql-test/suite/vcol/t/vcol_keys_myisam.test index 3269979fc9e..99b1c9a444b 100644 --- a/mysql-test/suite/vcol/t/vcol_keys_myisam.test +++ b/mysql-test/suite/vcol/t/vcol_keys_myisam.test @@ -272,3 +272,31 @@ create table t1 ( id int primary key, insert into t1 (id) select 100; select * from t1; drop table t1; + + +--echo # +--echo # MDEV-15881 Assertion `is_valid_value_slow()' failed in Datetime::Datetime or corrupt data after ALTER with indexed persistent column +--echo # + +CREATE TABLE t1 (i INT, d1 DATE, d2 DATE NOT NULL, t TIMESTAMP, KEY(t)) ENGINE=MyISAM; +INSERT INTO t1 VALUES (1,'2023-03-16','2023-03-15','2012-12-12 12:12:12'); +ALTER TABLE t1 MODIFY t FLOAT AS (i) PERSISTENT; +SELECT i, d1, d2 FROM t1 INTO OUTFILE 'load_t1'; +DELETE FROM t1; +LOAD DATA INFILE 'load_t1' INTO TABLE t1 (i,d1,d2); +SELECT * FROM t1 WHERE d2 < d1; +DROP TABLE t1; + +CREATE TABLE t1 ( + i INT DEFAULT NULL, + d1 DATE DEFAULT NULL, + d2 DATE NOT NULL, + t FLOAT GENERATED ALWAYS AS (i) STORED, + KEY (t) +) ENGINE=MyISAM; +LOAD DATA INFILE 'load_t1' INTO TABLE t1 (i,d1,d2); +SELECT * FROM t1 WHERE d2 < d1; +DROP TABLE t1; +# Cleanup +--let $datadir= `SELECT @@datadir` +--remove_file $datadir/test/load_t1