From d13408f8e40705f6a01db6aff9365fe5bd8f7659 Mon Sep 17 00:00:00 2001 From: Annamalai Gurusami Date: Thu, 30 Jan 2014 12:38:13 +0530 Subject: [PATCH] Bug #14668683 ASSERT REC_GET_DELETED_FLAG(REC, PAGE_IS_COMP(PAGE)) Problem: The function row_upd_changes_ord_field_binary() is used to decide whether to use row_upd_clust_rec_by_insert() or row_upd_clust_rec(). The function row_upd_changes_ord_field_binary() does not make use of charset information. Based on binary comparison it decides that r1 and r2 differ in their ordering fields. In the function row_upd_clust_rec_by_insert(), an update is done by delete + insert. These operations internally make use of cmp_dtuple_rec_with_match() to compare records r1 and r2. This comparison takes place with the use of charset information. This means that it is possible for the deleted record to be reused in the subsequent insert. In the given scenario, the characters 'a' and 'A' are considered equal in the my_charset_latin1. When this happens, the ownership information of externally stored blobs are not correctly handled. Solution: When an update is done by delete followed by insert, disown the relevant externally stored fields during the delete marking itself (within the same mtr). If the insert succeeds, then nothing with respect to blob ownership needs to be done. If the insert fails, then the disown done earlier will be removed when the operation is rolled back. rb#4479 approved by Marko. --- .../innodb/r/innodb-update-insert.result | 43 +++++++++++++++++++ .../suite/innodb/t/innodb-update-insert.test | 38 ++++++++++++++++ storage/innobase/row/row0upd.c | 39 ++++------------- 3 files changed, 89 insertions(+), 31 deletions(-) create mode 100644 mysql-test/suite/innodb/r/innodb-update-insert.result create mode 100644 mysql-test/suite/innodb/t/innodb-update-insert.test diff --git a/mysql-test/suite/innodb/r/innodb-update-insert.result b/mysql-test/suite/innodb/r/innodb-update-insert.result new file mode 100644 index 00000000000..cd0fed101ab --- /dev/null +++ b/mysql-test/suite/innodb/r/innodb-update-insert.result @@ -0,0 +1,43 @@ +# +# Bug#14668683 ASSERT REC_GET_DELETED_FLAG(REC, PAGE_IS_COMP(PAGE)) +# +create table t1(f1 char(1) primary key, f2 int not null, f3 blob) +engine=innodb; +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `f1` char(1) NOT NULL, + `f2` int(11) NOT NULL, + `f3` blob, + PRIMARY KEY (`f1`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +insert into t1 values ('a', 0, repeat('b',8102)); +select f1 from t1; +f1 +a +update t1 set f1='A'; +select f1 from t1; +f1 +A +drop table t1; +# +# Another test case +# +create table t1 (f1 char(1), f2 longblob, f3 blob, primary key(f1)) +charset=utf8 engine=innodb; +replace into t1 set f1=0xa3; +Warnings: +Warning 1366 Incorrect string value: '\xA3' for column 'f1' at row 1 +select f1 from t1; +f1 + +update t1 set f1=0x6a; +update t1 set f3=repeat(0xb1,8103); +update t1 set f1=0x4a; +update t1 set f1=0x82; +Warnings: +Warning 1366 Incorrect string value: '\x82' for column 'f1' at row 1 +select f1 from t1; +f1 + +drop table t1; diff --git a/mysql-test/suite/innodb/t/innodb-update-insert.test b/mysql-test/suite/innodb/t/innodb-update-insert.test new file mode 100644 index 00000000000..95387e0564e --- /dev/null +++ b/mysql-test/suite/innodb/t/innodb-update-insert.test @@ -0,0 +1,38 @@ +# +# Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved. +# + +# This file contains test cases for checking the functionality "update by +# delete + insert". + +--source include/have_innodb.inc + +--echo # +--echo # Bug#14668683 ASSERT REC_GET_DELETED_FLAG(REC, PAGE_IS_COMP(PAGE)) +--echo # + +create table t1(f1 char(1) primary key, f2 int not null, f3 blob) +engine=innodb; +show create table t1; + +insert into t1 values ('a', 0, repeat('b',8102)); +select f1 from t1; +update t1 set f1='A'; +select f1 from t1; +drop table t1; + +--echo # +--echo # Another test case +--echo # +create table t1 (f1 char(1), f2 longblob, f3 blob, primary key(f1)) +charset=utf8 engine=innodb; + +replace into t1 set f1=0xa3; +select f1 from t1; +update t1 set f1=0x6a; +update t1 set f3=repeat(0xb1,8103); +update t1 set f1=0x4a; +update t1 set f1=0x82; +select f1 from t1; + +drop table t1; diff --git a/storage/innobase/row/row0upd.c b/storage/innobase/row/row0upd.c index 8a4d52d7e3d..dccfa5ceb5c 100644 --- a/storage/innobase/row/row0upd.c +++ b/storage/innobase/row/row0upd.c @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 1996, 2013, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 1996, 2014, 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 @@ -1873,7 +1873,13 @@ err_exit: rec, offsets, entry, node->update); if (change_ownership) { - btr_pcur_store_position(pcur, mtr); + /* The blobs are disowned here, expecting the + insert down below to inherit them. But if the + insert fails, then this disown will be undone + when the operation is rolled back. */ + btr_cur_disown_inherited_fields( + btr_cur_get_page_zip(btr_cur), + rec, index, offsets, node->update, mtr); } } @@ -1899,35 +1905,6 @@ err_exit: ? UPD_NODE_INSERT_BLOB : UPD_NODE_INSERT_CLUSTERED; - if (err == DB_SUCCESS && change_ownership) { - /* Mark the non-updated fields disowned by the old record. */ - - /* NOTE: this transaction has an x-lock on the record - and therefore other transactions cannot modify the - record when we have no latch on the page. In addition, - we assume that other query threads of the same - transaction do not modify the record in the meantime. - Therefore we can assert that the restoration of the - cursor succeeds. */ - - mtr_start(mtr); - - if (!btr_pcur_restore_position(BTR_MODIFY_LEAF, pcur, mtr)) { - ut_error; - } - - rec = btr_cur_get_rec(btr_cur); - offsets = rec_get_offsets(rec, index, offsets, - ULINT_UNDEFINED, &heap); - ut_ad(page_rec_is_user_rec(rec)); - - btr_cur_disown_inherited_fields( - btr_cur_get_page_zip(btr_cur), - rec, index, offsets, node->update, mtr); - - mtr_commit(mtr); - } - mem_heap_free(heap); return(err);