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

[Valentine Gostev]

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

[Valentine Gostev (longbow)]

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

[Oleg Tsarev (tsarev)]

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

[Alexey Kopytov (akopytov)]

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

[Alexey Kopytov (akopytov)]

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

[Alexey Kopytov (akopytov)]

OK to merge.

review: Approve

=== 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 @@
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (a INT primary key, b int, unique key (b)) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1,1);
+SET autocommit=1;
+SET innodb_fake_changes=1;
+# Confirm that duplicate key errors on REPLACE works
+REPLACE INTO t1 VALUES (1,1);
+ERROR HY000: Got error 131 during COMMIT
+REPLACE INTO t1 VALUES (1,2);
+ERROR HY000: Got error 131 during COMMIT
+# Confirm that duplicate key errors are OK
+BEGIN;
+REPLACE INTO t1 VALUES (1,2);
+SELECT * from t1;
+a	b
+1	1
+REPLACE INTO t1 VALUES (1,1);
+SELECT * from t1;
+a	b
+1	1
+ROLLBACK;
+BEGIN;
+REPLACE INTO t1 VALUES (2,1);
+ERROR 23000: Duplicate entry '1' for key 'b'
+INSERT INTO t1 VALUES (1,1);
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+INSERT INTO t1 VALUES (1,2);
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+INSERT INTO t1 VALUES (2,1);
+ERROR 23000: Duplicate entry '1' for key 'b'
+ROLLBACK;
+INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=b+10;
+ERROR HY000: Got error 131 during COMMIT
+INSERT INTO t1 VALUES (1,2) ON DUPLICATE KEY UPDATE b=b+10;
+ERROR HY000: Got error 131 during COMMIT
+SET innodb_fake_changes=0;
+SELECT * from t1;
+a	b
+1	1
+DROP TABLE t1;

=== 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 @@
+# Test case for Bug #898306: innodb_fake_changes doesn't handle duplicate keys on REPLACE
+# https://bugs.launchpad.net/percona-server/+bug/898306
+# Test ensures that REPLACE statement behaviour respects innodb_fake_changes feature
+--source include/have_innodb_plugin.inc
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (a INT primary key, b int, unique key (b)) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1,1);
+
+SET autocommit=1;
+SET innodb_fake_changes=1;
+
+--echo # Confirm that duplicate key errors on REPLACE works
+
+--error ER_ERROR_DURING_COMMIT
+REPLACE INTO t1 VALUES (1,1);
+
+--error ER_ERROR_DURING_COMMIT
+REPLACE INTO t1 VALUES (1,2);
+
+--echo # Confirm that duplicate key errors are OK
+
+BEGIN;
+REPLACE INTO t1 VALUES (1,2);
+SELECT * from t1;
+REPLACE INTO t1 VALUES (1,1);
+SELECT * from t1;
+ROLLBACK;
+
+BEGIN;
+
+--error ER_DUP_ENTRY
+REPLACE INTO t1 VALUES (2,1);
+
+--error ER_DUP_ENTRY
+INSERT INTO t1 VALUES (1,1);
+
+--error ER_DUP_ENTRY
+INSERT INTO t1 VALUES (1,2);
+
+--error ER_DUP_ENTRY
+INSERT INTO t1 VALUES (2,1);
+
+ROLLBACK;
+
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t1 VALUES (1,1) ON DUPLICATE KEY UPDATE b=b+10;
+
+--error ER_ERROR_DURING_COMMIT
+INSERT INTO t1 VALUES (1,2) ON DUPLICATE KEY UPDATE b=b+10;
+
+SET innodb_fake_changes=0;
+SELECT * from t1;
+
+DROP TABLE t1;

=== 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 @@
 
   void update_global_table_stats();
   void update_global_index_stats();
+  
+  /** 
+    Return true when innodb_fake_changes was set for the current transaction
+    on this handler
+  */
+  virtual my_bool is_fake_change_enabled(THD *thd) { return FALSE; }
 
 #define CHF_CREATE_FLAG 0
 #define CHF_DELETE_FLAG 1

=== 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 @@
             trg_error= 1;
             goto ok_or_after_trg_err;
           }
+
+          /* 
+            Avoid the infinite loop
+            1) get dup key on fake insert
+            2) do nothing on fake delete
+            3) goto #1
+          */
+          if (table->file->is_fake_change_enabled(thd))
+            goto ok_or_after_trg_err;
           /* Let us attempt do write_row() once more */
         }
       }

=== 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 @@
 	return(*(trx_t**) thd_ha_data(thd, innodb_hton_ptr));
 }
 
+my_bool
+ha_innobase::is_fake_change_enabled(THD* thd)
+{
+	trx_t*  trx = thd_to_trx(thd);
+	return (trx && trx->fake_changes);
+}
 /********************************************************************//**
 Call this function when mysqld passes control to the client. That is to
 avoid deadlocks on the adaptive hash S-latch possibly held by thd. For more

=== 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 @@
 	int close(void);
 	double scan_time();
 	double read_time(uint index, uint ranges, ha_rows rows);
+	my_bool is_fake_change_enabled(THD *thd);
 	bool is_corrupt() const;
 
 	int write_row(uchar * buf);