DatabasePopulatorUtils.execute should commit if the current Connection has auto-commit set to false

[sixrandanes]

Hello,

I would like to submit an issue. I am facing a problem with creation of spring-batch tables when you use a datasource in auto-commit mode => false;

My project is using spring-boot@2.2.7 and postgresql.

Here is my configuration :

spring:
  batch:
    initialize-schema: always

spring:
  datasource:
    type: com.zaxxer.hikari.HikariDataSource
    hikari:
      poolName: Hikari
      auto-commit: false

I have checked the BatchDataSourceInitializer class which is the subclass of AbstractDataSourceInitializer.
It appears that the code at line 65 DatabasePopulatorUtils.execute(populator, this.dataSource); is responsible for running the sql script of spring-batch tables, with spring-jdbc, but the transaction is never committed.
And consequently, my tables are never created in my database.

If I switch the configuration of my datasource, ie auto-commit: true, everything is working fine.

I was expecting my spring-batch tables be created (initialize-schema => always) even if my datasource is not in auto-commit mode.
It's not so easy and not very intuitive, at first sight, to discover why the tables are not created. We first think about a misconfiguration relative to spring-batch but nothing related to datasource configuration.

What do you think about that ?

Thanks for your help.

[snicoll]

See this related thread.

I think that DatabasePopulatorUtils#execute should call commit considering it is the one retrieving the connection. Doing so would fix this particular issue since we're calling that internally. paging @jhoeller and @sbrannen for insights.

[snicoll]

After discussing with the framework team, we've decided to fix this in framework itself so I've moved this issue accordingly.

[sbrannen]

This has been addressed in 89c7797 and will be available in 5.3.11 snapshots in a few minutes in case you want to try it out immediately.

Otherwise, you can wait for the 5.3.11 release which is tentatively scheduled for tomorrow.

[eivinhb]

We had an issue with this change that I think might be interesting to note.

In our testrig we use a homegrown LiquibaseDatabasePopulator. We run C3P0 as connection pool. In our populator we have a try-finally and creates a liquibase.database.jvm.JdbcConnection that is passed into Liquibase. When liquibase has done it's job the connection is commited and closed. The type of the actual connection is com.mchange.v2.c3p0.impl.NewProxyConnection and wraps the inner connection. When the connection is closed, this inner connection is then set as null. The implementation of NewProxyConnection.getAutoCommit is so that when getAutoCommit() is called on a closed instance of the connection it throws a NullPointerExecption. And this causes DatabasePopulatorUtils to fail with version 5.3.11 and onwards.
So the code that this issue has added has created a possibility for a new type of error.
For us, the solution is just to drop closing the connection and leave that to DatabasePopulatorUtils.

connection.getAutoCommit() throws NullPointerException if connection closed and C3P0 connection instance.
89c7797#diff-4b698689a0e2f1788f2d8e40e2f6811cc4ac7f305734be0fdc44cf30ed84ec6eR55

I'm not sure if I should report this as a bug or anything particular, but I think i might be good if anyone else come across with the same kind of error. The error says that you cannot operate on a closed connection. But the connection sent into the populate function was not closed.
Maby a updated comment in DatabasePopulator.populate might say something like "do not close the connection".

Caused by: org.springframework.jdbc.datasource.init.UncategorizedScriptException: Failed to execute database script; nested exception is java.sql.SQLException: You can't operate on a closed Connection!!!
	at org.springframework.jdbc.datasource.init.DatabasePopulatorUtils.execute(DatabasePopulatorUtils.java:67)
	at org.springframework.jdbc.datasource.init.DataSourceInitializer.execute(DataSourceInitializer.java:111)
	at org.springframework.jdbc.datasource.init.DataSourceInitializer.afterPropertiesSet(DataSourceInitializer.java:96)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1863)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1800)
	... 84 more
Caused by: java.sql.SQLException: You can't operate on a closed Connection!!!
	at com.mchange.v2.sql.SqlUtils.toSQLException(SqlUtils.java:118)
	at com.mchange.v2.sql.SqlUtils.toSQLException(SqlUtils.java:77)
	at com.mchange.v2.c3p0.impl.NewProxyConnection.getAutoCommit(NewProxyConnection.java:1232)
	at org.springframework.jdbc.datasource.init.DatabasePopulatorUtils.execute(DatabasePopulatorUtils.java:55)
	... 88 more
Caused by: java.lang.NullPointerException
	at com.mchange.v2.c3p0.impl.NewProxyConnection.getAutoCommit(NewProxyConnection.java:1226)
	... 89 more

[sbrannen]

Hi @eivinhb,

Thanks for the feedback.

For us, the solution is just to drop closing the connection and leave that to DatabasePopulatorUtils.

That sounds like the best course of action to me.

Maby a updated comment in DatabasePopulator.populate might say something like "do not close the connection".

Good point.

I added a note on that to the Javadoc in fb312d0.

[eivinhb]

Nice. Thank you for replying. Just to be clear about this. It's closing the connection inside an implemention of DatabasePopulator.populate that cause an error in DatabasePopulatorUtil.

[sbrannen]

Just to be clear about this. It's closing the connection inside an implemention of DatabasePopulator.populate that cause an error in DatabasePopulatorUtil.

Yep, that's the method I added the warning to in fb312d0. Feel free to review that commit.