From a31e99a89cc75804c9d118835b39d9780f504312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 30 May 2018 10:48:48 +0300 Subject: [PATCH 1/5] Remove an unnecessary #include --- storage/innobase/handler/handler0alter.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index d1a68813a29..bc88001edf6 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -54,10 +54,6 @@ Smart ALTER TABLE #include "ha_innodb.h" #include "ut0new.h" #include "ut0stage.h" -#ifdef WITH_WSREP -//#include "wsrep_api.h" -#include // PROCESS_ACL -#endif static const char *MSG_UNSUPPORTED_ALTER_ONLINE_ON_VIRTUAL_COLUMN= "INPLACE ADD or DROP of virtual columns cannot be " From b2f86ebdd254d923daf6f29e64e61e19187044b9 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Thu, 31 May 2018 18:55:07 -0700 Subject: [PATCH 2/5] MDEV-16353 Server crash on query with CTE This bug caused crashes for queries with unreferenced non-recursive CTEs specified by unions.It happened because the function st_select_lex_unit::prepare() tried to use the value of the field 'derived' that could not be set for unferenced CTEs as there was no derived table associated with an unreferenced CTE. --- mysql-test/r/cte_nonrecursive.result | 16 ++++++++++++++++ mysql-test/t/cte_nonrecursive.test | 18 ++++++++++++++++++ sql/sql_union.cc | 2 +- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/cte_nonrecursive.result b/mysql-test/r/cte_nonrecursive.result index 001df909bcf..1d079c3bfa6 100644 --- a/mysql-test/r/cte_nonrecursive.result +++ b/mysql-test/r/cte_nonrecursive.result @@ -1462,3 +1462,19 @@ a b 4 5 4 3 DROP TABLE t1; +# +# MDEV-16353: unreferenced CTE specified by query with UNION +# +with cte as +(select 1 union select 2 union select 3) +select 1 as f; +f +1 +create table t1 (a int); +insert into t1 values (2), (1), (7), (1), (4); +with cte as +(select * from t1 where a < 2 union select * from t1 where a > 5) +select 2 as f; +f +2 +drop table t1; diff --git a/mysql-test/t/cte_nonrecursive.test b/mysql-test/t/cte_nonrecursive.test index 5e1770496f6..98a77940c99 100644 --- a/mysql-test/t/cte_nonrecursive.test +++ b/mysql-test/t/cte_nonrecursive.test @@ -1012,3 +1012,21 @@ SELECT a FROM cte; WITH cte(a,b) AS (SELECT 4,5 UNION SELECT 4,3) SELECT a,b FROM cte; DROP TABLE t1; + +--echo # +--echo # MDEV-16353: unreferenced CTE specified by query with UNION +--echo # + +with cte as + (select 1 union select 2 union select 3) +select 1 as f; + +create table t1 (a int); +insert into t1 values (2), (1), (7), (1), (4); + +with cte as + (select * from t1 where a < 2 union select * from t1 where a > 5) +select 2 as f; + +drop table t1; + \ No newline at end of file diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 13c19dae342..178d7393878 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -625,7 +625,7 @@ bool st_select_lex_unit::prepare(THD *thd_arg, select_result *sel_result, { if (with_element) { - if (derived->with->rename_columns_of_derived_unit(thd, this)) + if (with_element->rename_columns_of_derived_unit(thd, this)) goto err; if (check_duplicate_names(thd, sl->item_list, 0)) goto err; From 40dc1a684694f1023c42f46b587d917a9988284a Mon Sep 17 00:00:00 2001 From: gvtek <34262684+gvtek@users.noreply.github.com> Date: Sat, 26 May 2018 16:42:13 -0700 Subject: [PATCH 3/5] Better Link Spacing Just spacing out the links on separate linkes, they were running together in a block of text and could be a little hard for some people to differentiate where one begins and the next ends. Seems silly, but just trying to help in the formatting a bit. --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index f46888e4b2f..00587110004 100644 --- a/README.md +++ b/README.md @@ -17,10 +17,15 @@ see the Credits appendix. You can also run 'SHOW authors' to get a list of active contributors. A description of the MariaDB project and a manual can be found at: + https://mariadb.org/ + https://mariadb.com/kb/en/ + https://mariadb.com/kb/en/mariadb-vs-mysql-features/ + https://mariadb.com/kb/en/mariadb-versus-mysql-features/ + https://mariadb.com/kb/en/mariadb-versus-mysql-compatibility/ As MariaDB is a full replacement of MySQL, the MySQL manual at From 5932a4e77de93880d06ac537a566a8a0f312d675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 4 Jun 2018 11:31:39 +0300 Subject: [PATCH 4/5] MDEV-13834: Upgrade failure from 10.1 innodb_encrypt_log log_crypt_101_read_block(): Mimic MariaDB 10.1, and use the first encryption key if the key for the checkpoint cannot be found. Redo log encryption key rotation was ultimately disabled in MDEV-9422 (MariaDB 10.1.13) due to design issues. So, from MariaDB 10.1.13 onwards only one log encryption key should matter. recv_log_format_0_recover(): Add the parameter 'bool crypt'. Indicate when the log cannot be decrypted for upgrade, instead of making a possibly false claim that the log requires crash recovery. init_crypt_key(): Remove extra space from a message. --- storage/innobase/log/log0crypt.cc | 11 ++++++++--- storage/innobase/log/log0recv.cc | 16 +++++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/storage/innobase/log/log0crypt.cc b/storage/innobase/log/log0crypt.cc index 0358057612d..478f021cbe4 100644 --- a/storage/innobase/log/log0crypt.cc +++ b/storage/innobase/log/log0crypt.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (C) 2013, 2015, Google Inc. All Rights Reserved. -Copyright (C) 2014, 2017, MariaDB Corporation. All Rights Reserved. +Copyright (C) 2014, 2018, 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 @@ -159,7 +159,7 @@ static bool init_crypt_key(crypt_info_t* info, bool upgrade = false) << "Obtaining redo log encryption key version " << info->key_version << " failed (" << rc << "). Maybe the key or the required encryption " - << " key management plugin was not found."; + "key management plugin was not found."; return false; } @@ -279,7 +279,12 @@ log_crypt_101_read_block(byte* buf) } } - return false; + if (infos_used == 0) { + return false; + } + /* MariaDB Server 10.1 would use the first key if it fails to + find a key for the current checkpoint. */ + info = infos; found: byte dst[OS_FILE_LOG_BLOCK_SIZE]; uint dst_len; diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc index 59f8595c2c4..b7ab1311cb7 100644 --- a/storage/innobase/log/log0recv.cc +++ b/storage/innobase/log/log0recv.cc @@ -871,12 +871,11 @@ recv_find_max_checkpoint_0(log_group_t** max_group, ulint* max_field) /** Determine if a pre-MySQL 5.7.9/MariaDB 10.2.2 redo log is clean. @param[in] lsn checkpoint LSN +@param[in] crypt whether the log might be encrypted @return error code @retval DB_SUCCESS if the redo log is clean @retval DB_ERROR if the redo log is corrupted or dirty */ -static -dberr_t -recv_log_format_0_recover(lsn_t lsn) +static dberr_t recv_log_format_0_recover(lsn_t lsn, bool crypt) { log_mutex_enter(); log_group_t* group = &log_sys->log; @@ -911,7 +910,13 @@ recv_log_format_0_recover(lsn_t lsn) } if (log_block_get_data_len(buf) - != (source_offset & (OS_FILE_LOG_BLOCK_SIZE - 1))) { + == (source_offset & (OS_FILE_LOG_BLOCK_SIZE - 1))) { + } else if (crypt) { + ib::error() << "Cannot decrypt log for upgrading." + " The encrypted log was created before MariaDB 10.2.2" + << NO_UPGRADE_RTFM_MSG; + return DB_ERROR; + } else { ib::error() << NO_UPGRADE_RECOVERY_MSG << NO_UPGRADE_RTFM_MSG; return(DB_ERROR); @@ -3259,7 +3264,8 @@ recv_recovery_from_checkpoint_start(lsn_t flush_lsn) switch (group->format) { case 0: log_mutex_exit(); - return(recv_log_format_0_recover(checkpoint_lsn)); + return recv_log_format_0_recover(checkpoint_lsn, + buf[20 + 32 * 9] == 2); default: if (end_lsn == 0) { break; From 8dc70c862b8ec115fd9a3c2b37c746ffc4f0d3cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 4 Jun 2018 15:55:37 +0300 Subject: [PATCH 5/5] MDEV-16376 ASAN: heap-use-after-free in gcol.innodb_virtual_debug After a failed ADD INDEX, dict_index_remove_from_cache_low() could iterate the index fields and dereference a freed virtual column object when trying to remove the index from the v_indexes of the virtual column. This regression was caused by a merge of MDEV-16119 InnoDB lock->index refers to a freed object. ha_innobase_inplace_ctx::clear_added_indexes(): Detach the indexes of uncommitted indexes from virtual columns, so that the iteration in dict_index_remove_from_cache_low() can be avoided. ha_innobase::prepare_inplace_alter_table(): Ignore uncommitted corrupted indexes when rejecting ALTER TABLE. (This minor bug was revealed by the extension of the test case.) dict_index_t::detach_columns(): Detach an index from virtual columns. Invoked by both dict_index_remove_from_cache_low() and ha_innobase_inplace_ctx::clear_added_indexes(). dict_col_t::detach(const dict_index_t& index): Detach an index from a column. dict_col_t::is_virtual(): Replaces dict_col_is_virtual(). dict_index_t::has_virtual(): Replaces dict_index_has_virtual(). --- .../suite/gcol/r/innodb_virtual_debug.result | 14 +++++-- .../suite/gcol/t/innodb_virtual_debug.test | 15 +++++-- storage/innobase/dict/dict0dict.cc | 32 +------------- storage/innobase/handler/handler0alter.cc | 14 ++++++- storage/innobase/include/dict0dict.h | 19 +++------ storage/innobase/include/dict0dict.ic | 24 ----------- storage/innobase/include/dict0mem.h | 42 +++++++++++++++++++ storage/innobase/row/row0upd.cc | 2 +- 8 files changed, 85 insertions(+), 77 deletions(-) diff --git a/mysql-test/suite/gcol/r/innodb_virtual_debug.result b/mysql-test/suite/gcol/r/innodb_virtual_debug.result index 7774c6c347c..50b714566d9 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_debug.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_debug.result @@ -64,11 +64,19 @@ INSERT INTO t VALUES (18, 1, DEFAULT, 'mm'); INSERT INTO t VALUES (28, 1, DEFAULT, 'mm'); INSERT INTO t VALUES (null, null, DEFAULT, 'mm'); CREATE INDEX idx_1 on t(c); -SET SESSION debug_dbug="+d,create_index_fail"; -ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x); +SET @saved_dbug = @@SESSION.debug_dbug; +SET debug_dbug = '+d,create_index_fail'; +ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x), +ADD INDEX idcx (c,x); ERROR 23000: Duplicate entry '' for key '*UNKNOWN*' -SET SESSION debug_dbug=""; +UPDATE t SET a=a+1; +affected rows: 3 +info: Rows matched: 4 Changed: 3 Warnings: 0 +ALTER TABLE t ADD INDEX idc(c); +ERROR 23000: Duplicate entry '' for key '*UNKNOWN*' +SET debug_dbug = @saved_dbug; affected rows: 0 +UPDATE t SET b=b-1; SHOW CREATE TABLE t; Table Create Table t CREATE TABLE `t` ( diff --git a/mysql-test/suite/gcol/t/innodb_virtual_debug.test b/mysql-test/suite/gcol/t/innodb_virtual_debug.test index 3870f84e066..ccdd16c9ebe 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_debug.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_debug.test @@ -119,14 +119,23 @@ INSERT INTO t VALUES (null, null, DEFAULT, 'mm'); CREATE INDEX idx_1 on t(c); -SET SESSION debug_dbug="+d,create_index_fail"; +SET @saved_dbug = @@SESSION.debug_dbug; +SET debug_dbug = '+d,create_index_fail'; --enable_info --error ER_DUP_ENTRY -ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x); -SET SESSION debug_dbug=""; +ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x), +ADD INDEX idcx (c,x); + +UPDATE t SET a=a+1; + +--error ER_DUP_ENTRY +ALTER TABLE t ADD INDEX idc(c); +SET debug_dbug = @saved_dbug; --disable_info +UPDATE t SET b=b-1; + SHOW CREATE TABLE t; SELECT c FROM t; diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index 9a7d9eef092..f34da118a31 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -2676,37 +2676,7 @@ dict_index_remove_from_cache_low( UT_LIST_REMOVE(table->indexes, index); /* Remove the index from affected virtual column index list */ - if (dict_index_has_virtual(index)) { - const dict_col_t* col; - const dict_v_col_t* vcol; - - for (ulint i = 0; i < dict_index_get_n_fields(index); i++) { - col = dict_index_get_nth_col(index, i); - if (dict_col_is_virtual(col)) { - vcol = reinterpret_cast( - col); - - /* This could be NULL, when we do add virtual - column, add index together. We do not need to - track this virtual column's index */ - if (vcol->v_indexes == NULL) { - continue; - } - - dict_v_idx_list::iterator it; - - for (it = vcol->v_indexes->begin(); - it != vcol->v_indexes->end(); ++it) { - dict_v_idx_t v_index = *it; - if (v_index.index == index) { - vcol->v_indexes->erase(it); - break; - } - } - } - - } - } + index->detach_columns(); dict_mem_index_free(index); } diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index bc88001edf6..517f20006af 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -245,6 +245,16 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx @return whether the table will be rebuilt */ bool need_rebuild () const { return(old_table != new_table); } + /** Clear uncommmitted added indexes after a failed operation. */ + void clear_added_indexes() + { + for (ulint i = 0; i < num_to_add_index; i++) { + if (!add_index[i]->is_committed()) { + add_index[i]->detach_columns(); + } + } + } + private: // Disable copying ha_innobase_inplace_ctx(const ha_innobase_inplace_ctx&); @@ -5996,7 +6006,8 @@ check_if_can_drop_indexes: for (dict_index_t* index = dict_table_get_first_index(indexed_table); index != NULL; index = dict_table_get_next_index(index)) { - if (!index->to_be_dropped && index->is_corrupted()) { + if (!index->to_be_dropped && index->is_committed() + && index->is_corrupted()) { my_error(ER_INDEX_CORRUPT, MYF(0), index->name()); goto err_exit; } @@ -6567,6 +6578,7 @@ oom: that we hold at most a shared lock on the table. */ m_prebuilt->trx->error_info = NULL; ctx->trx->error_state = DB_SUCCESS; + ctx->clear_added_indexes(); DBUG_RETURN(true); } diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h index 51ce248b98d..9822b57d190 100644 --- a/storage/innobase/include/dict0dict.h +++ b/storage/innobase/include/dict0dict.h @@ -750,13 +750,9 @@ dict_index_is_spatial( /*==================*/ const dict_index_t* index) /*!< in: index */ MY_ATTRIBUTE((warn_unused_result)); -/** Check whether the index contains a virtual column. -@param[in] index index -@return nonzero for index on virtual column, zero for other indexes */ -UNIV_INLINE -ulint -dict_index_has_virtual( - const dict_index_t* index); + +#define dict_index_has_virtual(index) (index)->has_virtual() + /********************************************************************//** Check whether the index is the insert buffer tree. @return nonzero for insert buffer, zero for other indexes */ @@ -1964,13 +1960,8 @@ dict_index_node_ptr_max_size( /*=========================*/ const dict_index_t* index) /*!< in: index */ MY_ATTRIBUTE((warn_unused_result)); -/** Check if a column is a virtual column -@param[in] col column -@return true if it is a virtual column, false otherwise */ -UNIV_INLINE -bool -dict_col_is_virtual( - const dict_col_t* col); + +#define dict_col_is_virtual(col) (col)->is_virtual() /** encode number of columns and number of virtual columns in one 4 bytes value. We could do this because the number of columns in diff --git a/storage/innobase/include/dict0dict.ic b/storage/innobase/include/dict0dict.ic index e20da7c708a..ca2ea769612 100644 --- a/storage/innobase/include/dict0dict.ic +++ b/storage/innobase/include/dict0dict.ic @@ -72,16 +72,6 @@ dict_col_copy_type( type->mbminlen = col->mbminlen; type->mbmaxlen = col->mbmaxlen; } -/** Check if a column is a virtual column -@param[in] col column -@return true if it is a virtual column, false otherwise */ -UNIV_INLINE -bool -dict_col_is_virtual( - const dict_col_t* col) -{ - return(col->prtype & DATA_VIRTUAL); -} #ifdef UNIV_DEBUG /*********************************************************************//** @@ -325,20 +315,6 @@ dict_index_is_spatial( return(index->type & DICT_SPATIAL); } -/** Check whether the index contains a virtual column -@param[in] index index -@return nonzero for the index has virtual column, zero for other indexes */ -UNIV_INLINE -ulint -dict_index_has_virtual( - const dict_index_t* index) -{ - ut_ad(index); - ut_ad(index->magic_n == DICT_INDEX_MAGIC_N); - - return(index->type & DICT_VIRTUAL); -} - /********************************************************************//** Check whether the index is the insert buffer tree. @return nonzero for insert buffer, zero for other indexes */ diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 18e600c25d3..a18d6c1abdd 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -612,6 +612,13 @@ struct dict_col_t{ unsigned max_prefix:12; /*!< maximum index prefix length on this column. Our current max limit is 3072 for Barracuda table */ + + /** @return whether this is a virtual column */ + bool is_virtual() const { return prtype & DATA_VIRTUAL; } + + /** Detach the column from an index. + @param[in] index index to be detached from */ + inline void detach(const dict_index_t& index); }; /** Index information put in a list of virtual column structure. Index @@ -981,10 +988,45 @@ struct dict_index_t{ return DICT_CLUSTERED == (type & (DICT_CLUSTERED | DICT_IBUF)); } + /** @return whether the index includes virtual columns */ + bool has_virtual() const { return type & DICT_VIRTUAL; } + /** @return whether the index is corrupted */ inline bool is_corrupted() const; + + /** Detach the columns from the index that is to be freed. */ + void detach_columns() + { + if (has_virtual()) { + for (unsigned i = 0; i < n_fields; i++) { + fields[i].col->detach(*this); + } + + n_fields = 0; + } + } }; +/** Detach a column from an index. +@param[in] index index to be detached from */ +inline void dict_col_t::detach(const dict_index_t& index) +{ + if (!is_virtual()) { + return; + } + + if (dict_v_idx_list* v_indexes = reinterpret_cast + (this)->v_indexes) { + for (dict_v_idx_list::iterator i = v_indexes->begin(); + i != v_indexes->end(); i++) { + if (i->index == &index) { + v_indexes->erase(i); + return; + } + } + } +} + /** The status of online index creation */ enum online_index_status { /** the index is complete and ready for access */ diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc index 946bbae9534..18a6a26f3ce 100644 --- a/storage/innobase/row/row0upd.cc +++ b/storage/innobase/row/row0upd.cc @@ -593,7 +593,7 @@ row_upd_changes_field_size_or_external( /* We should ignore virtual field if the index is not a virtual index */ if (upd_fld_is_virtual_col(upd_field) - && dict_index_has_virtual(index) != DICT_VIRTUAL) { + && !index->has_virtual()) { continue; }