Percona Server moved to https://jira.percona.com/projects/PS

Merge lp:~longbow/percona-server/bug_898306_5.1 into lp:percona-server/5.1

  1. bug_898306_5.1
  2. Merge into 5.1
Proposed by Valentine Gostev
Status: Merged
Approved by: Valentine Gostev
Approved revision: no longer in the source branch.
Merged at revision: 416
Proposed branch: lp:~longbow/percona-server/bug_898306_5.1
Merge into: lp:percona-server/5.1
Diff against target: 173 lines (+120/-0)
6 files modified
Percona-Server/mysql-test/r/percona_innodb_fake_changes_bug_898306.result (+40/-0)
Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_898306.test (+58/-0)
Percona-Server/sql/handler.h (+6/-0)
Percona-Server/sql/sql_insert.cc (+9/-0)
Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc (+6/-0)
Percona-Server/storage/innodb_plugin/handler/ha_innodb.h (+1/-0)
To merge this branch: bzr merge lp:~longbow/percona-server/bug_898306_5.1
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Review via email: mp+88859@code.launchpad.net

This proposal supersedes a proposal from 2012-01-17.

Description of the change

Added test and fix (Mark's patch) for bug 898306

To post a comment you must log in.
Revision history for this message
Valentine Gostev (longbow) wrote : Posted in a previous version of this proposal

http://jenkins.percona.com/view/Percona%20Server%205.1/job/percona-server-5.1-param/222

Revision history for this message
Oleg Tsarev (tsarev) wrote : Posted in a previous version of this proposal

Please add description of changes to commit message.

For example, this part of patch important

133 + /* Avoid the infinite loop
134 + * 1) get dup key on fake insert
135 + * 2) do nothing on fake delete
136 + * 3) goto #1
137 + */

And also important

review: Needs Fixing
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

The code itself looks good, but the style and test case need a few tweaks. Indentation and comments should match the server and InnoDB code style.

And the additional connection created in the test is not really used, so it should be removed. A comment header in the test referencing the bug number would also be useful.

review: Needs Fixing
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

My previous comments were addressed rather selectively:

1. Still no comment header in the test case.
2. Wrong comments style in sql_insert.cc.
3. Wrong indentation in ha_innodb.cc (spaces instead of tabs).

review: Needs Fixing
Revision history for this message
Alexey Kopytov (akopytov) wrote :

OK to merge.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== added file 'Percona-Server/mysql-test/r/percona_innodb_fake_changes_bug_898306.result'
--- Percona-Server/mysql-test/r/percona_innodb_fake_changes_bug_898306.result 1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/r/percona_innodb_fake_changes_bug_898306.result 2012-01-17 14:13:06 +0000
@@ -0,0 +1,40 @@
1DROP TABLE IF EXISTS t1;
2CREATE TABLE t1 (a INT primary key, b int, unique key (b)) ENGINE=InnoDB;
3INSERT INTO t1 VALUES (1,1);
4SET autocommit=1;
5SET innodb_fake_changes=1;
6# Confirm that duplicate key errors on REPLACE works
7REPLACE INTO t1 VALUES (1,1);
8ERROR HY000: Got error 131 during COMMIT
9REPLACE INTO t1 VALUES (1,2);
10ERROR HY000: Got error 131 during COMMIT
11# Confirm that duplicate key errors are OK
12BEGIN;
13REPLACE INTO t1 VALUES (1,2);
14SELECT * from t1;
15a b
161 1
17REPLACE INTO t1 VALUES (1,1);
18SELECT * from t1;
19a b
201 1
21ROLLBACK;
22BEGIN;
23REPLACE INTO t1 VALUES (2,1);
24ERROR 23000: Duplicate entry '1' for key 'b'
25INSERT INTO t1 VALUES (1,1);
26ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
27INSERT INTO t1 VALUES (1,2);
28ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
29INSERT INTO t1 VALUES (2,1);
30ERROR 23000: Duplicate entry '1' for key 'b'
31ROLLBACK;
32INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=b+10;
33ERROR HY000: Got error 131 during COMMIT
34INSERT INTO t1 VALUES (1,2) ON DUPLICATE KEY UPDATE b=b+10;
35ERROR HY000: Got error 131 during COMMIT
36SET innodb_fake_changes=0;
37SELECT * from t1;
38a b
391 1
40DROP TABLE t1;
041
=== added file 'Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_898306.test'
--- Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_898306.test 1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/t/percona_innodb_fake_changes_bug_898306.test 2012-01-17 14:13:06 +0000
@@ -0,0 +1,58 @@
1# Test case for Bug #898306: innodb_fake_changes doesn't handle duplicate keys on REPLACE
2# https://bugs.launchpad.net/percona-server/+bug/898306
3# Test ensures that REPLACE statement behaviour respects innodb_fake_changes feature
4--source include/have_innodb_plugin.inc
5
6--disable_warnings
7DROP TABLE IF EXISTS t1;
8--enable_warnings
9
10CREATE TABLE t1 (a INT primary key, b int, unique key (b)) ENGINE=InnoDB;
11INSERT INTO t1 VALUES (1,1);
12
13SET autocommit=1;
14SET innodb_fake_changes=1;
15
16--echo # Confirm that duplicate key errors on REPLACE works
17
18--error ER_ERROR_DURING_COMMIT
19REPLACE INTO t1 VALUES (1,1);
20
21--error ER_ERROR_DURING_COMMIT
22REPLACE INTO t1 VALUES (1,2);
23
24--echo # Confirm that duplicate key errors are OK
25
26BEGIN;
27REPLACE INTO t1 VALUES (1,2);
28SELECT * from t1;
29REPLACE INTO t1 VALUES (1,1);
30SELECT * from t1;
31ROLLBACK;
32
33BEGIN;
34
35--error ER_DUP_ENTRY
36REPLACE INTO t1 VALUES (2,1);
37
38--error ER_DUP_ENTRY
39INSERT INTO t1 VALUES (1,1);
40
41--error ER_DUP_ENTRY
42INSERT INTO t1 VALUES (1,2);
43
44--error ER_DUP_ENTRY
45INSERT INTO t1 VALUES (2,1);
46
47ROLLBACK;
48
49--error ER_ERROR_DURING_COMMIT
50INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=b+10;
51
52--error ER_ERROR_DURING_COMMIT
53INSERT INTO t1 VALUES (1,2) ON DUPLICATE KEY UPDATE b=b+10;
54
55SET innodb_fake_changes=0;
56SELECT * from t1;
57
58DROP TABLE t1;
059
=== modified file 'Percona-Server/sql/handler.h'
--- Percona-Server/sql/handler.h 2011-11-24 02:01:07 +0000
+++ Percona-Server/sql/handler.h 2012-01-17 14:13:06 +0000
@@ -1655,6 +1655,12 @@
16551655
1656 void update_global_table_stats();1656 void update_global_table_stats();
1657 void update_global_index_stats();1657 void update_global_index_stats();
1658
1659 /**
1660 Return true when innodb_fake_changes was set for the current transaction
1661 on this handler
1662 */
1663 virtual my_bool is_fake_change_enabled(THD *thd) { return FALSE; }
16581664
1659#define CHF_CREATE_FLAG 01665#define CHF_CREATE_FLAG 0
1660#define CHF_DELETE_FLAG 11666#define CHF_DELETE_FLAG 1
16611667
=== modified file 'Percona-Server/sql/sql_insert.cc'
--- Percona-Server/sql/sql_insert.cc 2011-11-24 01:59:48 +0000
+++ Percona-Server/sql/sql_insert.cc 2012-01-17 14:13:06 +0000
@@ -1579,6 +1579,15 @@
1579 trg_error= 1;1579 trg_error= 1;
1580 goto ok_or_after_trg_err;1580 goto ok_or_after_trg_err;
1581 }1581 }
1582
1583 /*
1584 Avoid the infinite loop
1585 1) get dup key on fake insert
1586 2) do nothing on fake delete
1587 3) goto #1
1588 */
1589 if (table->file->is_fake_change_enabled(thd))
1590 goto ok_or_after_trg_err;
1582 /* Let us attempt do write_row() once more */1591 /* Let us attempt do write_row() once more */
1583 }1592 }
1584 }1593 }
15851594
=== modified file 'Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc'
--- Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc 2011-11-24 16:33:30 +0000
+++ Percona-Server/storage/innodb_plugin/handler/ha_innodb.cc 2012-01-17 14:13:06 +0000
@@ -795,6 +795,12 @@
795 return(*(trx_t**) thd_ha_data(thd, innodb_hton_ptr));795 return(*(trx_t**) thd_ha_data(thd, innodb_hton_ptr));
796}796}
797797
798my_bool
799ha_innobase::is_fake_change_enabled(THD* thd)
800{
801 trx_t* trx = thd_to_trx(thd);
802 return (trx && trx->fake_changes);
803}
798/********************************************************************//**804/********************************************************************//**
799Call this function when mysqld passes control to the client. That is to805Call this function when mysqld passes control to the client. That is to
800avoid deadlocks on the adaptive hash S-latch possibly held by thd. For more806avoid deadlocks on the adaptive hash S-latch possibly held by thd. For more
801807
=== modified file 'Percona-Server/storage/innodb_plugin/handler/ha_innodb.h'
--- Percona-Server/storage/innodb_plugin/handler/ha_innodb.h 2011-11-24 02:00:38 +0000
+++ Percona-Server/storage/innodb_plugin/handler/ha_innodb.h 2012-01-17 14:13:06 +0000
@@ -136,6 +136,7 @@
136 int close(void);136 int close(void);
137 double scan_time();137 double scan_time();
138 double read_time(uint index, uint ranges, ha_rows rows);138 double read_time(uint index, uint ranges, ha_rows rows);
139 my_bool is_fake_change_enabled(THD *thd);
139 bool is_corrupt() const;140 bool is_corrupt() const;
140141
141 int write_row(uchar * buf);142 int write_row(uchar * buf);

Subscribers

People subscribed via source and target branches