Merge lp:~jaypipes/drizzle/bug534806 into lp:~drizzle-trunk/drizzle/development

[Jay Pipes]

Fixes bug #534806 (RANDGEN assert in InnobaseEngine::doSetSavepoint()

Manually issue a call to TransactionStorageEngine::startTransaction() inside
TransactionServices::registerResourceForTransaction(). An assert() in the InnoDB
handler was being hit because although startStatement() was properly being
called for an UPDATE ... LIMIT 0, startTransaction() was not called, and because
external_lock() bailed out without committing the transaction, an
assert(conc_state ! TRX_NOT_STARTED) was being hit when a SAVEPOINT A statement
expected the transaction to have been started...

=== modified file 'drizzled/transaction_services.cc'
--- drizzled/transaction_services.cc	2010-03-03 20:21:57 +0000
+++ drizzled/transaction_services.cc	2010-03-11 00:00:46 +0000
@@ -348,7 +348,6 @@
 
   session->server_status|= SERVER_STATUS_IN_TRANS;
 
-
   trans->registerResource(resource_context);
 
   assert(monitored->participatesInSqlTransaction());
@@ -361,6 +360,8 @@
   if (session->transaction.xid_state.xid.is_null())
     session->transaction.xid_state.xid.set(session->getQueryId());
 
+  engine->startTransaction(session, START_TRANS_NO_OPTIONS);
+
   /* Only true if user is executing a BEGIN WORK/START TRANSACTION */
   if (! session->getResourceContext(monitored, 0)->isStarted())
     registerResourceForStatement(session, monitored, engine);
@@ -391,6 +392,8 @@
   if (session->transaction.xid_state.xid.is_null())
     session->transaction.xid_state.xid.set(session->getQueryId());
 
+  engine->startTransaction(session, START_TRANS_NO_OPTIONS);
+
   /* Only true if user is executing a BEGIN WORK/START TRANSACTION */
   if (! session->getResourceContext(monitored, 0)->isStarted())
     registerResourceForStatement(session, monitored, engine, resource_manager);

=== modified file 'plugin/innobase/handler/ha_innodb.cc'
--- plugin/innobase/handler/ha_innodb.cc	2010-03-05 03:08:42 +0000
+++ plugin/innobase/handler/ha_innodb.cc	2010-03-11 00:00:46 +0000
@@ -2323,7 +2323,6 @@
 
 	innobase_release_stat_resources(trx);
 
-	/* TODO: use provided savepoint data area to store savepoint data */
 	error= (int)trx_rollback_to_savepoint_for_mysql(trx, named_savepoint.getName().c_str(),
                                                         &mysql_binlog_cache_pos);
 	return(convert_error_code_to_mysql(error, 0, NULL));
@@ -2347,7 +2346,6 @@
 
 	trx = check_trx_exists(session);
 
-	/* TODO: use provided savepoint data area to store savepoint data */
 	error = (int) trx_release_savepoint_for_mysql(trx, named_savepoint.getName().c_str());
 
 	return(convert_error_code_to_mysql(error, 0, NULL));
@@ -2384,7 +2382,6 @@
 	/* cannot happen outside of transaction */
 	assert(trx->conc_state != TRX_NOT_STARTED);
 
-	/* TODO: use provided savepoint data area to store savepoint data */
 	error = (int) trx_savepoint_for_mysql(trx, named_savepoint.getName().c_str(), (ib_int64_t)0);
 
 	return(convert_error_code_to_mysql(error, 0, NULL));

=== added file 'tests/r/savepoints.result'
--- tests/r/savepoints.result	1970-01-01 00:00:00 +0000
+++ tests/r/savepoints.result	2010-03-11 00:00:46 +0000
@@ -0,0 +1,46 @@
+SET AUTOCOMMIT = 0;
+CREATE TABLE t1 (id INT NOT NULL PRIMARY KEY);
+COMMIT;
+UPDATE t1 SET id = 2 WHERE id != 2 LIMIT 0;
+SAVEPOINT A;
+DROP TABLE t1;
+CREATE TABLE t1 (id INT NOT NULL PRIMARY KEY);
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+SAVEPOINT A;
+INSERT INTO t1 VALUES (2);
+SAVEPOINT B;
+INSERT INTO t1 VALUES (3);
+COMMIT;
+SELECT * FROM t1;
+id
+1
+2
+3
+START TRANSACTION;
+INSERT INTO t1 VALUES (4);
+SAVEPOINT A;
+INSERT INTO t1 VALUES (5);
+SAVEPOINT B;
+INSERT INTO t1 VALUES (6);
+ROLLBACK;
+SELECT * FROM t1;
+id
+1
+2
+3
+START TRANSACTION;
+INSERT INTO t1 VALUES (4);
+SAVEPOINT A;
+INSERT INTO t1 VALUES (5);
+SAVEPOINT B;
+INSERT INTO t1 VALUES (6);
+ROLLBACK TO SAVEPOINT A;
+COMMIT;
+SELECT * FROM t1;
+id
+1
+2
+3
+4
+DROP TABLE t1;

=== added file 'tests/t/savepoints.test'
--- tests/t/savepoints.test	1970-01-01 00:00:00 +0000
+++ tests/t/savepoints.test	2010-03-11 00:00:46 +0000
@@ -0,0 +1,82 @@
+# Tests of various SAVEPOINT functionality
+
+# Test for Bug #534806 - SAVEPOINT without active transaction
+# triggers assert in InnoDB handler
+
+SET AUTOCOMMIT = 0;
+CREATE TABLE t1 (id INT NOT NULL PRIMARY KEY);
+COMMIT;
+UPDATE t1 SET id = 2 WHERE id != 2 LIMIT 0;
+SAVEPOINT A;
+
+DROP TABLE t1;
+
+# Let's test the non-edge case for SAVEPOINTS:
+#
+# Typical usage pattern of starting a transaction, doing
+# some work, savepointing, do more work, savepointing, etc
+# and committing without any rollbacks or savepoint releases.
+
+CREATE TABLE t1 (id INT NOT NULL PRIMARY KEY);
+
+START TRANSACTION;
+
+INSERT INTO t1 VALUES (1);
+
+SAVEPOINT A;
+
+INSERT INTO t1 VALUES (2);
+
+SAVEPOINT B;
+
+INSERT INTO t1 VALUES (3);
+
+COMMIT;
+
+# t1 should now have 1,2,3 in it.
+SELECT * FROM t1;
+
+# We now test another typical usage pattern, similar to above,
+# but we issue a ROLLBACK at the end instead of a COMMIT.  All
+# work done in all savepoints should be rolled back.
+
+START TRANSACTION;
+
+INSERT INTO t1 VALUES (4);
+
+SAVEPOINT A;
+
+INSERT INTO t1 VALUES (5);
+
+SAVEPOINT B;
+
+INSERT INTO t1 VALUES (6);
+
+ROLLBACK;
+
+# t1 should still have 1,2,3 in it.
+SELECT * FROM t1;
+
+# We now test the final typical usage pattern, where we
+# ROLLBACK work to a specific SAVEPOINT and then COMMIT.
+
+START TRANSACTION;
+
+INSERT INTO t1 VALUES (4);
+
+SAVEPOINT A;
+
+INSERT INTO t1 VALUES (5);
+
+SAVEPOINT B;
+
+INSERT INTO t1 VALUES (6);
+
+ROLLBACK TO SAVEPOINT A;
+
+COMMIT;
+
+# t1 should have 1,2,3,4 in it.
+SELECT * FROM t1;
+
+DROP TABLE t1;