Mybatis-spring doesn't propagate exceptions up the stack when using batch mode

[GoogleCodeExporter]

What version of the MyBatis are you using?

3.0.3

Please describe the problem.  Unit tests are best!

When using a batch executor type with Mybatis Spring, if there is a problem 
with the batch (e.g. a key violation), the database exception is consumed by 
the transaction manager and is not propagated. It appears to the caller that 
the batch submission succeeded when it did not. The issue is that the executor 
flush() is being called during a commit() inside the transaction manager. The 
exception is merely logged but not propagated by the transaction manager.

I worked around this issue previous to RC3 by creating my own 
SqlSessionTemplate and using execute(). I added an implicit flush() call after 
execute() was complete, so the exception was thrown before the commit rather 
than during the commit.. This workaround is no longer possible with RC3, as 
there is no more execute() method.

What is the expected output? What do you see instead?

The database exception should be wrapped into a Spring database exception, 
thrown, and propagated up the stack. Instead, the exception is consumed and not 
propagated.

Please provide any additional information below.

Original issue reported on code.google.com by brya...@gmail.com on 27 Nov 2010 at 7:12

[GoogleCodeExporter]

[copied from ML]

You are comletely right Bryan. This is not a bug included in RC3, it 
was also in RC2, but you found a workaround :) 

The problem is really important because batches do not even work if 
there is not any problem in the sentence. 

This is caused by the commit order. When executor is not batch, all 
statements are executed in the moment they are called. When a Spring 
commit is started we first commit the connection and then the session. 
But batches work different. Statements are buffered in BatchExecutor 
so no statement is sent to the database until a SqlSession.commit() is 
called. But as we commited the database first, no updates are done at 
all. 

I suppose you was calling a commit over the SqlSession but now commit 
is a no-op. We did that because transaction should never be handled by 
hand or data integrity problems may arise. We was in fact plannig to 
throw an Exception because the user may believe that the commit/ 
rollback is working and it is not. 

This issue is not going to be easy to solve. For the time being simply 
add a dummy select as your last sentence. That will make MyBatis flush 
its buffer and your code will work fine. 

<select id="dummySelect" > 
   select 1 
</select> 

Original comment by eduardo.macarron on 27 Nov 2010 at 10:24

• Changed state: Accepted
• Added labels: Priority-Critical, Component-Spring
• Removed labels: Priority-Low

[GoogleCodeExporter]

Seems that this patch in SqlSessionSynchronization does the job. Its a pitty 
SqlSesion does not have a .flush(). Anyway the result with a commit() is almost 
the same because SpringTransactionManager will not do the jdbc connection 
commit().

        /**
         * {@inheritDoc}
         */
        @Override
        public void beforeCommit(boolean readOnly) {
            if (TransactionSynchronizationManager.isActualTransactionActive() 
                && this.holder.getExecutorType() == ExecutorType.BATCH ) {
                // this will force the executor to flush
                // but no commit will be done because SpringTransactionManager
                // will no-op the commit
                this.holder.getSqlSession().commit(false);
                if (logger.isDebugEnabled()) {
                    logger.debug("SqlSession commited to flush batch before JDBC connection commit");
                }
            }
        }

Original comment by eduardo.macarron on 27 Nov 2010 at 11:02

[GoogleCodeExporter]

I forgot to say that there is a flush method in 
TransactionSynchronizationAdapter but DataSourceTransactionManager never calls 
it. I suppose this is for JPA/Hibernate transaction managers.

Original comment by eduardo.macarron on 27 Nov 2010 at 2:09

[GoogleCodeExporter]

I have taken Eduardo's suggestion and modified SqlSessionUtils so that batches 
should go through before the JDBC connection commits.

Bryan, can you try revision 3294 in the SVN repository and let us know if you 
are still having issues? If SVN isn't feasible, let me know and we can get you 
a patch.

The one known issue with the current changes is that you get a MyBatis 
PersistenceException when a batch statement fails. It is not translated to a 
Spring DataAccessException. Is this critical to fix?

Original comment by hpresnall@gmail.com on 28 Nov 2010 at 4:06

[GoogleCodeExporter]

I tested the change and a PersistenceException is now being thrown, so at least 
I can now detect a batch failure. Really the executor needs to be exposed by 
the SqlSession in mybatis-core. This way, flushStatements() can be called 
explicitly, and the List<BatchResult> can be retrieved. Currently there is no 
way to get the batch results.

Original comment by brya...@gmail.com on 28 Nov 2010 at 5:31

[GoogleCodeExporter]

Just to note, I WAS able to get the batch results prior to RC3, by overriding 
DefaultSqlSession that cached the executor and exposed it, and then 
implementing my own SqlSessionFactory. Now SqlSessionTemplate "hides" the 
actual SqlSession being used, so I cannot access it. Perhaps the 
sqlSessionProxy member could be exposed? Or ideally, SqlSession would expose 
the executor in mybatis-core.

Original comment by brya...@gmail.com on 28 Nov 2010 at 5:37

[GoogleCodeExporter]

Hi Again Bryan. Thanks first for the testing. I have committed the change to 
translate the PersistenceException to a DataAccessException.

I think the retrieving of the batch results is a mybatis-core matter. 
SqlSession could expose the flushStatements so the end user can retrieve the 
batch results. That way the Spring SqlSession will directly expose the feature.

Original comment by eduardo.macarron on 28 Nov 2010 at 8:29

[GoogleCodeExporter]

To make your code work again I would use directly the SqlSessionUtils, have a 
look at the SqlSessionInterceptor#invoke method to do so. That will give you 
the SqlSession you have created with your SqlSessionFactory. 

This code may not work in future versions because you are digging into 
internals but  I cannot think a better way to do it if MyBatis does not expose 
the flushStatements on SqlSession. 

Original comment by eduardo.macarron on 28 Nov 2010 at 11:09

[GoogleCodeExporter]

Also, it would probably be worth discussing what you want to do with the core 
team in the mailing list. I don't know the core code that well, but it seems 
like the intent of the batch code as implemented was that the only data you get 
out of batches was update / insert counts so there was no need to pass back 
anything to clients. Also, note that any selects done in batches are executed 
immediately (along with flushing any outstanding updates), so the code isn't 
really set up for batching selects at all.

Original comment by hpresnall@gmail.com on 28 Nov 2010 at 7:15

[GoogleCodeExporter]

Original comment by eduardo.macarron on 28 Nov 2010 at 8:34

• Changed state: Fixed

[GoogleCodeExporter]

Would you be against exposing the sqlSessionProxy via a getter on 
SqlSessionTemplate?

Original comment by brya...@gmail.com on 28 Nov 2010 at 11:06

[GoogleCodeExporter]

Yes Bryan, I am sorry but I think that will not be a good idea. Our intention 
is not to expose the SqlSessionTemplate at all and we encourage users to use 
mappers as a first option (no dependencies at all) or a SqlSession (no 
dependencies to mybatis-spring). So I suppose you agree with us that that 
sqlSessionProxy is a really internal object that should not be exposed for any 
reason.

Besides I think we would be trying to solve a problem in the wrong place.

I guess you will need a SqlSessionWithFlush that extends the SqlSession 
interface and ads the flushStatements(). Then: a SqlSessionWithFlushImpl, a 
SqlSessionWithFlushFactory and a SqlSessionWithFlush template. Tedious but not 
difficult to code.

Original comment by eduardo.macarron on 29 Nov 2010 at 1:44

[GoogleCodeExporter]

This is not possible unless I totally reimplement SqlSessionTemplate, as 
sqlSessionProxy is private. Perhaps you could make it protected, then I could 
override SqlSessionTemplate and add in a flush() method.

Original comment by brya...@gmail.com on 29 Nov 2010 at 5:09

[GoogleCodeExporter]

I am afraid that even making it protected you will be able to do nothing with 
it. Download the code and try changing it yourself, as it is a dynamic proxy 
you cannot cast it. You need to create a new one that implements another 
interface, so will will end up with a new whole template because as you see the 
template itself is just a wrapper over the proxy.

Original comment by eduardo.macarron on 29 Nov 2010 at 5:40

[GoogleCodeExporter]

Issue 207 has been merged into this issue.

Original comment by eduardo.macarron on 17 Dec 2010 at 1:16

[GoogleCodeExporter]

Original comment by eduardo.macarron on 30 Dec 2010 at 9:00

• Added labels: Target-Release1.0.0

[GoogleCodeExporter]

Original comment by eduardo.macarron on 5 Apr 2012 at 6:28