From 76e064e209247bef208c119bd7a1d01965f06149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 8 Mar 2012 14:56:22 +0200 Subject: [PATCH] Bug#13807811 BTR_PCUR_RESTORE_POSITION() CAN SKIP A RECORD This bug has been there at least since MySQL 4.0.9. (Before 4.0.9, the code probably was even more severely broken.) btr_pcur_restore_position(): When cursor restoration fails, before invoking btr_pcur_store_position() move to the previous or next record unless cursor->rel_pos==BTR_PCUR_ON or the record was not a user record. This bug can cause skipped records when btr_pcur_store_position() is called on the last record of a page. A symptom would be record count mismatch in CHECK TABLE, or failure to find a record to delete-mark or update or purge. The following operations should be affected by the bug: * row_search_for_mysql(): SELECT, UPDATE, REPLACE, CHECK TABLE, (almost anything else than INSERT) * foreign key CASCADE operations * row_merge_read_clustered_index(): index creation (since MySQL 5.1 InnoDB Plugin) * multi-threaded purge (after MySQL 5.5): not sure, but it might fail to purge some records Not all callers of btr_pcur_restore_position() should be affected. Anything that asserts or checks that restoration succeeds is unaffected. For example, cursor restoration on the change buffer tree should always succeed, because access is being protected by additional latches. Likewise, rollback, or any code accesses data dictionary tables while holding dict_sys->mutex should be safe. rb:967 approved by Jimmy Yang --- storage/innobase/btr/btr0pcur.c | 66 +++++++++++++++++++-------- storage/innodb_plugin/ChangeLog | 5 ++ storage/innodb_plugin/btr/btr0pcur.c | 68 ++++++++++++++++++++-------- 3 files changed, 99 insertions(+), 40 deletions(-) diff --git a/storage/innobase/btr/btr0pcur.c b/storage/innobase/btr/btr0pcur.c index 8d473794243..439ab3093b2 100644 --- a/storage/innobase/btr/btr0pcur.c +++ b/storage/innobase/btr/btr0pcur.c @@ -291,13 +291,20 @@ btr_pcur_restore_position( /* Save the old search mode of the cursor */ old_mode = cursor->search_mode; - if (UNIV_LIKELY(cursor->rel_pos == BTR_PCUR_ON)) { + switch (cursor->rel_pos) { + case BTR_PCUR_ON: mode = PAGE_CUR_LE; - } else if (cursor->rel_pos == BTR_PCUR_AFTER) { + break; + case BTR_PCUR_AFTER: mode = PAGE_CUR_G; - } else { - ut_ad(cursor->rel_pos == BTR_PCUR_BEFORE); + break; + case BTR_PCUR_BEFORE: mode = PAGE_CUR_L; + break; +#ifdef UNIV_DEBUG + default: + ut_error; +#endif /* UNIV_DEBUG */ } btr_pcur_open_with_no_init(index, tuple, mode, latch_mode, @@ -306,26 +313,45 @@ btr_pcur_restore_position( /* Restore the old search mode */ cursor->search_mode = old_mode; - if (cursor->rel_pos == BTR_PCUR_ON - && btr_pcur_is_on_user_rec(cursor, mtr) - && 0 == cmp_dtuple_rec(tuple, btr_pcur_get_rec(cursor), - rec_get_offsets( - btr_pcur_get_rec(cursor), index, - NULL, ULINT_UNDEFINED, &heap))) { + if (btr_pcur_is_on_user_rec(cursor, mtr)) { + switch (cursor->rel_pos) { + case BTR_PCUR_ON: + if (!cmp_dtuple_rec( + tuple, btr_pcur_get_rec(cursor), + rec_get_offsets(btr_pcur_get_rec(cursor), + index, NULL, + ULINT_UNDEFINED, &heap))) { - /* We have to store the NEW value for the modify clock, since - the cursor can now be on a different page! But we can retain - the value of old_rec */ + /* We have to store the NEW value for + the modify clock, since the cursor can + now be on a different page! But we can + retain the value of old_rec */ - cursor->block_when_stored = buf_block_align( - btr_pcur_get_page(cursor)); - cursor->modify_clock = buf_block_get_modify_clock( - cursor->block_when_stored); - cursor->old_stored = BTR_PCUR_OLD_STORED; + cursor->block_when_stored = + buf_block_align( + btr_pcur_get_page(cursor)); + cursor->modify_clock = + buf_block_get_modify_clock( + cursor->block_when_stored); + cursor->old_stored = BTR_PCUR_OLD_STORED; - mem_heap_free(heap); + mem_heap_free(heap); - return(TRUE); + return(TRUE); + } + + break; + case BTR_PCUR_BEFORE: + page_cur_move_to_next(btr_pcur_get_page_cur(cursor)); + break; + case BTR_PCUR_AFTER: + page_cur_move_to_prev(btr_pcur_get_page_cur(cursor)); + break; +#ifdef UNIV_DEBUG + default: + ut_error; +#endif /* UNIV_DEBUG */ + } } mem_heap_free(heap); diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 373335b6ff5..b43aed5cc06 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,8 @@ +2012-03-08 The InnoDB Team + + * btr/btr0pcur.c: + Fix Bug#13807811 BTR_PCUR_RESTORE_POSITION() CAN SKIP A RECORD + 2012-02-28 The InnoDB Team * btr/btr0btr.c, dict/dict0dict.c, include/btr0btr.h, diff --git a/storage/innodb_plugin/btr/btr0pcur.c b/storage/innodb_plugin/btr/btr0pcur.c index 57d9752649f..6cbfd8a64b7 100644 --- a/storage/innodb_plugin/btr/btr0pcur.c +++ b/storage/innodb_plugin/btr/btr0pcur.c @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 1996, 2011, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 1996, 2012, 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 @@ -121,6 +121,8 @@ btr_pcur_store_position( ut_a(btr_page_get_next(page, mtr) == FIL_NULL); ut_a(btr_page_get_prev(page, mtr) == FIL_NULL); + ut_ad(page_is_leaf(page)); + ut_ad(page_get_page_no(page) == index->page); cursor->old_stored = BTR_PCUR_OLD_STORED; @@ -313,13 +315,20 @@ btr_pcur_restore_position_func( /* Save the old search mode of the cursor */ old_mode = cursor->search_mode; - if (UNIV_LIKELY(cursor->rel_pos == BTR_PCUR_ON)) { + switch (cursor->rel_pos) { + case BTR_PCUR_ON: mode = PAGE_CUR_LE; - } else if (cursor->rel_pos == BTR_PCUR_AFTER) { + break; + case BTR_PCUR_AFTER: mode = PAGE_CUR_G; - } else { - ut_ad(cursor->rel_pos == BTR_PCUR_BEFORE); + break; + case BTR_PCUR_BEFORE: mode = PAGE_CUR_L; + break; +#ifdef UNIV_DEBUG + default: + ut_error; +#endif /* UNIV_DEBUG */ } btr_pcur_open_with_no_init_func(index, tuple, mode, latch_mode, @@ -328,25 +337,44 @@ btr_pcur_restore_position_func( /* Restore the old search mode */ cursor->search_mode = old_mode; - if (cursor->rel_pos == BTR_PCUR_ON - && btr_pcur_is_on_user_rec(cursor) - && 0 == cmp_dtuple_rec(tuple, btr_pcur_get_rec(cursor), - rec_get_offsets( - btr_pcur_get_rec(cursor), index, - NULL, ULINT_UNDEFINED, &heap))) { + if (btr_pcur_is_on_user_rec(cursor)) { + switch (cursor->rel_pos) { + case BTR_PCUR_ON: + if (!cmp_dtuple_rec( + tuple, btr_pcur_get_rec(cursor), + rec_get_offsets(btr_pcur_get_rec(cursor), + index, NULL, + ULINT_UNDEFINED, &heap))) { - /* We have to store the NEW value for the modify clock, since - the cursor can now be on a different page! But we can retain - the value of old_rec */ + /* We have to store the NEW value for + the modify clock, since the cursor can + now be on a different page! But we can + retain the value of old_rec */ - cursor->block_when_stored = btr_pcur_get_block(cursor); - cursor->modify_clock = buf_block_get_modify_clock( - cursor->block_when_stored); - cursor->old_stored = BTR_PCUR_OLD_STORED; + cursor->block_when_stored = + btr_pcur_get_block(cursor); + cursor->modify_clock = + buf_block_get_modify_clock( + cursor->block_when_stored); + cursor->old_stored = BTR_PCUR_OLD_STORED; - mem_heap_free(heap); + mem_heap_free(heap); - return(TRUE); + return(TRUE); + } + + break; + case BTR_PCUR_BEFORE: + page_cur_move_to_next(btr_pcur_get_page_cur(cursor)); + break; + case BTR_PCUR_AFTER: + page_cur_move_to_prev(btr_pcur_get_page_cur(cursor)); + break; +#ifdef UNIV_DEBUG + default: + ut_error; +#endif /* UNIV_DEBUG */ + } } mem_heap_free(heap);