MDEV-28465 Some calls to btr_pcur_close() are unnecessary

The function btr_pcur_close() is being invoked on local variables
even when no cleanup needs to be done. In particular, for B-tree
indexes (not SPATIAL INDEX), unless btr_pcur_store_position()
was invoked in the past, there is no need to invoke btr_pcur_close().

On purge and rollback, we will retain btr_pcur_close(&pcur)
because otherwise some ./mtr --suite=innodb_gis tests would leak memory.

storage/innobase/dict/dict0load.cc

@@ -1315,7 +1315,7 @@ dict_load_columns(
 
 	ut_ad(dict_sys.locked());
 
-	mtr_start(&mtr);
+	mtr.start();
 
 	dict_index_t* sys_index = dict_sys.sys_columns->indexes.start;
 	ut_ad(!dict_sys.sys_columns->not_redundant());
@@ -1404,8 +1404,7 @@ dict_load_columns(
 		btr_pcur_move_to_next_user_rec(&pcur, &mtr);
 	}
 
-	btr_pcur_close(&pcur);
-	mtr_commit(&mtr);
+	mtr.commit();
 }
 
 /** Loads SYS_VIRTUAL info for one virtual column
@@ -1437,7 +1436,7 @@ dict_load_virtual_one_col(
 		return;
 	}
 
-	mtr_start(&mtr);
+	mtr.start();
 
 	sys_virtual_index = dict_sys.sys_virtual->indexes.start;
 	ut_ad(!dict_sys.sys_virtual->not_redundant());
@@ -1495,8 +1494,7 @@ dict_load_virtual_one_col(
 		btr_pcur_move_to_next_user_rec(&pcur, &mtr);
 	}
 
-	btr_pcur_close(&pcur);
-	mtr_commit(&mtr);
+	mtr.commit();
 }
 
 /** Loads info from SYS_VIRTUAL for virtual columns.
@@ -1682,7 +1680,7 @@ dict_load_fields(
 
 	ut_ad(dict_sys.locked());
 
-	mtr_start(&mtr);
+	mtr.start();
 
 	dict_index_t* sys_index = dict_sys.sys_fields->indexes.start;
 	ut_ad(!dict_sys.sys_fields->not_redundant());
@@ -1730,9 +1728,8 @@ dict_load_fields(
 
 	error = DB_SUCCESS;
 func_exit:
-	btr_pcur_close(&pcur);
-	mtr_commit(&mtr);
-	return(error);
+	mtr.commit();
+	return error;
 }
 
 /** Error message for a delete-marked record in dict_load_index_low() */
@@ -1936,7 +1933,7 @@ dict_load_indexes(
 
 	ut_ad(dict_sys.locked());
 
-	mtr_start(&mtr);
+	mtr.start();
 
 	sys_index = dict_sys.sys_indexes->indexes.start;
 	ut_ad(!dict_sys.sys_indexes->not_redundant());
@@ -2140,10 +2137,8 @@ dict_load_indexes(
 	}
 
 func_exit:
-	btr_pcur_close(&pcur);
-	mtr_commit(&mtr);
-
-	return(error);
+	mtr.commit();
+	return error;
 }
 
 /** Load a table definition from a SYS_TABLES record to dict_table_t.
@@ -2320,7 +2315,7 @@ static dict_table_t *dict_load_table_one(const span<const char> &name,
 
 	heap = mem_heap_create(32000);
 
-	mtr_start(&mtr);
+	mtr.start();
 
 	dict_index_t *sys_index = dict_sys.sys_tables->indexes.start;
 	ut_ad(!dict_sys.sys_tables->not_redundant());
@@ -2348,8 +2343,7 @@ static dict_table_t *dict_load_table_one(const span<const char> &name,
 	if (!btr_pcur_is_on_user_rec(&pcur)) {
 		/* Not found */
 err_exit:
-		btr_pcur_close(&pcur);
-		mtr_commit(&mtr);
+		mtr.commit();
 		mem_heap_free(heap);
 
 		DBUG_RETURN(NULL);
@@ -2372,8 +2366,7 @@ static dict_table_t *dict_load_table_one(const span<const char> &name,
 		goto err_exit;
 	}
 
-	btr_pcur_close(&pcur);
-	mtr_commit(&mtr);
+	mtr.commit();
 
 	dict_load_tablespace(table, ignore_err);
 
@@ -2618,10 +2611,8 @@ dict_load_table_on_id(
 		}
 	}
 
-	btr_pcur_close(&pcur);
 	mtr.commit();
-
-	return(table);
+	return table;
 }
 
 /********************************************************************//**
@@ -2678,7 +2669,7 @@ static void dict_load_foreign_cols(dict_foreign_t *foreign, trx_id_t trx_id)
 		mem_heap_alloc(foreign->heap,
 			       foreign->n_fields * sizeof(void*)));
 
-	mtr_start(&mtr);
+	mtr.start();
 
 	dict_index_t* sys_index = dict_sys.sys_foreign_cols->indexes.start;
 	ut_ad(!dict_sys.sys_foreign_cols->not_redundant());
@@ -2789,7 +2780,6 @@ static void dict_load_foreign_cols(dict_foreign_t *foreign, trx_id_t trx_id)
 		btr_pcur_move_to_next_user_rec(&pcur, &mtr);
 	}
 
-	btr_pcur_close(&pcur);
 	mtr.commit();
 	if (UNIV_LIKELY_NULL(heap)) {
 		mem_heap_free(heap);
@@ -2862,7 +2852,6 @@ dict_load_foreign(
 
 	if (!btr_pcur_is_on_user_rec(&pcur)) {
 	not_found:
-		btr_pcur_close(&pcur);
 		mtr.commit();
 		if (UNIV_LIKELY_NULL(heap)) {
 			mem_heap_free(heap);
@@ -2950,7 +2939,6 @@ dict_load_foreign(
 		foreign->heap, (const char*) field, len);
 	dict_mem_referenced_table_name_lookup_set(foreign, TRUE);
 
-	btr_pcur_close(&pcur);
 	mtr.commit();
 	if (UNIV_LIKELY_NULL(heap)) {
 		mem_heap_free(heap);
@@ -3110,7 +3098,7 @@ dict_load_foreigns(
 		rec, DICT_FLD__SYS_FOREIGN_FOR_NAME__ID, &len);
 
 	/* Copy the string because the page may be modified or evicted
-	after mtr_commit() below. */
+	after mtr.commit() below. */
 	char	fk_id[MAX_TABLE_NAME_LEN + 1];
 
 	ut_a(len <= MAX_TABLE_NAME_LEN);
@@ -3137,7 +3125,7 @@ dict_load_foreigns(
 				"SYS_FOREIGN", int(len), fk_id);
 		/* fall through */
 	default:
-		btr_pcur_close(&pcur);
+		ut_free(pcur.old_rec_buf);
 		DBUG_RETURN(err);
 	}
 
@@ -3149,8 +3137,8 @@ dict_load_foreigns(
 	goto loop;
 
 load_next_index:
-	btr_pcur_close(&pcur);
-	mtr_commit(&mtr);
+	mtr.commit();
+	ut_free(pcur.old_rec_buf);
 
 	sec_index = dict_table_get_next_index(sec_index);
 

storage/innobase/dict/dict0stats.cc

@@ -1884,11 +1884,8 @@ dict_stats_analyze_index_level(
 	}
 #endif /* UNIV_STATS_DEBUG */
 
-	/* Release the latch on the last page, because that is not done by
-	btr_pcur_close(). This function works also for non-leaf pages. */
 	btr_leaf_page_release(btr_pcur_get_block(&pcur), BTR_SEARCH_LEAF, mtr);
 
-	btr_pcur_close(&pcur);
 	ut_free(prev_rec_buf);
 	mem_heap_free(heap);
 }
@@ -2394,8 +2391,6 @@ dict_stats_analyze_index_for_n_prefix(
 
 		n_diff_data->n_external_pages_sum += n_external_pages;
 	}
-
-	btr_pcur_close(&pcur);
 }
 
 /** statistics for an index */

storage/innobase/fts/fts0fts.cc

@@ -3494,13 +3494,13 @@ fts_add_doc_by_id(
 		}
 
 		if (!is_id_cluster) {
-			btr_pcur_close(doc_pcur);
+			ut_free(doc_pcur->old_rec_buf);
 		}
 	}
 func_exit:
 	mtr_commit(&mtr);
 
-	btr_pcur_close(&pcur);
+	ut_free(pcur.old_rec_buf);
 
 	mem_heap_free(heap);
 	return(TRUE);
@@ -3581,7 +3581,6 @@ fts_get_max_doc_id(
 	}
 
 func_exit:
-	btr_pcur_close(&pcur);
 	mtr_commit(&mtr);
 	return(doc_id);
 }

storage/innobase/handler/ha_innodb.cc

@@ -2161,9 +2161,9 @@ static void drop_garbage_tables_after_restore()
     pcur.restore_position(BTR_SEARCH_LEAF, &mtr);
   }
 
-  btr_pcur_close(&pcur);
   mtr.commit();
   trx->free();
+  ut_free(pcur.old_rec_buf);
   ut_d(purge_sys.resume_FTS());
 }
 

storage/innobase/handler/handler0alter.cc

@@ -5983,7 +5983,7 @@ static bool innobase_instant_try(
 		if (offsets_heap) {
 			mem_heap_free(offsets_heap);
 		}
-		btr_pcur_close(&pcur);
+		ut_free(pcur.old_rec_buf);
 		goto func_exit;
 	} else if (is_root && page_rec_is_supremum(rec)
 		   && !index->table->instant) {

storage/innobase/ibuf/ibuf0ibuf.cc

@@ -2332,13 +2332,11 @@ static void ibuf_read_merge_pages(const uint32_t* space_ids,
 
 			if (btr_pcur_is_after_last_on_page(&pcur)) {
 				ibuf_mtr_commit(&mtr);
-				btr_pcur_close(&pcur);
 				goto loop;
 			}
 		}
 done:
 		ibuf_mtr_commit(&mtr);
-		btr_pcur_close(&pcur);
 		mem_heap_empty(heap);
 #endif
 	}
@@ -2390,7 +2388,6 @@ ibuf_merge_pages(
 		      == page_id_t(IBUF_SPACE_ID, FSP_IBUF_TREE_ROOT_PAGE_NO));
 
 		ibuf_mtr_commit(&mtr);
-		btr_pcur_close(&pcur);
 
 		return(0);
 	}
@@ -2400,7 +2397,6 @@ ibuf_merge_pages(
 					    space_ids,
 					    page_nos, n_pages);
 	ibuf_mtr_commit(&mtr);
-	btr_pcur_close(&pcur);
 
 	ibuf_read_merge_pages(space_ids, page_nos, *n_pages);
 
@@ -2458,8 +2454,6 @@ ibuf_merge_space(
 
 	ibuf_mtr_commit(&mtr);
 
-	btr_pcur_close(&pcur);
-
 	if (n_pages > 0) {
 		ut_ad(n_pages <= UT_ARR_SIZE(pages));
 
@@ -3431,8 +3425,7 @@ ibuf_insert_low(
 
 func_exit:
 	ibuf_mtr_commit(&mtr);
-	btr_pcur_close(&pcur);
-
+	ut_free(pcur.old_rec_buf);
 	mem_heap_free(heap);
 
 	if (err == DB_SUCCESS
@@ -4423,8 +4416,7 @@ void ibuf_merge_or_delete_for_page(buf_block_t *block, const page_id_t page_id,
 			goto loop;
 		} else if (btr_pcur_is_after_last_on_page(&pcur)) {
 			ibuf_mtr_commit(&mtr);
-			btr_pcur_close(&pcur);
-
+			ut_free(pcur.old_rec_buf);
 			goto loop;
 		}
 	}
@@ -4435,12 +4427,12 @@ void ibuf_merge_or_delete_for_page(buf_block_t *block, const page_id_t page_id,
 	}
 
 	ibuf_mtr_commit(&mtr);
+	ut_free(pcur.old_rec_buf);
 
 	if (space) {
 		space->release();
 	}
 
-	btr_pcur_close(&pcur);
 	mem_heap_free(heap);
 
 	ibuf.n_merges++;
@@ -4506,20 +4498,20 @@ void ibuf_delete_for_discarded_space(ulint space)
 			we start from the beginning again */
 
 			ut_ad(mtr.has_committed());
+clear:
+			ut_free(pcur.old_rec_buf);
 			goto loop;
 		}
 
 		if (btr_pcur_is_after_last_on_page(&pcur)) {
 			ibuf_mtr_commit(&mtr);
-			btr_pcur_close(&pcur);
-
-			goto loop;
+			goto clear;
 		}
 	}
 
 leave_loop:
 	ibuf_mtr_commit(&mtr);
-	btr_pcur_close(&pcur);
+	ut_free(pcur.old_rec_buf);
 
 	ibuf_add_ops(ibuf.n_discarded_ops, dops);
 

storage/innobase/include/btr0pcur.h

@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2017, 2021, MariaDB Corporation.
+Copyright (c) 2017, 2022, 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
@@ -92,8 +92,7 @@ btr_pcur_free(
 	btr_pcur_t*	pcur);
 
 /**************************************************************//**
-Initializes and opens a persistent cursor to an index tree. It should be
-closed with btr_pcur_close. */
+Initializes and opens a persistent cursor to an index tree. */
 UNIV_INLINE
 dberr_t
 btr_pcur_open_low(
@@ -218,9 +217,7 @@ cursor is currently positioned. The latch is acquired by the
 are not allowed, you must take care (if using the cursor in S-mode) to
 manually release the latch by either calling
 btr_leaf_page_release(btr_pcur_get_block(&pcur), pcur.latch_mode, mtr)
-or by committing the mini-transaction right after btr_pcur_close().
-A subsequent attempt to crawl the same page in the same mtr would cause
-an assertion failure. */
+or by mtr_t::commit(). */
 UNIV_INLINE
 void
 btr_pcur_close(