Zend Framework

Mysql does not support nested transactions, code ignitor specifically supports this, zend does not (yet)

Details

  • Type: Improvement Improvement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: Next Major Release
  • Fix Version/s: None
  • Component/s: Zend_Db_Adapter_Mysqli
  • Labels:
    None

Description

I noticed if you nest transactions, if the inner transaction commits but the outer transaction rolls back, the changes take effect and ZF does not throw any exceptions or handle that nicely. It appears from this thread (http://codeigniter.com/forums/viewthread/82771/) that CI has a feature that makes it so only the outer commit/rollback takes effect.

This prevented me from testing code that uses Zend_Db & transactions, because my test itself needed to rollback any changes that happened during the test. But the code I was testing had it's own beginTransaction() and commit()

I guess in code ignitor that inner commit() would be ignored.

Attachments

  1. Text File
    transaction_emulation.patch
    08/Dec/09 3:48 PM
    2 kB
    Josh Ribakoff
  2. Text File
    transaction_emulation2.patch
    08/Dec/09 4:46 PM
    2 kB
    Josh Ribakoff
  3. Text File
    transaction_emulation3.patch
    08/Dec/09 5:27 PM
    2 kB
    Josh Ribakoff

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Josh Ribakoff added a comment -

Possible patch is attached. The $_should_emulate_nesting flag which defaults to false will retain it's current behavior. Setting the flag to true mimics code ignitor's feature (which makes my green bar happy)

Show
Josh Ribakoff added a comment - Possible patch is attached. The $_should_emulate_nesting flag which defaults to false will retain it's current behavior. Setting the flag to true mimics code ignitor's feature (which makes my green bar happy)
Hide
Permalink
Josh Ribakoff added a comment -

Fixed isNested() method (see patch2)

Show
Josh Ribakoff added a comment - Fixed isNested() method (see patch2)
Hide
Permalink
Josh Ribakoff added a comment -

After trying it out in some more unit tests, it appears patch 3 would hold the correct logic.

It should be noted 1 caveat of using this patch, and enabling the flag, would be that extraneous calls to commit would put the transaction counter of sync. But this is good enough for my own development (I wrapped the adapter seeing as you may not desire to use this functionality in the zend core)

Show
Josh Ribakoff added a comment - After trying it out in some more unit tests, it appears patch 3 would hold the correct logic. It should be noted 1 caveat of using this patch, and enabling the flag, would be that extraneous calls to commit would put the transaction counter of sync. But this is good enough for my own development (I wrapped the adapter seeing as you may not desire to use this functionality in the zend core)
Hide
Permalink
Josh Ribakoff added a comment -

On PDO you get an exception when you call beginTransaction() more than once, so I augmented the code like this:

function beginTransaction()
{
$this->_transaction_depth++;
if( $this->_transaction_depth > 0 )

{ return; }

return parent::beginTransaction();
}

Show
Josh Ribakoff added a comment - On PDO you get an exception when you call beginTransaction() more than once, so I augmented the code like this: function beginTransaction() { $this->_transaction_depth++; if( $this->_transaction_depth > 0 ) { return; } return parent::beginTransaction(); }
Hide
Permalink
Josh Ribakoff added a comment -

I should also note, this specific behavior (how it works now) is detrimental to unit testing. If you have a piece of code that uses the database, the standard unit testing practice is to wrap the unit test method in a transaction itself. However if the SUT tries to start it's own transaction, or commit it's "inner" transaction - you either crash your tests - or worse yet the "inner" transaction would be committed, leaving global state for the next unit test method.

As you can see, the way it works now is completely contrary to best practices. (in regards to this one issue - the rest of the component is terrific)

Show
Josh Ribakoff added a comment - I should also note, this specific behavior (how it works now) is detrimental to unit testing. If you have a piece of code that uses the database, the standard unit testing practice is to wrap the unit test method in a transaction itself. However if the SUT tries to start it's own transaction, or commit it's "inner" transaction - you either crash your tests - or worse yet the "inner" transaction would be committed, leaving global state for the next unit test method. As you can see, the way it works now is completely contrary to best practices. (in regards to this one issue - the rest of the component is terrific)
Hide
Permalink
Josh Ribakoff added a comment -

One pitfall to this I discovered over the months, you'd have to make the 'nest' count a public property, and the user should remember to reset it to -1 when catching exceptions.

Show
Josh Ribakoff added a comment - One pitfall to this I discovered over the months, you'd have to make the 'nest' count a public property, and the user should remember to reset it to -1 when catching exceptions.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.