Merge lp:~dshrews/drizzle/bug599582 into lp:~drizzle-trunk/drizzle/development

[David Shrewsbury]

This modifies transaction services logging to use the row stored in table->record[1] (which will be the row causing the unique key conflict) to log any DELETEs resulting from a REPLACE statement. This is also the record sent to the storage engine for deleting. Note: This will happen only for REPLACE.

We don't have Field pointers for the update row (table->record[1]), only the insert row (table->record[0]), so I have to "borrow" the value parsing abilities from the insert row Field pointers to get at the value in the update row. This seemed to be least expensive way to do this, and the Field::val_str() calls don't appear to modify anything in Field, so this seems to be safe.

Also added some tests to make sure this functions properly.

[Jay Pipes (jaypipes)]

Very nice work, David. ++ for comments explaining code. :)

review: Approve

=== modified file 'drizzled/cursor.cc'
--- drizzled/cursor.cc	2010-08-09 00:00:11 +0000
+++ drizzled/cursor.cc	2010-08-26 18:28:46 +0000
@@ -1327,7 +1327,7 @@
      * called.  If it fails, then a call to deleteRecord()
      * is called, followed by a repeat of the original
      * call to insertRecord().  So, log_row_for_replication
-     * could be called either once or twice for a REPLACE
+     * could be called multiple times for a REPLACE
      * statement.  The below looks at the values of before_record
      * and after_record to determine which call to this
      * function is for the delete or the insert, since NULL
@@ -1340,7 +1340,12 @@
      */
     if (after_record == NULL)
     {
-      transaction_services.deleteRecord(session, table);
+      /*
+       * The storage engine is passed the record in table->record[1]
+       * as the row to delete (this is the conflicting row), so
+       * we need to notify TransactionService to use that row.
+       */
+      transaction_services.deleteRecord(session, table, true);
       /* 
        * We set the "current" statement message to NULL.  This triggers
        * the replication services component to generate a new statement

=== modified file 'drizzled/transaction_services.cc'
--- drizzled/transaction_services.cc	2010-08-09 00:00:11 +0000
+++ drizzled/transaction_services.cc	2010-08-26 18:28:46 +0000
@@ -1463,7 +1463,7 @@
   }
 }
 
-void TransactionServices::deleteRecord(Session *in_session, Table *in_table)
+void TransactionServices::deleteRecord(Session *in_session, Table *in_table, bool use_update_record)
 {
   ReplicationServices &replication_services= ReplicationServices::singleton();
   if (! replication_services.isActive())
@@ -1490,11 +1490,28 @@
      */
     if (in_table->getShare()->fieldInPrimaryKey(current_field))
     {
-      string_value= current_field->val_str(string_value);
+      if (use_update_record)
+      {
+        /*
+         * Temporarily point to the update record to get its value.
+         * This is pretty much a hack in order to get the PK value from
+         * the update record rather than the insert record. Field::val_str()
+         * should not change anything in Field::ptr, so this should be safe.
+         * We are careful not to change anything in old_ptr.
+         */
+        const unsigned char *old_ptr= current_field->ptr;
+        current_field->ptr= in_table->getUpdateRecord() + static_cast<ptrdiff_t>(old_ptr - in_table->getInsertRecord());
+        string_value= current_field->val_str(string_value);
+        current_field->ptr= const_cast<unsigned char *>(old_ptr);
+      }
+      else
+      {
+        string_value= current_field->val_str(string_value);
+        /**
+         * @TODO Store optional old record value in the before data member
+         */
+      }
       record->add_key_value(string_value->c_ptr(), string_value->length());
-      /**
-       * @TODO Store optional old record value in the before data member
-       */
       string_value->free();
     }
   }

=== modified file 'drizzled/transaction_services.h'
--- drizzled/transaction_services.h	2010-03-31 19:04:12 +0000
+++ drizzled/transaction_services.h	2010-08-26 18:28:46 +0000
@@ -239,10 +239,11 @@
    * Creates a new DeleteRecord GPB message and pushes it to
    * replicators.
    *
-   * @param Pointer to the Session which has deleted a record
-   * @param Pointer to the Table containing delete information
+   * @param in_session Pointer to the Session which has deleted a record
+   * @param in_table Pointer to the Table containing delete information
+   * @param use_update_record If true, uses the values from the update row instead
    */
-  void deleteRecord(Session *in_session, Table *in_table);
+  void deleteRecord(Session *in_session, Table *in_table, bool use_update_record= false);
   /**
    * Creates a CreateSchema Statement GPB message and adds it
    * to the Session's active Transaction GPB message for pushing

=== modified file 'plugin/transaction_log/tests/r/replace.result'
--- plugin/transaction_log/tests/r/replace.result	2010-07-27 19:02:25 +0000
+++ plugin/transaction_log/tests/r/replace.result	2010-08-26 18:28:46 +0000
@@ -20,6 +20,48 @@
 INSERT INTO t1 VALUES (2, "I hate testing.");
 REPLACE INTO t1 VALUE (2, "I love testing.");
 DROP TABLE t2, t1;
+create table t1 (a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values (1,"a",1,1),(2,"b",2,2),(3,"c",3,3);
+replace into t1 values (1,"b",3,4);
+select * from t1 order by a;
+a	b	c	d
+1	b	3	4
+drop table t1;
+create table t1 (a CHAR(5) NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values ("a","a",1,1),("bb","b",2,2),("ccc","c",3,3);
+replace into t1 values ("a","b",3,4);
+select * from t1 order by a;
+a	b	c	d
+a	b	3	4
+drop table t1;
+create table t1 (a DATE NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values ("2010-01-01","a",1,1),("2010-02-02","b",2,2),("2010-03-03","c",3,3);
+replace into t1 values ("2010-01-01","b",3,4);
+select * from t1 order by a;
+a	b	c	d
[phone]	b	3	4
+drop table t1;
+create table t1 (a DOUBLE NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values (1.1,"a",1,1),(22.22,"b",2,2),(333.333,"c",3,3);
+replace into t1 values (1.1,"b",3,4);
+select * from t1 order by a;
+a	b	c	d
+1.1	b	3	4
+drop table t1;
+create table t1 (a FLOAT NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values (1.1,"a",1,1),(22.22,"b",2,2),(333.333,"c",3,3);
+replace into t1 values (1.1,"b",3,4);
+select * from t1 order by a;
+a	b	c	d
+1.1	b	3	4
+drop table t1;
+create table t1 (a ENUM("a","bb","ccc") NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values ("a","a",1,1),("bb","b",2,2),("ccc","c",3,3);
+replace into t1 values ("a","b",3,4);
+select * from t1 order by a;
+a	b	c	d
+a	b	3	4
+drop table t1;
 START TRANSACTION;
 DROP TABLE IF EXISTS `test`.`t1`;
 COMMIT;
@@ -63,4 +105,100 @@
 START TRANSACTION;
 DROP TABLE `test`.`t1`;
 COMMIT;
+START TRANSACTION;
+CREATE TABLE `test`.`t1` (   `a` int NOT NULL AUTO_INCREMENT,   `b` varchar(1) DEFAULT NULL,   `c` int DEFAULT NULL,   `d` int DEFAULT NULL,   PRIMARY KEY (`a`),   UNIQUE KEY `b` (`b`),   UNIQUE KEY `c` (`c`) ) ENGINE=InnoDB COLLATE = utf8_general_ci;
+COMMIT;
+START TRANSACTION;
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (1,'a',1,1);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (2,'b',2,2);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (3,'c',3,3);
+COMMIT;
+START TRANSACTION;
+DELETE FROM `test`.`t1` WHERE `a`=1;
+DELETE FROM `test`.`t1` WHERE `a`=2;
+UPDATE `test`.`t1` SET `a`=1,`b`='b',`d`=4 WHERE `a`=3;
+COMMIT;
+START TRANSACTION;
+DROP TABLE `test`.`t1`;
+COMMIT;
+START TRANSACTION;
+CREATE TABLE `test`.`t1` (   `a` varchar(5) NOT NULL,   `b` varchar(1) DEFAULT NULL,   `c` int DEFAULT NULL,   `d` int DEFAULT NULL,   PRIMARY KEY (`a`),   UNIQUE KEY `b` (`b`),   UNIQUE KEY `c` (`c`) ) ENGINE=InnoDB COLLATE = utf8_general_ci;
+COMMIT;
+START TRANSACTION;
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES ('a','a',1,1);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES ('bb','b',2,2);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES ('ccc','c',3,3);
+COMMIT;
+START TRANSACTION;
+DELETE FROM `test`.`t1` WHERE `a`='a';
+DELETE FROM `test`.`t1` WHERE `a`='bb';
+UPDATE `test`.`t1` SET `a`='a',`b`='b',`d`=4 WHERE `a`='ccc';
+COMMIT;
+START TRANSACTION;
+DROP TABLE `test`.`t1`;
+COMMIT;
+START TRANSACTION;
+CREATE TABLE `test`.`t1` (   `a` date NOT NULL,   `b` varchar(1) DEFAULT NULL,   `c` int DEFAULT NULL,   `d` int DEFAULT NULL,   PRIMARY KEY (`a`),   UNIQUE KEY `b` (`b`),   UNIQUE KEY `c` (`c`) ) ENGINE=InnoDB COLLATE = utf8_general_ci;
+COMMIT;
+START TRANSACTION;
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES ('2010-01-01','a',1,1);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES ('2010-02-02','b',2,2);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES ('2010-03-03','c',3,3);
+COMMIT;
+START TRANSACTION;
+DELETE FROM `test`.`t1` WHERE `a`='2010-01-01';
+DELETE FROM `test`.`t1` WHERE `a`='2010-02-02';
+UPDATE `test`.`t1` SET `a`='2010-01-01',`b`='b',`d`=4 WHERE `a`='2010-03-03';
+COMMIT;
+START TRANSACTION;
+DROP TABLE `test`.`t1`;
+COMMIT;
+START TRANSACTION;
+CREATE TABLE `test`.`t1` (   `a` double NOT NULL,   `b` varchar(1) DEFAULT NULL,   `c` int DEFAULT NULL,   `d` int DEFAULT NULL,   PRIMARY KEY (`a`),   UNIQUE KEY `b` (`b`),   UNIQUE KEY `c` (`c`) ) ENGINE=InnoDB COLLATE = utf8_general_ci;
+COMMIT;
+START TRANSACTION;
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (1.1,'a',1,1);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (22.22,'b',2,2);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (333.333,'c',3,3);
+COMMIT;
+START TRANSACTION;
+DELETE FROM `test`.`t1` WHERE `a`=1.1;
+DELETE FROM `test`.`t1` WHERE `a`=22.22;
+UPDATE `test`.`t1` SET `a`=1.1,`b`='b',`d`=4 WHERE `a`=333.333;
+COMMIT;
+START TRANSACTION;
+DROP TABLE `test`.`t1`;
+COMMIT;
+START TRANSACTION;
+CREATE TABLE `test`.`t1` (   `a` double NOT NULL,   `b` varchar(1) DEFAULT NULL,   `c` int DEFAULT NULL,   `d` int DEFAULT NULL,   PRIMARY KEY (`a`),   UNIQUE KEY `b` (`b`),   UNIQUE KEY `c` (`c`) ) ENGINE=InnoDB COLLATE = utf8_general_ci;
+COMMIT;
+START TRANSACTION;
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (1.1,'a',1,1);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (22.22,'b',2,2);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (333.333,'c',3,3);
+COMMIT;
+START TRANSACTION;
+DELETE FROM `test`.`t1` WHERE `a`=1.1;
+DELETE FROM `test`.`t1` WHERE `a`=22.22;
+UPDATE `test`.`t1` SET `a`=1.1,`b`='b',`d`=4 WHERE `a`=333.333;
+COMMIT;
+START TRANSACTION;
+DROP TABLE `test`.`t1`;
+COMMIT;
+START TRANSACTION;
+CREATE TABLE `test`.`t1` (   `a` enum('a','bb','ccc') NOT NULL,   `b` varchar(1) DEFAULT NULL,   `c` int DEFAULT NULL,   `d` int DEFAULT NULL,   PRIMARY KEY (`a`),   UNIQUE KEY `b` (`b`),   UNIQUE KEY `c` (`c`) ) ENGINE=InnoDB COLLATE = utf8_general_ci;
+COMMIT;
+START TRANSACTION;
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (a,'a',1,1);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (bb,'b',2,2);
+INSERT INTO `test`.`t1` (`a`,`b`,`c`,`d`) VALUES (ccc,'c',3,3);
+COMMIT;
+START TRANSACTION;
+DELETE FROM `test`.`t1` WHERE `a`=a;
+DELETE FROM `test`.`t1` WHERE `a`=bb;
+UPDATE `test`.`t1` SET `a`=a,`b`='b',`d`=4 WHERE `a`=ccc;
+COMMIT;
+START TRANSACTION;
+DROP TABLE `test`.`t1`;
+COMMIT;
 SET GLOBAL transaction_log_truncate_debug= true;

=== modified file 'plugin/transaction_log/tests/t/replace.inc'
--- plugin/transaction_log/tests/t/replace.inc	2009-10-23 16:30:06 +0000
+++ plugin/transaction_log/tests/t/replace.inc	2010-08-26 18:28:46 +0000
@@ -45,3 +45,47 @@
 REPLACE INTO t1 VALUE (2, "I love testing.");
 
 DROP TABLE t2, t1;
+
+#
+# Test when using replace on table with multiple unique keys
+# and a primary key of various data types. This tests code
+# that, during a REPLACE, deletes multiple conflicting rows.
+# (FYI, In the transaction log, there should be 2 DELETEs
+# and 1 UPDATE for each of these REPLACE statements.)
+#
+
+create table t1 (a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values (1,"a",1,1),(2,"b",2,2),(3,"c",3,3);
+replace into t1 values (1,"b",3,4);
+select * from t1 order by a;
+drop table t1;
+
+create table t1 (a CHAR(5) NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values ("a","a",1,1),("bb","b",2,2),("ccc","c",3,3);
+replace into t1 values ("a","b",3,4);
+select * from t1 order by a;
+drop table t1;
+
+create table t1 (a DATE NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values ("2010-01-01","a",1,1),("2010-02-02","b",2,2),("2010-03-03","c",3,3);
+replace into t1 values ("2010-01-01","b",3,4);
+select * from t1 order by a;
+drop table t1;
+
+create table t1 (a DOUBLE NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values (1.1,"a",1,1),(22.22,"b",2,2),(333.333,"c",3,3);
+replace into t1 values (1.1,"b",3,4);
+select * from t1 order by a;
+drop table t1;
+
+create table t1 (a FLOAT NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values (1.1,"a",1,1),(22.22,"b",2,2),(333.333,"c",3,3);
+replace into t1 values (1.1,"b",3,4);
+select * from t1 order by a;
+drop table t1;
+
+create table t1 (a ENUM("a","bb","ccc") NOT NULL PRIMARY KEY, b char(1), c INT, d INT, unique key(b), unique key(c));
+insert into t1 values ("a","a",1,1),("bb","b",2,2),("ccc","c",3,3);
+replace into t1 values ("a","b",3,4);
+select * from t1 order by a;
+drop table t1;