From 9f57e595b49744e3c1920f8c46a2ec8e7fc5fd66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 17 May 2017 16:14:41 +0300 Subject: [PATCH] Refactor trx_undo_report_row_operation() Fix a -fsanitizer=undefined warning that trx_undo_report_row_operation() was being passed thr=NULL when the BTR_NO_UNDO_LOG_FLAG flag was set. trx_undo_report_row_operation(): Remove the first two parameters. The parameter clust_entry!=NULL distinguishes inserts from updates. This should be a non-functional change (no observable change in behaviour; slightly smaller code). --- storage/innobase/btr/btr0cur.cc | 26 +++++++++-------- storage/innobase/include/trx0rec.h | 11 ++------ storage/innobase/trx/trx0rec.cc | 45 +++++++----------------------- storage/xtradb/btr/btr0cur.cc | 26 +++++++++-------- storage/xtradb/include/trx0rec.h | 11 ++------ storage/xtradb/trx/trx0rec.cc | 42 ++++++---------------------- 6 files changed, 53 insertions(+), 108 deletions(-) diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 7846debc115..7ca38c5b67d 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -1164,18 +1164,21 @@ btr_cur_ins_lock_and_undo( index, thr, mtr, inherit); if (err != DB_SUCCESS + || !(~flags | (BTR_NO_UNDO_LOG_FLAG | BTR_KEEP_SYS_FLAG)) || !dict_index_is_clust(index) || dict_index_is_ibuf(index)) { return(err); } - err = trx_undo_report_row_operation(flags, TRX_UNDO_INSERT_OP, - thr, index, entry, - NULL, 0, NULL, NULL, - &roll_ptr); - if (err != DB_SUCCESS) { - - return(err); + if (flags & BTR_NO_UNDO_LOG_FLAG) { + roll_ptr = 0; + } else { + err = trx_undo_report_row_operation(thr, index, entry, + NULL, 0, NULL, NULL, + &roll_ptr); + if (err != DB_SUCCESS) { + return(err); + } } /* Now we can fill in the roll ptr field in entry */ @@ -1712,9 +1715,10 @@ btr_cur_upd_lock_and_undo( /* Append the info about the update in the undo log */ - return(trx_undo_report_row_operation( - flags, TRX_UNDO_MODIFY_OP, thr, - index, NULL, update, + return((flags & BTR_NO_UNDO_LOG_FLAG) + ? DB_SUCCESS + : trx_undo_report_row_operation( + thr, index, NULL, update, cmpl_info, rec, offsets, roll_ptr)); } @@ -2985,7 +2989,7 @@ btr_cur_del_mark_set_clust_rec( return(err); } - err = trx_undo_report_row_operation(0, TRX_UNDO_MODIFY_OP, thr, + err = trx_undo_report_row_operation(thr, index, NULL, NULL, 0, rec, offsets, &roll_ptr); if (err != DB_SUCCESS) { diff --git a/storage/innobase/include/trx0rec.h b/storage/innobase/include/trx0rec.h index 359937e3583..a6e202d04e4 100644 --- a/storage/innobase/include/trx0rec.h +++ b/storage/innobase/include/trx0rec.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -212,10 +213,6 @@ UNIV_INTERN dberr_t trx_undo_report_row_operation( /*==========================*/ - ulint flags, /*!< in: if BTR_NO_UNDO_LOG_FLAG bit is - set, does nothing */ - ulint op_type, /*!< in: TRX_UNDO_INSERT_OP or - TRX_UNDO_MODIFY_OP */ que_thr_t* thr, /*!< in: query thread */ dict_index_t* index, /*!< in: clustered index */ const dtuple_t* clust_entry, /*!< in: in the case of an insert, @@ -233,7 +230,7 @@ trx_undo_report_row_operation( inserted undo log record, 0 if BTR_NO_UNDO_LOG flag was specified */ - MY_ATTRIBUTE((nonnull(3,4,10), warn_unused_result)); + MY_ATTRIBUTE((nonnull(1,2,8), warn_unused_result)); /******************************************************************//** Copies an undo record to heap. This function can be called if we know that the undo log record exists. @@ -313,10 +310,6 @@ record */ storage fields: used by purge to free the external storage */ -/* Operation type flags used in trx_undo_report_row_operation */ -#define TRX_UNDO_INSERT_OP 1 -#define TRX_UNDO_MODIFY_OP 2 - #ifndef UNIV_NONINL #include "trx0rec.ic" #endif diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc index 5fa74846243..f694cdd3d5e 100644 --- a/storage/innobase/trx/trx0rec.cc +++ b/storage/innobase/trx/trx0rec.cc @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -1185,10 +1186,6 @@ UNIV_INTERN dberr_t trx_undo_report_row_operation( /*==========================*/ - ulint flags, /*!< in: if BTR_NO_UNDO_LOG_FLAG bit is - set, does nothing */ - ulint op_type, /*!< in: TRX_UNDO_INSERT_OP or - TRX_UNDO_MODIFY_OP */ que_thr_t* thr, /*!< in: query thread */ dict_index_t* index, /*!< in: clustered index */ const dtuple_t* clust_entry, /*!< in: in the case of an insert, @@ -1221,17 +1218,8 @@ trx_undo_report_row_operation( ut_ad(!srv_read_only_mode); ut_a(dict_index_is_clust(index)); ut_ad(!rec || rec_offs_validate(rec, index, offsets)); - - if (flags & BTR_NO_UNDO_LOG_FLAG) { - - *roll_ptr = 0; - - return(DB_SUCCESS); - } - ut_ad(thr); - ut_ad((op_type != TRX_UNDO_INSERT_OP) - || (clust_entry && !update && !rec)); + ut_ad(!clust_entry || (!update && !rec)); trx = thr_get_trx(thr); @@ -1252,8 +1240,7 @@ trx_undo_report_row_operation( /* If the undo log is not assigned yet, assign one */ - switch (op_type) { - case TRX_UNDO_INSERT_OP: + if (clust_entry) { undo = trx->insert_undo; if (undo == NULL) { @@ -1269,10 +1256,7 @@ trx_undo_report_row_operation( ut_ad(err == DB_SUCCESS); } - break; - default: - ut_ad(op_type == TRX_UNDO_MODIFY_OP); - + } else { undo = trx->update_undo; if (undo == NULL) { @@ -1296,23 +1280,14 @@ trx_undo_report_row_operation( buf_block_dbg_add_level(undo_block, SYNC_TRX_UNDO_PAGE); do { - page_t* undo_page; - ulint offset; - - undo_page = buf_block_get_frame(undo_block); ut_ad(page_no == buf_block_get_page_no(undo_block)); - - switch (op_type) { - case TRX_UNDO_INSERT_OP: - offset = trx_undo_page_report_insert( - undo_page, trx, index, clust_entry, &mtr); - break; - default: - ut_ad(op_type == TRX_UNDO_MODIFY_OP); - offset = trx_undo_page_report_modify( + page_t* undo_page = buf_block_get_frame(undo_block); + ulint offset = clust_entry + ? trx_undo_page_report_insert( + undo_page, trx, index, clust_entry, &mtr) + : trx_undo_page_report_modify( undo_page, trx, index, rec, offsets, update, cmpl_info, &mtr); - } if (UNIV_UNLIKELY(offset == 0)) { /* The record did not fit on the page. We erase the @@ -1363,7 +1338,7 @@ trx_undo_report_row_operation( mutex_exit(&trx->undo_mutex); *roll_ptr = trx_undo_build_roll_ptr( - op_type == TRX_UNDO_INSERT_OP, + clust_entry != NULL, rseg->id, page_no, offset); return(DB_SUCCESS); } diff --git a/storage/xtradb/btr/btr0cur.cc b/storage/xtradb/btr/btr0cur.cc index 0b554b271c4..c495e178b21 100644 --- a/storage/xtradb/btr/btr0cur.cc +++ b/storage/xtradb/btr/btr0cur.cc @@ -1269,18 +1269,21 @@ btr_cur_ins_lock_and_undo( index, thr, mtr, inherit); if (err != DB_SUCCESS + || !(~flags | (BTR_NO_UNDO_LOG_FLAG | BTR_KEEP_SYS_FLAG)) || !dict_index_is_clust(index) || dict_index_is_ibuf(index)) { return(err); } - err = trx_undo_report_row_operation(flags, TRX_UNDO_INSERT_OP, - thr, index, entry, - NULL, 0, NULL, NULL, - &roll_ptr); - if (err != DB_SUCCESS) { - - return(err); + if (flags & BTR_NO_UNDO_LOG_FLAG) { + roll_ptr = 0; + } else { + err = trx_undo_report_row_operation(thr, index, entry, + NULL, 0, NULL, NULL, + &roll_ptr); + if (err != DB_SUCCESS) { + return(err); + } } /* Now we can fill in the roll ptr field in entry */ @@ -1851,9 +1854,10 @@ btr_cur_upd_lock_and_undo( /* Append the info about the update in the undo log */ - return(trx_undo_report_row_operation( - flags, TRX_UNDO_MODIFY_OP, thr, - index, NULL, update, + return((flags & BTR_NO_UNDO_LOG_FLAG) + ? DB_SUCCESS + : trx_undo_report_row_operation( + thr, index, NULL, update, cmpl_info, rec, offsets, roll_ptr)); } @@ -3175,7 +3179,7 @@ btr_cur_del_mark_set_clust_rec( return(err); } - err = trx_undo_report_row_operation(0, TRX_UNDO_MODIFY_OP, thr, + err = trx_undo_report_row_operation(thr, index, NULL, NULL, 0, rec, offsets, &roll_ptr); if (err != DB_SUCCESS) { diff --git a/storage/xtradb/include/trx0rec.h b/storage/xtradb/include/trx0rec.h index 359937e3583..a6e202d04e4 100644 --- a/storage/xtradb/include/trx0rec.h +++ b/storage/xtradb/include/trx0rec.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 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 @@ -212,10 +213,6 @@ UNIV_INTERN dberr_t trx_undo_report_row_operation( /*==========================*/ - ulint flags, /*!< in: if BTR_NO_UNDO_LOG_FLAG bit is - set, does nothing */ - ulint op_type, /*!< in: TRX_UNDO_INSERT_OP or - TRX_UNDO_MODIFY_OP */ que_thr_t* thr, /*!< in: query thread */ dict_index_t* index, /*!< in: clustered index */ const dtuple_t* clust_entry, /*!< in: in the case of an insert, @@ -233,7 +230,7 @@ trx_undo_report_row_operation( inserted undo log record, 0 if BTR_NO_UNDO_LOG flag was specified */ - MY_ATTRIBUTE((nonnull(3,4,10), warn_unused_result)); + MY_ATTRIBUTE((nonnull(1,2,8), warn_unused_result)); /******************************************************************//** Copies an undo record to heap. This function can be called if we know that the undo log record exists. @@ -313,10 +310,6 @@ record */ storage fields: used by purge to free the external storage */ -/* Operation type flags used in trx_undo_report_row_operation */ -#define TRX_UNDO_INSERT_OP 1 -#define TRX_UNDO_MODIFY_OP 2 - #ifndef UNIV_NONINL #include "trx0rec.ic" #endif diff --git a/storage/xtradb/trx/trx0rec.cc b/storage/xtradb/trx/trx0rec.cc index 5fa74846243..257e0380db3 100644 --- a/storage/xtradb/trx/trx0rec.cc +++ b/storage/xtradb/trx/trx0rec.cc @@ -1185,10 +1185,6 @@ UNIV_INTERN dberr_t trx_undo_report_row_operation( /*==========================*/ - ulint flags, /*!< in: if BTR_NO_UNDO_LOG_FLAG bit is - set, does nothing */ - ulint op_type, /*!< in: TRX_UNDO_INSERT_OP or - TRX_UNDO_MODIFY_OP */ que_thr_t* thr, /*!< in: query thread */ dict_index_t* index, /*!< in: clustered index */ const dtuple_t* clust_entry, /*!< in: in the case of an insert, @@ -1222,16 +1218,8 @@ trx_undo_report_row_operation( ut_a(dict_index_is_clust(index)); ut_ad(!rec || rec_offs_validate(rec, index, offsets)); - if (flags & BTR_NO_UNDO_LOG_FLAG) { - - *roll_ptr = 0; - - return(DB_SUCCESS); - } - ut_ad(thr); - ut_ad((op_type != TRX_UNDO_INSERT_OP) - || (clust_entry && !update && !rec)); + ut_ad(!clust_entry || (!update && !rec)); trx = thr_get_trx(thr); @@ -1252,8 +1240,7 @@ trx_undo_report_row_operation( /* If the undo log is not assigned yet, assign one */ - switch (op_type) { - case TRX_UNDO_INSERT_OP: + if (clust_entry) { undo = trx->insert_undo; if (undo == NULL) { @@ -1269,10 +1256,7 @@ trx_undo_report_row_operation( ut_ad(err == DB_SUCCESS); } - break; - default: - ut_ad(op_type == TRX_UNDO_MODIFY_OP); - + } else { undo = trx->update_undo; if (undo == NULL) { @@ -1296,23 +1280,15 @@ trx_undo_report_row_operation( buf_block_dbg_add_level(undo_block, SYNC_TRX_UNDO_PAGE); do { - page_t* undo_page; - ulint offset; - - undo_page = buf_block_get_frame(undo_block); ut_ad(page_no == buf_block_get_page_no(undo_block)); - switch (op_type) { - case TRX_UNDO_INSERT_OP: - offset = trx_undo_page_report_insert( - undo_page, trx, index, clust_entry, &mtr); - break; - default: - ut_ad(op_type == TRX_UNDO_MODIFY_OP); - offset = trx_undo_page_report_modify( + page_t* undo_page = buf_block_get_frame(undo_block); + ulint offset = clust_entry + ? trx_undo_page_report_insert( + undo_page, trx, index, clust_entry, &mtr) + : trx_undo_page_report_modify( undo_page, trx, index, rec, offsets, update, cmpl_info, &mtr); - } if (UNIV_UNLIKELY(offset == 0)) { /* The record did not fit on the page. We erase the @@ -1363,7 +1339,7 @@ trx_undo_report_row_operation( mutex_exit(&trx->undo_mutex); *roll_ptr = trx_undo_build_roll_ptr( - op_type == TRX_UNDO_INSERT_OP, + clust_entry != NULL, rseg->id, page_no, offset); return(DB_SUCCESS); }