Merge lp:~stewart/drizzle/embedded-innodb-isolation-level into lp:~drizzle-trunk/drizzle/development

[Stewart Smith]

supports isolation levels in the proper way.

https://bugs.launchpad.net/drizzle/+bug/587772

means that READ COMMITTED doesn't work (a server problem, not embedded innodb)

[Jay Pipes (jaypipes)]

Hi!

I think a static_cast<> is the correct cast template to use here instead of the reinterpret_cast<>:

53 + reinterpret_cast<EmbeddedInnoDBEngine*>(getEngine())->
54 + doStartTransaction(session, START_TRANS_NO_OPTIONS);

since EmbeddedInnoDBEngine is a direct child of plugin::StorageEngine and forward casting should use static_cast<>.

Other than that, great work and good test cases, Stew :)

-jay

review: Needs Fixing

[Monty Taylor (mordred)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/31/2010 02:25 AM, Stewart Smith wrote:
> - ib_trx_t *transaction= get_trx(session);
> - *transaction= ib_trx_begin(IB_TRX_REPEATABLE_READ);
> + reinterpret_cast<EmbeddedInnoDBEngine*>(getEngine())->
> + doStartTransaction(session, START_TRANS_NO_OPTIONS);

Is the reinterpret_cast really needed here? Seems like
getEngine()->doStartTransaction(session, START_TRANS_NO_OPTIONS); should
just work and that the virtual path should take care of getting it back
to EmbeddedInnoDBEngine for you.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwD4/8ACgkQ2Jv7/VK1RgFvWACffvA0riO/9q2uCFPXcGKMbqYw
yzkAn2wYxU06Ox9yWqJJ+I0teS7XrWZJ
=VJK0
-----END PGP SIGNATURE-----

[Jay Pipes (jaypipes)]

> Is the reinterpret_cast really needed here?

haha. beat you to it ;)

:)
-jay

[Jay Pipes (jaypipes)]

And a static_cast<> *is* needed because getEngine() returns plugin::StorageEngine and doStartTransaction() is a method of plugin::TransactionalStorageEngine.

So, the code should/could be:

static_cast<plugin::TransactionalStorageEngine *>(getEngine())->doStartTransaction(session, START_TRANS_NO_OPTIONS);

Cheers!
jay

[Stewart Smith (stewart)]

> Is the reinterpret_cast really needed here? Seems like
> getEngine()->doStartTransaction(session, START_TRANS_NO_OPTIONS); should
> just work and that the virtual path should take care of getting it back
> to EmbeddedInnoDBEngine for you.

But getEngine() returns StorageEngine, not TransactionalStorageEngine.

[Stewart Smith (stewart)]

> And a static_cast<> *is* needed because getEngine() returns
> plugin::StorageEngine and doStartTransaction() is a method of
> plugin::TransactionalStorageEngine.
>
> So, the code should/could be:
>
> static_cast<plugin::TransactionalStorageEngine
> *>(getEngine())->doStartTransaction(session, START_TRANS_NO_OPTIONS);

done. thanks!

[Jay Pipes (jaypipes)]

w00t. :)

review: Approve

[Brian Aker (brianaker)]

Hi!

This needs to be remerged with trunk.

Cheers,
   -Brian

review: Needs Fixing

[Stewart Smith (stewart)]

merged with trunk (at least last nights trunk)

=== modified file 'plugin/embedded_innodb/embedded_innodb_engine.cc'
--- plugin/embedded_innodb/embedded_innodb_engine.cc	2010-05-28 22:08:11 +0000
+++ plugin/embedded_innodb/embedded_innodb_engine.cc	2010-06-02 23:17:25 +0000
@@ -25,7 +25,7 @@
 Copyright (c) 2008, 2009 Google Inc.
 
 Portions of this file contain modifications contributed and copyrighted by
-staticGoogle, Inc. Those modifications are gratefully acknowledged and are described
+Google, Inc. Those modifications are gratefully acknowledged and are described
 briefly in the InnoDB documentation. The contributions by Google are
 incorporated with their permission, and subject to the conditions contained in
 the file COPYING.Google.
@@ -225,15 +225,32 @@
   return (ib_trx_t*) session->getEngineData(embedded_innodb_engine);
 }
 
+static ib_trx_level_t tx_isolation_to_ib_trx_level(enum_tx_isolation level)
+{
+  switch(level)
+  {
+  case ISO_REPEATABLE_READ:
+    return IB_TRX_REPEATABLE_READ;
+  case ISO_READ_COMMITTED:
+    return IB_TRX_READ_COMMITTED;
+  case ISO_SERIALIZABLE:
+    return IB_TRX_SERIALIZABLE;
+  case ISO_READ_UNCOMMITTED:
+    return IB_TRX_READ_UNCOMMITTED;
+  }
+}
+
 int EmbeddedInnoDBEngine::doStartTransaction(Session *session,
                                              start_transaction_option_t options)
 {
   ib_trx_t *transaction;
+  ib_trx_level_t isolation_level;
 
   (void)options;
 
   transaction= get_trx(session);
-  *transaction= ib_trx_begin(IB_TRX_REPEATABLE_READ);
+  isolation_level= tx_isolation_to_ib_trx_level((enum_tx_isolation)session_tx_isolation(session));
+  *transaction= ib_trx_begin(isolation_level);
 
   return 0;
 }
@@ -512,8 +529,8 @@
 
   if(*get_trx(session) == NULL)
   {
-    ib_trx_t *transaction= get_trx(session);
-    *transaction= ib_trx_begin(IB_TRX_REPEATABLE_READ);
+    static_cast<EmbeddedInnoDBEngine*>(getEngine())->
+                    doStartTransaction(session, START_TRANS_NO_OPTIONS);
   }
 
   if (lock_type != TL_UNLOCK)
@@ -639,6 +656,16 @@
   return 0;
 }
 
+int EmbeddedInnoDBCursor::external_lock(Session* session, int lock_type)
+{
+  ib_cursor_stmt_begin(cursor);
+
+  (void)session;
+  (void)lock_type;
+
+  return 0;
+}
+
 static int create_table_add_field(ib_tbl_sch_t schema,
                                   const message::Table::Field &field,
                                   ib_err_t *err)

=== modified file 'plugin/embedded_innodb/embedded_innodb_engine.h'
--- plugin/embedded_innodb/embedded_innodb_engine.h	2010-05-26 13:14:09 +0000
+++ plugin/embedded_innodb/embedded_innodb_engine.h	2010-06-02 23:17:25 +0000
@@ -51,6 +51,7 @@
   uint32_t index_flags(uint32_t inx) const;
   int open(const char *name, int mode, uint32_t test_if_locked);
   int close(void);
+  int external_lock(drizzled::Session* session, int lock_type);
   int doInsertRecord(unsigned char * buf);
   int doStartTableScan(bool scan);
   int rnd_next(unsigned char *buf);

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_read_uncommitted.result'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_read_uncommitted.result	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_read_uncommitted.result	2010-06-02 23:17:25 +0000
@@ -0,0 +1,32 @@
+CREATE TABLE t1 (a int);
+SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+BEGIN;
+INSERT INTO t1 values (1);
+SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+BEGIN;
+SELECT * from t1 order by a;
+a
+1
+INSERT INTO t1 values (2);
+SELECT * from t1 order by a;
+a
+1
+2
+COMMIT;
+BEGIN;
+SELECT * FROM t1 order by a;
+a
+1
+2
+COMMIT;
+SELECT * from t1 order by a;
+a
+1
+2
+COMMIT;
+BEGIN;
+SELECT * from t1 order by a;
+a
+1
+2
+DROP TABLE t1;

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_repeatable_read.result'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_repeatable_read.result	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_repeatable_read.result	2010-06-02 23:17:25 +0000
@@ -0,0 +1,25 @@
+CREATE TABLE t1 (a int);
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+INSERT INTO t1 values (1);
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+SELECT * from t1;
+a
+INSERT INTO t1 values (2);
+SELECT * from t1;
+a
+COMMIT;
+BEGIN;
+SELECT * FROM t1;
+a
+COMMIT;
+SELECT * from t1;
+a
+COMMIT;
+BEGIN;
+SELECT * from t1 order by a;
+a
+1
+2
+DROP TABLE t1;

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_serializable.result'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_serializable.result	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/r/trx_isolation_serializable.result	2010-06-02 23:17:25 +0000
@@ -0,0 +1,25 @@
+CREATE TABLE t1 (a int);
+SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE;
+BEGIN;
+INSERT INTO t1 values (1);
+SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE;
+BEGIN;
+SELECT * from t1;
+a
+INSERT INTO t1 values (2);
+SELECT * from t1;
+a
+COMMIT;
+BEGIN;
+SELECT * FROM t1;
+a
+COMMIT;
+SELECT * from t1;
+a
+COMMIT;
+BEGIN;
+SELECT * from t1 order by a;
+a
+1
+2
+DROP TABLE t1;

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_read_uncommitted-master.opt'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_read_uncommitted-master.opt	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_read_uncommitted-master.opt	2010-06-02 23:17:25 +0000
@@ -0,0 +1,1 @@
+--plugin_add=embedded_innodb --plugin_remove=innobase

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_read_uncommitted.test'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_read_uncommitted.test	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_read_uncommitted.test	2010-06-02 23:17:25 +0000
@@ -0,0 +1,37 @@
+connect (a,localhost,root,,);
+connect (b,localhost,root,,);
+
+connection a;
+CREATE TABLE t1 (a int);
+SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+BEGIN;
+INSERT INTO t1 values (1);
+
+connection b;
+SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+BEGIN;
+SELECT * from t1 order by a;
+
+connection a;
+INSERT INTO t1 values (2);
+
+connection b;
+SELECT * from t1 order by a;
+COMMIT;
+BEGIN;
+SELECT * FROM t1 order by a;
+
+connection a;
+COMMIT;
+
+connection b;
+SELECT * from t1 order by a;
+COMMIT;
+BEGIN;
+SELECT * from t1 order by a;
+
+DROP TABLE t1;
+
+disconnect a;
+disconnect b;
+connection default;

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_repeatable_read-master.opt'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_repeatable_read-master.opt	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_repeatable_read-master.opt	2010-06-02 23:17:25 +0000
@@ -0,0 +1,1 @@
+--plugin_add=embedded_innodb --plugin_remove=innobase

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_repeatable_read.test'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_repeatable_read.test	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_repeatable_read.test	2010-06-02 23:17:25 +0000
@@ -0,0 +1,37 @@
+connect (a,localhost,root,,);
+connect (b,localhost,root,,);
+
+connection a;
+CREATE TABLE t1 (a int);
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+INSERT INTO t1 values (1);
+
+connection b;
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+SELECT * from t1;
+
+connection a;
+INSERT INTO t1 values (2);
+
+connection b;
+SELECT * from t1;
+COMMIT;
+BEGIN;
+SELECT * FROM t1;
+
+connection a;
+COMMIT;
+
+connection b;
+SELECT * from t1;
+COMMIT;
+BEGIN;
+SELECT * from t1 order by a;
+
+DROP TABLE t1;
+
+disconnect a;
+disconnect b;
+connection default;

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_serializable-master.opt'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_serializable-master.opt	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_serializable-master.opt	2010-06-02 23:17:25 +0000
@@ -0,0 +1,1 @@
+--plugin_add=embedded_innodb --plugin_remove=innobase

=== added file 'plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_serializable.test'
--- plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_serializable.test	1970-01-01 00:00:00 +0000
+++ plugin/embedded_innodb/test-suite-dir/embedded_innodb/tests/t/trx_isolation_serializable.test	2010-06-02 23:17:25 +0000
@@ -0,0 +1,37 @@
+connect (a,localhost,root,,);
+connect (b,localhost,root,,);
+
+connection a;
+CREATE TABLE t1 (a int);
+SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE;
+BEGIN;
+INSERT INTO t1 values (1);
+
+connection b;
+SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE;
+BEGIN;
+SELECT * from t1;
+
+connection a;
+INSERT INTO t1 values (2);
+
+connection b;
+SELECT * from t1;
+COMMIT;
+BEGIN;
+SELECT * FROM t1;
+
+connection a;
+COMMIT;
+
+connection b;
+SELECT * from t1;
+COMMIT;
+BEGIN;
+SELECT * from t1 order by a;
+
+DROP TABLE t1;
+
+disconnect a;
+disconnect b;
+connection default;