Merge 10.2 into 10.3

mysql-test/suite/maria/lock.result

@@ -109,6 +109,39 @@ ALTER TABLE t1 ADD UNIQUE KEY (f1);
 ERROR 23000: Duplicate entry 'foo' for key 'f1'
 ALTER TABLE t1 ADD KEY (f2);
 DROP TABLE t1;
+#
+# MDEV-10748 Server crashes in ha_maria::implicit_commit upon ALTER TABLE
+#
+CREATE TABLE t1 (a INT, b INT) ENGINE=Aria;
+SELECT * FROM t1;
+a	b
+CREATE TABLE t2 (c INT) ENGINE=Aria;
+LOCK TABLE t2 READ, t1 WRITE;
+ALTER TABLE t1 CHANGE b a INT;
+ERROR 42S21: Duplicate column name 'a'
+UNLOCK TABLES;
+DROP TABLE t1, t2;
+#
+# MDEV-10748 Server crashes in ha_maria::implicit_commit upon ALTER TABLE
+#
+CREATE TABLE t1 (a INT) ENGINE=Aria;
+CREATE TABLE t2 (b INT) ENGINE=Aria;
+LOCK TABLES t1 WRITE, t2 AS t2a WRITE, t2 WRITE;
+ALTER TABLE t2 CHANGE b c VARBINARY(30000), ALGORITHM=COPY;
+UNLOCK TABLES;
+DROP TABLE t1, t2;
+# More complex test, from RQG
+CREATE TABLE t1 (a INT) ENGINE=Aria;
+CREATE TABLE t2 (b INT) ENGINE=Aria;
+CREATE OR REPLACE VIEW v2 AS SELECT * FROM t2 ;
+LOCK TABLES t1 WRITE, t2 AS t2a WRITE, v2 WRITE CONCURRENT, t2 WRITE;
+ALTER TABLE t1 FORCE;
+ALTER TABLE t2 CHANGE b c VARBINARY(30000), ALGORITHM=INPLACE;
+ERROR 0A000: ALGORITHM=INPLACE is not supported for this operation. Try ALGORITHM=COPY
+ALTER TABLE t2 CHANGE b c VARBINARY(30000), ALGORITHM=COPY;
+UNLOCK TABLES;
+DROP VIEW v2;
+DROP TABLE t1, t2;
 # End of 10.2 tests
 #
 # MDEV-14669 Assertion `file->trn == trn' failed in ha_maria::start_stmt

mysql-test/suite/maria/lock.test

@@ -118,6 +118,52 @@ ALTER TABLE t1 ADD UNIQUE KEY (f1);
 ALTER TABLE t1 ADD KEY (f2);
 DROP TABLE t1;
 
+
+--echo #
+--echo # MDEV-10748 Server crashes in ha_maria::implicit_commit upon ALTER TABLE
+--echo #
+
+CREATE TABLE t1 (a INT, b INT) ENGINE=Aria;
+SELECT * FROM t1;
+CREATE TABLE t2 (c INT) ENGINE=Aria;
+ 
+LOCK TABLE t2 READ, t1 WRITE;
+--error ER_DUP_FIELDNAME
+ALTER TABLE t1 CHANGE b a INT;
+ 
+# Cleanup
+UNLOCK TABLES;
+DROP TABLE t1, t2;
+
+--echo #
+--echo # MDEV-10748 Server crashes in ha_maria::implicit_commit upon ALTER TABLE
+--echo #
+
+CREATE TABLE t1 (a INT) ENGINE=Aria;
+CREATE TABLE t2 (b INT) ENGINE=Aria;
+
+LOCK TABLES t1 WRITE, t2 AS t2a WRITE, t2 WRITE;
+ALTER TABLE t2 CHANGE b c VARBINARY(30000), ALGORITHM=COPY;
+UNLOCK TABLES;
+DROP TABLE t1, t2;
+
+--echo # More complex test, from RQG
+
+CREATE TABLE t1 (a INT) ENGINE=Aria;
+CREATE TABLE t2 (b INT) ENGINE=Aria;
+CREATE OR REPLACE VIEW v2 AS SELECT * FROM t2 ;
+
+LOCK TABLES t1 WRITE, t2 AS t2a WRITE, v2 WRITE CONCURRENT, t2 WRITE;
+
+ALTER TABLE t1 FORCE;
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+ALTER TABLE t2 CHANGE b c VARBINARY(30000), ALGORITHM=INPLACE;
+ALTER TABLE t2 CHANGE b c VARBINARY(30000), ALGORITHM=COPY;
+
+UNLOCK TABLES;
+DROP VIEW v2;
+DROP TABLE t1, t2;
+
 --echo # End of 10.2 tests
 
 --echo #

sql/keycaches.cc

@@ -68,7 +68,7 @@ uchar* find_named(I_List<NAMED_ILINK> *list, const char *name, size_t length,
 }
 
 
-bool NAMED_ILIST::delete_element(const char *name, size_t length, void (*free_element)(const char *name, uchar*))
+bool NAMED_ILIST::delete_element(const char *name, size_t length, void (*free_element)(const char *name, void*))
 {
   I_List_iterator<NAMED_ILINK> it(*this);
   NAMED_ILINK *element;
@@ -85,7 +85,7 @@ bool NAMED_ILIST::delete_element(const char *name, size_t length, void (*free_el
   DBUG_RETURN(1);
 }
 
-void NAMED_ILIST::delete_elements(void (*free_element)(const char *name, uchar*))
+void NAMED_ILIST::delete_elements(void (*free_element)(const char *name, void*))
 {
   NAMED_ILINK *element;
   DBUG_ENTER("NAMED_ILIST::delete_elements");
@@ -157,9 +157,9 @@ KEY_CACHE *get_or_create_key_cache(const char *name, size_t length)
 }
 
 
-void free_key_cache(const char *name, KEY_CACHE *key_cache)
+void free_key_cache(const char *name, void *key_cache)
 {
-  end_key_cache(key_cache, 1);		// Can never fail
+  end_key_cache(static_cast<KEY_CACHE *>(key_cache), 1); // Can never fail
   my_free(key_cache);
 }
 
@@ -221,13 +221,12 @@ Rpl_filter *get_or_create_rpl_filter(const char *name, size_t length)
   return filter;
 }
 
-void free_rpl_filter(const char *name, Rpl_filter *filter)
+void free_rpl_filter(const char *name, void *filter)
 {
-  delete filter;
-  filter= 0;
+  delete static_cast<Rpl_filter*>(filter);
 }
 
 void free_all_rpl_filters()
 {
-  rpl_filters.delete_elements((void (*)(const char*, uchar*)) free_rpl_filter);
+  rpl_filters.delete_elements(free_rpl_filter);
 }

sql/keycaches.h

@@ -30,8 +30,8 @@ class NAMED_ILINK;
 class NAMED_ILIST: public I_List<NAMED_ILINK>
 {
   public:
-  void delete_elements(void (*free_element)(const char*, uchar*));
-  bool delete_element(const char *name, size_t length, void (*free_element)(const char*, uchar*));
+  void delete_elements(void (*free_element)(const char*, void*));
+  bool delete_element(const char *name, size_t length, void (*free_element)(const char*, void*));
 };
 
 /* For key cache */
@@ -42,7 +42,7 @@ extern NAMED_ILIST key_caches;
 KEY_CACHE *create_key_cache(const char *name, size_t length);
 KEY_CACHE *get_key_cache(const LEX_CSTRING *cache_name);
 KEY_CACHE *get_or_create_key_cache(const char *name, size_t length);
-void free_key_cache(const char *name, KEY_CACHE *key_cache);
+void free_key_cache(const char *name, void *key_cache);
 bool process_key_caches(process_key_cache_t func, void *param);
 
 /* For Rpl_filter */
@@ -52,7 +52,6 @@ extern NAMED_ILIST rpl_filters;
 Rpl_filter *create_rpl_filter(const char *name, size_t length);
 Rpl_filter *get_rpl_filter(LEX_CSTRING *filter_name);
 Rpl_filter *get_or_create_rpl_filter(const char *name, size_t length);
-void free_rpl_filter(const char *name, Rpl_filter *filter);
 void free_all_rpl_filters(void);
 
 #endif /* KEYCACHES_INCLUDED */

sql/mysqld.cc

@@ -2254,7 +2254,7 @@ void clean_up(bool print_message)
   tdc_deinit();
   mdl_destroy();
   dflt_key_cache= 0;
-  key_caches.delete_elements((void (*)(const char*, uchar*)) free_key_cache);
+  key_caches.delete_elements(free_key_cache);
   wt_end();
   multi_keycache_free();
   sp_cache_end();

sql/sql_base.cc

@@ -2494,6 +2494,7 @@ Locked_tables_list::reopen_tables(THD *thd, bool need_reopen)
         {
           thd->locked_tables_list.unlink_from_list(thd, table_list, false);
           mysql_lock_remove(thd, thd->lock, *prev);
+          (*prev)->file->extra(HA_EXTRA_PREPARE_FOR_FORCED_CLOSE);
           close_thread_table(thd, prev);
           break;
         }

storage/innobase/btr/btr0bulk.cc

 /*****************************************************************************
 
-Copyright (c) 2014, 2016, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 2014, 2019, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2017, 2019, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
@@ -51,7 +51,7 @@ PageBulk::init()
 	m_heap = mem_heap_create(1000);
 
 	m_mtr.start();
-	mtr_x_lock(&m_index->lock, &m_mtr);
+
 	if (m_flush_observer) {
 		m_mtr.set_log_mode(MTR_LOG_NO_REDO);
 		m_mtr.set_flush_observer(m_flush_observer);
@@ -609,22 +609,20 @@ PageBulk::storeExt(
 	btr_pcur.pos_state = BTR_PCUR_IS_POSITIONED;
 	btr_pcur.latch_mode = BTR_MODIFY_LEAF;
 	btr_pcur.btr_cur.index = m_index;
-
-	page_cur_t*	page_cur = &btr_pcur.btr_cur.page_cur;
-	page_cur->index = m_index;
-	page_cur->rec = m_cur_rec;
-	page_cur->offsets = offsets;
-	page_cur->block = m_block;
+	btr_pcur.btr_cur.page_cur.index = m_index;
+	btr_pcur.btr_cur.page_cur.rec = m_cur_rec;
+	btr_pcur.btr_cur.page_cur.offsets = offsets;
+	btr_pcur.btr_cur.page_cur.block = m_block;
 
 	dberr_t	err = btr_store_big_rec_extern_fields(
 		&btr_pcur, offsets, big_rec, &m_mtr, BTR_STORE_INSERT_BULK);
 
-	ut_ad(page_offset(m_cur_rec) == page_offset(page_cur->rec));
-
 	/* Reset m_block and m_cur_rec from page cursor, because
-	block may be changed during blob insert. */
-	m_block = page_cur->block;
-	m_cur_rec = page_cur->rec;
+	block may be changed during blob insert. (FIXME: Can it really?) */
+	ut_ad(m_block == btr_pcur.btr_cur.page_cur.block);
+
+	m_block = btr_pcur.btr_cur.page_cur.block;
+	m_cur_rec = btr_pcur.btr_cur.page_cur.rec;
 	m_page = buf_block_get_frame(m_block);
 
 	return(err);
@@ -651,7 +649,7 @@ dberr_t
 PageBulk::latch()
 {
 	m_mtr.start();
-	mtr_x_lock(&m_index->lock, &m_mtr);
+
 	if (m_flush_observer) {
 		m_mtr.set_log_mode(MTR_LOG_NO_REDO);
 		m_mtr.set_flush_observer(m_flush_observer);
@@ -756,6 +754,10 @@ BtrBulk::pageCommit(
 		page_bulk->setNext(FIL_NULL);
 	}
 
+	ut_ad(!rw_lock_own_flagged(&m_index->lock,
+				   RW_LOCK_FLAG_X | RW_LOCK_FLAG_SX
+				   | RW_LOCK_FLAG_S));
+
 	/* Compress page if it's a compressed table. */
 	if (page_bulk->getPageZip() != NULL && !page_bulk->compress()) {
 		return(pageSplit(page_bulk, next_page_bulk));

storage/innobase/btr/btr0cur.cc

@@ -7206,7 +7206,7 @@ struct btr_blob_log_check_t {
 		ulint		page_no = ULINT_UNDEFINED;
 		FlushObserver*	observer = m_mtr->get_flush_observer();
 
-		if (m_op == BTR_STORE_INSERT_BULK) {
+		if (UNIV_UNLIKELY(m_op == BTR_STORE_INSERT_BULK)) {
 			offs = page_offset(*m_rec);
 			page_no = page_get_page_no(
 				buf_block_get_frame(*m_block));
@@ -7229,8 +7229,7 @@ struct btr_blob_log_check_t {
 		index->set_modified(*m_mtr);
 		m_mtr->set_flush_observer(observer);
 
-		if (m_op == BTR_STORE_INSERT_BULK) {
-			mtr_x_lock(dict_index_get_lock(index), m_mtr);
+		if (UNIV_UNLIKELY(m_op == BTR_STORE_INSERT_BULK)) {
 			m_pcur->btr_cur.page_cur.block = btr_block_get(
 				page_id_t(index->table->space_id, page_no),
 				page_size_t(index->table->space->flags),
@@ -7259,9 +7258,10 @@ struct btr_blob_log_check_t {
 		      *m_rec,
 		      MTR_MEMO_PAGE_X_FIX | MTR_MEMO_PAGE_SX_FIX));
 
-		ut_ad(mtr_memo_contains_flagged(m_mtr,
-		      dict_index_get_lock(index),
-		      MTR_MEMO_SX_LOCK | MTR_MEMO_X_LOCK));
+		ut_ad((m_op == BTR_STORE_INSERT_BULK)
+		      == !mtr_memo_contains_flagged(m_mtr, &index->lock,
+						    MTR_MEMO_SX_LOCK
+						    | MTR_MEMO_X_LOCK));
 	}
 };
 
@@ -7315,8 +7315,10 @@ btr_store_big_rec_extern_fields(
 
 	ut_ad(rec_offs_validate(rec, index, offsets));
 	ut_ad(rec_offs_any_extern(offsets));
-	ut_ad(mtr_memo_contains_flagged(btr_mtr, dict_index_get_lock(index),
-					MTR_MEMO_X_LOCK | MTR_MEMO_SX_LOCK));
+	ut_ad(op == BTR_STORE_INSERT_BULK
+	      || mtr_memo_contains_flagged(btr_mtr, &index->lock,
+					   MTR_MEMO_X_LOCK
+					   | MTR_MEMO_SX_LOCK));
 	ut_ad(mtr_memo_contains(btr_mtr, rec_block, MTR_MEMO_PAGE_X_FIX));
 	ut_ad(buf_block_get_frame(rec_block) == page_align(rec));
 	ut_a(dict_index_is_clust(index));
@@ -7441,7 +7443,7 @@ btr_store_big_rec_extern_fields(
 
 			mtr_t	*alloc_mtr;
 
-			if (op == BTR_STORE_INSERT_BULK) {
+			if (UNIV_UNLIKELY(op == BTR_STORE_INSERT_BULK)) {
 				mtr_bulk.start();
 				mtr_bulk.set_spaces(mtr);
 				alloc_mtr = &mtr_bulk;
@@ -7464,7 +7466,7 @@ btr_store_big_rec_extern_fields(
 
 			index->table->space->release_free_extents(r_extents);
 
-			if (op == BTR_STORE_INSERT_BULK) {
+			if (UNIV_UNLIKELY(op == BTR_STORE_INSERT_BULK)) {
 				mtr_bulk.commit();
 			}
 
@@ -7620,7 +7622,7 @@ btr_store_big_rec_extern_fields(
 				}
 
 				/* We compress a page when finish bulk insert.*/
-				if (op != BTR_STORE_INSERT_BULK) {
+				if (UNIV_LIKELY(op != BTR_STORE_INSERT_BULK)) {
 					page_zip_write_blob_ptr(
 						page_zip, rec, index, offsets,
 						field_no, &mtr);

storage/innobase/fil/fil0crypt.cc

@@ -1766,8 +1766,6 @@ fil_crypt_rotate_page(
 		return;
 	}
 
-	ut_d(const bool was_free = fseg_page_is_free(space, (uint32_t)offset));
-
 	mtr_t mtr;
 	mtr.start();
 	if (buf_block_t* block = fil_crypt_get_page_throttle(state,
@@ -1782,9 +1780,9 @@ fil_crypt_rotate_page(
 		if (space->is_stopping()) {
 			/* The tablespace is closing (in DROP TABLE or
 			TRUNCATE TABLE or similar): avoid further access */
-		} else if (!*reinterpret_cast<uint32_t*>(FIL_PAGE_OFFSET
-							 + frame)) {
-			/* It looks like this page was never
+		} else if (!kv && !*reinterpret_cast<uint16_t*>
+			   (&frame[FIL_PAGE_TYPE])) {
+			/* It looks like this page is not
 			allocated. Because key rotation is accessing
 			pages in a pattern that is unlike the normal
 			B-tree and undo log access pattern, we cannot
@@ -1794,9 +1792,20 @@ fil_crypt_rotate_page(
 			tablespace latch before acquiring block->lock,
 			then the fseg_page_is_free() information
 			could be stale already. */
-			ut_ad(was_free);
-			ut_ad(kv == 0);
-			ut_ad(page_get_space_id(frame) == 0);
+
+			/* If the data file was originally created
+			before MariaDB 10.0 or MySQL 5.6, some
+			allocated data pages could carry 0 in
+			FIL_PAGE_TYPE. The FIL_PAGE_TYPE on those
+			pages will be updated in
+			buf_flush_init_for_writing() when the page
+			is modified the next time.
+
+			Also, when the doublewrite buffer pages are
+			allocated on bootstrap in a non-debug build,
+			some dummy pages will be allocated, with 0 in
+			the FIL_PAGE_TYPE. Those pages should be
+			skipped from key rotation forever. */
 		} else if (fil_crypt_needs_rotation(
 				crypt_data,
 				kv,

storage/innobase/row/row0uins.cc

@@ -267,10 +267,8 @@ row_undo_ins_remove_sec_low(
 	que_thr_t*	thr)	/*!< in: query thread */
 {
 	btr_pcur_t		pcur;
-	btr_cur_t*		btr_cur;
 	dberr_t			err	= DB_SUCCESS;
 	mtr_t			mtr;
-	enum row_search_result	search_result;
 	const bool		modify_leaf = mode == BTR_MODIFY_LEAF;
 
 	row_mtr_start(&mtr, index, !modify_leaf);
@@ -295,12 +293,15 @@ row_undo_ins_remove_sec_low(
 		mode |= BTR_RTREE_UNDO_INS;
 	}
 
-	search_result = row_search_index_entry(index, entry, mode,
-					       &pcur, &mtr);
-
-	switch (search_result) {
+	switch (row_search_index_entry(index, entry, mode, &pcur, &mtr)) {
+	case ROW_BUFFERED:
+	case ROW_NOT_DELETED_REF:
+		/* These are invalid outcomes, because the mode passed
+		to row_search_index_entry() did not include any of the
+		flags BTR_INSERT, BTR_DELETE, or BTR_DELETE_MARK. */
+		ut_error;
 	case ROW_NOT_FOUND:
-		goto func_exit;
+		break;
 	case ROW_FOUND:
 		if (dict_index_is_spatial(index)
 		    && rec_get_deleted_flag(
@@ -310,31 +311,22 @@ row_undo_ins_remove_sec_low(
 				<< " is deleted marked on insert rollback.";
 			ut_ad(0);
 		}
-		break;
 
-	case ROW_BUFFERED:
-	case ROW_NOT_DELETED_REF:
-		/* These are invalid outcomes, because the mode passed
-		to row_search_index_entry() did not include any of the
-		flags BTR_INSERT, BTR_DELETE, or BTR_DELETE_MARK. */
-		ut_error;
-	}
-
-	btr_cur = btr_pcur_get_btr_cur(&pcur);
+		btr_cur_t* btr_cur = btr_pcur_get_btr_cur(&pcur);
 
-	if (modify_leaf) {
-		err = btr_cur_optimistic_delete(btr_cur, 0, &mtr)
-			? DB_SUCCESS : DB_FAIL;
-	} else {
-		/* Passing rollback=false here, because we are
-		deleting a secondary index record: the distinction
-		only matters when deleting a record that contains
-		externally stored columns. */
-		ut_ad(!dict_index_is_clust(index));
-		btr_cur_pessimistic_delete(&err, FALSE, btr_cur, 0,
-					   false, &mtr);
+		if (modify_leaf) {
+			err = btr_cur_optimistic_delete(btr_cur, 0, &mtr)
+				? DB_SUCCESS : DB_FAIL;
+		} else {
+			/* Passing rollback=false here, because we are
+			deleting a secondary index record: the distinction
+			only matters when deleting a record that contains
+			externally stored columns. */
+			btr_cur_pessimistic_delete(&err, FALSE, btr_cur, 0,
+						   false, &mtr);
+		}
 	}
-func_exit:
+
 	btr_pcur_close(&pcur);
 func_exit_no_pcur:
 	mtr_commit(&mtr);

storage/maria/ma_state.c

@@ -455,7 +455,7 @@ my_bool _ma_trnman_end_trans_hook(TRN *trn, my_bool commit,
   MARIA_USED_TABLES *tables, *next;
   DBUG_ENTER("_ma_trnman_end_trans_hook");
   DBUG_PRINT("enter", ("trn: %p  used_tables: %p", trn, trn->used_tables));
-  
+
   for (tables= (MARIA_USED_TABLES*) trn->used_tables;
        tables;
        tables= next)
@@ -572,6 +572,7 @@ void _ma_remove_table_from_trnman(MARIA_HA *info)
   TRN *trn= info->trn;
   MARIA_USED_TABLES *tables, **prev;
   MARIA_HA *handler, **prev_file;
+  uint unlinked= 0;
   DBUG_ENTER("_ma_remove_table_from_trnman");
   DBUG_PRINT("enter", ("trn: %p  used_tables: %p  share: %p  in_trans: %d",
                        trn, trn->used_tables, share, share->in_trans));
@@ -580,7 +581,7 @@ void _ma_remove_table_from_trnman(MARIA_HA *info)
 
   if (trn == &dummy_transaction_object)
     DBUG_VOID_RETURN;
-  
+
   /* First remove share from used_tables */
   for (prev= (MARIA_USED_TABLES**) (char*) &trn->used_tables;
        (tables= *prev);
@@ -594,7 +595,7 @@ void _ma_remove_table_from_trnman(MARIA_HA *info)
       break;
     }
   }
-  if (tables != 0)
+  if (!tables)
   {
     /*
       This can only happens in case of rename of intermediate table as
@@ -603,18 +604,21 @@ void _ma_remove_table_from_trnman(MARIA_HA *info)
     DBUG_PRINT("warning", ("share: %p where not in used_tables_list", share));
   }
 
-  /* unlink table from used_instances */
-  for (prev_file= (MARIA_HA**) &trn->used_instances;
-       (handler= *prev_file);
-       prev_file= &handler->trn_next)
+  /* unlink all instances of the table from used_instances */
+  prev_file= (MARIA_HA**) &trn->used_instances;
+  while ((handler= *prev_file))
   {
-    if (handler == info)
+    if (handler->s == share)
     {
-      *prev_file= info->trn_next;
-      break;
+      unlinked++;
+      *prev_file= handler->trn_next;  /* Remove instance */
     }
+    else
+      prev_file= &handler->trn_next;  /* Continue with next instance */
   }
-  if (handler != 0)
+
+  DBUG_PRINT("note", ("unlinked tables: %u", unlinked));
+  if (!unlinked)
   {
     /*
       This can only happens in case of rename of intermediate table as