Bug 56586 - initSQL should be committed if defaultAutoCommit == false
Summary: initSQL should be committed if defaultAutoCommit == false
Status: NEW
Alias: None
Product: Tomcat Modules
Classification: Unclassified
Component: jdbc-pool (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
Depends on:
Reported: 2014-06-02 20:25 UTC by Craig Servin
Modified: 2014-07-25 01:40 UTC (History)
0 users


Note You need to log in before you can comment on or make changes to this bug.
Description Craig Servin 2014-06-02 20:25:50 UTC
The initSQL is meant to initialize the JDBC connections.

If defaultAutoCommit == false and rollbackOnReturn == true the connection initialization will be rolled back causing the initialization to be lost for that connection.

This could be fixed by inserting:

if ( !connection.getAutoCommit() ) connection.commit();

after line 375 in PooledConnection.java

Sorry for the lack of a proper diff, I have not built tomcat-jdbc from source.
Comment 1 Christopher Schultz 2014-06-05 00:30:07 UTC
Are you executing DML statements in your initSQL?

Are there drivers who allow rollbacks on session state changes?


Note that you could always use a stored procedure that contains a COMMIT.
Comment 2 Craig Servin 2014-06-05 13:58:58 UTC
I am using postgresql and the problem originally showed up when the initSQL was executing a set search_path and a rollback was triggered by the first transaction.  To test/recreate the issue I did trigger the same problem with DML( an insert into a temp table, which then is rolled back).

I was able to "fix" the issue in my case by adding a ";commit" to the end of my actual initSQL, although this makes me nervous since you are supposed to use the Connections commit() method when using JDBC.

That solution seamed a little bit hackish to me so I posted this ticket.
Comment 3 Christopher Schultz 2014-06-05 14:55:45 UTC
Honestly, I'm surprised that "SET search_path [..]" will be affected by a rollback.

I know it's no basis for comparison, but MySQL does not appear to behave this way:

mysql> begin;
Query OK, 0 rows affected (0.01 sec)

mysql> set @foo := 'bar';
Query OK, 0 rows affected (0.00 sec)

mysql> select @foo;
| @foo |
| bar  |
1 row in set (0.00 sec)

mysql> rollback;
Query OK, 0 rows affected (0.00 sec)

mysql> select @foo;
| @foo |
| bar  |
1 row in set (0.00 sec)

I think this should be an /option/ that defaults to false because it will execute another query (COMMIT) on the server side. If most clients do not need it, there's no need to execute a COMMIT unless the application absolutely needs it.

Any objections to an option rather than strict behavior?
Comment 4 Craig Servin 2014-06-05 15:14:18 UTC
It just seems like the connection initialization should persist for the life of the connection.  If I have some DML that initializes a connection, and it is returned to the pool, when I use that connection again it should still be initialized.  In this case it wasn't, and it became apparent when the wrong data was returned.

Adding the ";commit" worked as a solution for me it was just interesting to track down because I expected the initSQL to be permanent for the life of the connection.

Since different people will have different initSQL commiting the initSQL seems correct to me, but I have know idea what most people would expect.

No objection to option, default, or leave it as is.  I just wanted to let people know what I found.
Comment 5 Wes Clark 2014-07-24 15:14:59 UTC
I agree with the filer.  Alternately, turn on autocommit when running the initialization SQL, then if configured to be off, turn it back off.
Comment 6 Filip Hanik 2014-07-24 22:18:07 UTC
Thanks for the report gentlemen. Glad there is a workaround, the added ";commit".

If it did do 

if ( !connection.getAutoCommit() ) connection.commit();

what are the implications for other users?
Comment 7 Craig Servin 2014-07-25 01:40:39 UTC
I can't think of any issues with that unless someone was intentionally trying to lose their initSQL later on.