Bug#17565888: UP TO 75% INCREASE IN DBT3 QUERY (Q7) EXECUTION TIME

Regression from bug#14621190 due to disabled optimistic restoration
of cursor, which required full key lookup instead of verifying
if previously positioned btree cursor could be reused.

Fixed by enable optimistic restore and adjust cursor afterward.

rb#3324 approved by Marko.

storage/innobase/btr/btr0pcur.c

 /*****************************************************************************
 
-Copyright (c) 1996, 2012, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 1996, 2013, Oracle and/or its affiliates. All Rights Reserved.
 
 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
@@ -110,7 +110,7 @@ btr_pcur_store_position(
 	page_t*		page;
 	ulint		offs;
 
-	ut_a(cursor->pos_state == BTR_PCUR_IS_POSITIONED);
+	ut_ad(cursor->pos_state == BTR_PCUR_IS_POSITIONED);
 	ut_ad(cursor->latch_mode != BTR_NO_LATCHES);
 
 	block = btr_pcur_get_block(cursor);
@@ -235,21 +235,12 @@ btr_pcur_restore_position_func(
 
 	ut_ad(mtr);
 	ut_ad(mtr->state == MTR_ACTIVE);
+	ut_ad(cursor->old_stored == BTR_PCUR_OLD_STORED);
+	ut_ad(cursor->pos_state == BTR_PCUR_WAS_POSITIONED
+	      || cursor->pos_state == BTR_PCUR_IS_POSITIONED);
 
 	index = btr_cur_get_index(btr_pcur_get_btr_cur(cursor));
 
-	if (UNIV_UNLIKELY(cursor->old_stored != BTR_PCUR_OLD_STORED)
-	    || UNIV_UNLIKELY(cursor->pos_state != BTR_PCUR_WAS_POSITIONED
-			     && cursor->pos_state != BTR_PCUR_IS_POSITIONED)) {
-		ut_print_buf(stderr, cursor, sizeof(btr_pcur_t));
-		putc('\n', stderr);
-		if (cursor->trx_if_known) {
-			trx_print(stderr, cursor->trx_if_known, 0);
-		}
-
-		ut_error;
-	}
-
 	if (UNIV_UNLIKELY
 	    (cursor->rel_pos == BTR_PCUR_AFTER_LAST_IN_TREE
 	     || cursor->rel_pos == BTR_PCUR_BEFORE_FIRST_IN_TREE)) {
@@ -273,29 +264,26 @@ btr_pcur_restore_position_func(
 
 	if (UNIV_LIKELY(latch_mode == BTR_SEARCH_LEAF)
 	    || UNIV_LIKELY(latch_mode == BTR_MODIFY_LEAF)) {
-		/* Try optimistic restoration if cursor is expected to be
-		positioned on the same btr record as before (BTR_PCUR_ON). */
+		/* Try optimistic restoration. */
 
-		if (cursor->rel_pos == BTR_PCUR_ON
-		    && buf_page_optimistic_get(
-					latch_mode,
+		if (buf_page_optimistic_get(latch_mode,
 					    cursor->block_when_stored,
 					    cursor->modify_clock,
 					    file, line, mtr)) {
-			cursor->pos_state = BTR_PCUR_IS_POSITIONED;
 
 			buf_block_dbg_add_level(
 				btr_pcur_get_block(cursor),
 				dict_index_is_ibuf(index)
 				? SYNC_IBUF_TREE_NODE : SYNC_TREE_NODE);
 
-			{
+			if (cursor->rel_pos == BTR_PCUR_ON) {
 #ifdef UNIV_DEBUG
 				const rec_t*	rec;
 				const ulint*	offsets1;
 				const ulint*	offsets2;
 #endif /* UNIV_DEBUG */
 				cursor->latch_mode = latch_mode;
+				cursor->pos_state = BTR_PCUR_IS_POSITIONED;
 #ifdef UNIV_DEBUG
 				rec = btr_pcur_get_rec(cursor);
 
@@ -314,6 +302,11 @@ btr_pcur_restore_position_func(
 #endif /* UNIV_DEBUG */
 				return(TRUE);
 			}
+			/* This is the same record as stored,
+			may need to be adjusted for BTR_PCUR_BEFORE/AFTER,
+			depending on search mode and direction. */
+			cursor->pos_state = BTR_PCUR_IS_POSITIONED_OPTIMISTIC;
+			return(FALSE);
 		}
 	}
 
@@ -414,7 +407,7 @@ btr_pcur_move_to_next_page(
 	buf_block_t*	next_block;
 	page_t*		next_page;
 
-	ut_a(cursor->pos_state == BTR_PCUR_IS_POSITIONED);
+	ut_ad(cursor->pos_state == BTR_PCUR_IS_POSITIONED);
 	ut_ad(cursor->latch_mode != BTR_NO_LATCHES);
 	ut_ad(btr_pcur_is_after_last_on_page(cursor));
 
@@ -469,7 +462,6 @@ btr_pcur_move_backward_from_page(
 	ulint		latch_mode;
 	ulint		latch_mode2;
 
-	ut_a(cursor->pos_state == BTR_PCUR_IS_POSITIONED);
 	ut_ad(cursor->latch_mode != BTR_NO_LATCHES);
 	ut_ad(btr_pcur_is_before_first_on_page(cursor));
 	ut_ad(!btr_pcur_is_before_first_in_tree(cursor, mtr));

storage/innobase/include/btr0pcur.h

 /*****************************************************************************
 
-Copyright (c) 1996, 2011, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 1996, 2013, Oracle and/or its affiliates. All Rights Reserved.
 
 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
@@ -447,6 +447,27 @@ btr_pcur_move_to_prev_on_page(
 /*==========================*/
 	btr_pcur_t*	cursor);/*!< in/out: persistent cursor */
 
+/** Position state of persistent B-tree cursor. */
+enum pcur_pos_t {
+	/** The persistent cursor is not positioned. */
+	BTR_PCUR_NOT_POSITIONED = 0,
+	/** The persistent cursor was previously positioned.
+	TODO: currently, the state can be BTR_PCUR_IS_POSITIONED,
+	though it really should be BTR_PCUR_WAS_POSITIONED,
+	because we have no obligation to commit the cursor with
+	mtr; similarly latch_mode may be out of date. This can
+	lead to problems if btr_pcur is not used the right way;
+	all current code should be ok. */
+	BTR_PCUR_WAS_POSITIONED,
+	/** The persistent cursor is positioned by optimistic get to the same
+	record as it was positioned at. Not used for rel_pos == BTR_PCUR_ON.
+	It may need adjustment depending on previous/current search direction
+	and rel_pos. */
+	BTR_PCUR_IS_POSITIONED_OPTIMISTIC,
+	/** The persistent cursor is positioned by index search.
+	Or optimistic get for rel_pos == BTR_PCUR_ON. */
+	BTR_PCUR_IS_POSITIONED
+};
 
 /* The persistent B-tree cursor structure. This is used mainly for SQL
 selects, updates, and deletes. */
@@ -480,10 +501,8 @@ struct btr_pcur_struct{
 	ib_uint64_t	modify_clock;	/*!< the modify clock value of the
 					buffer block when the cursor position
 					was stored */
-	ulint		pos_state;	/*!< see TODO note below!
-					BTR_PCUR_IS_POSITIONED,
-					BTR_PCUR_WAS_POSITIONED,
-					BTR_PCUR_NOT_POSITIONED */
+	enum pcur_pos_t	pos_state;	/*!< btr_pcur_store_position() and
+					btr_pcur_restore_position() state. */
 	ulint		search_mode;	/*!< PAGE_CUR_G, ... */
 	trx_t*		trx_if_known;	/*!< the transaction, if we know it;
 					otherwise this field is not defined;
@@ -499,21 +518,6 @@ struct btr_pcur_struct{
 					is not NULL */
 };
 
-#define BTR_PCUR_IS_POSITIONED	1997660512	/* TODO: currently, the state
-						can be BTR_PCUR_IS_POSITIONED,
-						though it really should be
-						BTR_PCUR_WAS_POSITIONED,
-						because we have no obligation
-						to commit the cursor with
-						mtr; similarly latch_mode may
-						be out of date. This can
-						lead to problems if btr_pcur
-						is not used the right way;
-						all current code should be
-						ok. */
-#define BTR_PCUR_WAS_POSITIONED	1187549791
-#define BTR_PCUR_NOT_POSITIONED 1328997689
-
 #define BTR_PCUR_OLD_STORED	908467085
 #define BTR_PCUR_OLD_NOT_STORED	122766467
 

storage/innobase/include/btr0pcur.ic

 /*****************************************************************************
 
-Copyright (c) 1996, 2011, Oracle and/or its affiliates. All Rights Reserved.
+Copyright (c) 1996, 2013, Oracle and/or its affiliates. All Rights Reserved.
 
 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
@@ -309,7 +309,8 @@ btr_pcur_move_to_next_user_rec(
 				function may release the page latch */
 	mtr_t*		mtr)	/*!< in: mtr */
 {
-	ut_ad(cursor->pos_state == BTR_PCUR_IS_POSITIONED);
+	ut_ad(cursor->pos_state == BTR_PCUR_IS_POSITIONED_OPTIMISTIC
+	      || cursor->pos_state == BTR_PCUR_IS_POSITIONED);
 	ut_ad(cursor->latch_mode != BTR_NO_LATCHES);
 	cursor->old_stored = BTR_PCUR_OLD_NOT_STORED;
 loop:
@@ -379,7 +380,7 @@ btr_pcur_commit_specify_mtr(
 	btr_pcur_t*	pcur,	/*!< in: persistent cursor */
 	mtr_t*		mtr)	/*!< in: mtr to commit */
 {
-	ut_a(pcur->pos_state == BTR_PCUR_IS_POSITIONED);
+	ut_ad(pcur->pos_state == BTR_PCUR_IS_POSITIONED);
 
 	pcur->latch_mode = BTR_NO_LATCHES;
 

storage/innobase/row/row0sel.c

@@ -3096,54 +3096,75 @@ sel_restore_position_for_mysql(
 					mtr temporarily! */
 {
 	ibool		success;
-	ulint	relative_position;
-
-	relative_position = pcur->rel_pos;
 
 	success = btr_pcur_restore_position(latch_mode, pcur, mtr);
 
 	*same_user_rec = success;
 
-	if (relative_position == BTR_PCUR_ON) {
-		if (success) {
-			return(FALSE);
+	ut_ad(!success || pcur->rel_pos == BTR_PCUR_ON);
+#ifdef UNIV_DEBUG
+	if (pcur->pos_state == BTR_PCUR_IS_POSITIONED_OPTIMISTIC) {
+		ut_ad(pcur->rel_pos == BTR_PCUR_BEFORE
+		      || pcur->rel_pos == BTR_PCUR_AFTER);
+	} else {
+		ut_ad(pcur->pos_state == BTR_PCUR_IS_POSITIONED);
 	}
+#endif
 
-		if (moves_up) {
-			btr_pcur_move_to_next(pcur, mtr);
-		}
+	/* The position may need be adjusted for rel_pos and moves_up. */
 
+	switch (pcur->rel_pos) {
+	case BTR_PCUR_ON:
+		if (!success && moves_up) {
+next:
+			btr_pcur_move_to_next(pcur, mtr);
 			return(TRUE);
 		}
-
-	/* success can only be TRUE for BTR_PCUR_ON! */
-	ut_ad(!success);
-
-	/* BTR_PCUR_BEFORE -> the position is now set to the record before
-	pcur->old_rec.
-	BTR_PCUR_AFTER-> positioned to record after pcur->old_rec. */
-
-	if (relative_position == BTR_PCUR_AFTER
-	    || relative_position == BTR_PCUR_AFTER_LAST_IN_TREE) {
-
-		if (moves_up) {
+		return(!success);
+	case BTR_PCUR_AFTER_LAST_IN_TREE:
+	case BTR_PCUR_BEFORE_FIRST_IN_TREE:
 		return(TRUE);
-		}
-
-		if (btr_pcur_is_on_user_rec(pcur)) {
+	case BTR_PCUR_AFTER:
+		/* positioned to record after pcur->old_rec. */
+		pcur->pos_state = BTR_PCUR_IS_POSITIONED;
+prev:
+		if (btr_pcur_is_on_user_rec(pcur) && !moves_up) {
 			btr_pcur_move_to_prev(pcur, mtr);
 		}
-
 		return(TRUE);
+	case BTR_PCUR_BEFORE:
+		/* For non optimistic restoration:
+		The position is now set to the record before pcur->old_rec.
+
+		For optimistic restoration:
+		The position also needs to take the previous search_mode into
+		consideration. */
+
+		switch (pcur->pos_state) {
+		case BTR_PCUR_IS_POSITIONED_OPTIMISTIC:
+			pcur->pos_state = BTR_PCUR_IS_POSITIONED;
+			if (pcur->search_mode == PAGE_CUR_GE) {
+				/* Positioned during Greater or Equal search
+				with BTR_PCUR_BEFORE. Optimistic restore to
+				the same record. If scanning for lower then
+				we must move to previous record.
+				This can happen with:
+				HANDLER READ idx a = (const);
+				HANDLER READ idx PREV; */
+				goto prev;
 			}
-
-	ut_ad(relative_position == BTR_PCUR_BEFORE
-	      || relative_position == BTR_PCUR_BEFORE_FIRST_IN_TREE);
-
+			return(TRUE);
+		case BTR_PCUR_IS_POSITIONED:
 			if (moves_up && btr_pcur_is_on_user_rec(pcur)) {
-		btr_pcur_move_to_next(pcur, mtr);
+				goto next;
 			}
-
+			return(TRUE);
+		case BTR_PCUR_WAS_POSITIONED:
+		case BTR_PCUR_NOT_POSITIONED:
+			break;
+		}
+	}
+	ut_ad(0);
 	return(TRUE);
 }