SQLiteConnectionPoolDataSource.getConnection() should return connections from the pool

[tonsky]

Hi! Awesome driver, but I think I’m stuck trying to use connection pool

Describe the bug
SQLiteConnectionPoolDataSource.getConnection() should return connections from the pool. It currently creates new SQLiteConnection on each call.

To Reproduce

var ds = SQLiteConnectionPoolDataSource;
ds.setUrl("jdbc:sqlite:grumpy_data/db.sqlite");

// currently fast, that’s correct
try (var pc = ds.getPooledConnection()) {
  for (int i = 0; i < 10000; ++i) {
    try (var conn = pc.getConnection();
         var stmt = conn.prepareStatement("select addr from datascript where addr = 1");
         var rs   = stmt.executeQuery()) {
      rs.next();
      rs.getLong(1);
    }
  }
}

// currently slow, must be as fast as previous one
for (int i = 0; i < 10000; ++i) {
  try (var conn = ds.getConnection();
       var stmt = conn.prepareStatement("select addr from datascript where addr = 1");
       var rs   = stmt.executeQuery()) {
    rs.next();
    rs.getLong(1);
  }
}

Expected behavior

The idea is that user gets DataSource, calls .getConnection() when it needs one and calls .close() when they are done with it. That’s the contract. Pooled DataSource mimics that behaviour and obeys the same contract, but under the hood doesn’t really closes the connections, and returns live ones from pool as needed. As a user, I shouldn’t be bothered by DataSource implementation, and I shouldn't be going around calling getPooledConnection().getConnection(). I should get them directly from DataSource.

Here’s PostgreSQL driver:

https://github.com/pgjdbc/pgjdbc/blob/0a64f4c7eaaf176fd476ba243f54c2113992f754/pgjdbc/src/main/java/org/postgresql/ds/PGPoolingDataSource.java#L326-L338

Here’s JavaDoc:

https://docs.oracle.com/en/java/javase/21/docs/api/java.sql/javax/sql/package-summary.html

Connection pooling is totally transparent. It is done automatically in the middle tier of a Java EE configuration, so from an application's viewpoint, no change in code is required. An application simply uses the DataSource.getConnection method to get the pooled connection and uses it the same way it uses any Connection object.

https://docs.oracle.com/en/java/javase/21/docs/api/java.sql/javax/sql/PooledConnection.html#getConnection()

When an application calls the method DataSource.getConnection, it gets back a Connection object. If connection pooling is being done, that Connection object is actually a handle to a PooledConnection object, which is a physical connection.

Hope that helps!

[tonsky]

Reading the source, I realized that this happens because results of getPooledConnection().getConnection() share the same physicalConnection, which shouldn’t be the case? If previous one isn’t closed yet, new instance of physicalConn should be created?

Unless I’m reading this all wrong and it’s a colossal misunderstanding. If that’s the case, I apologize in advance

[gotson]

The pgjdbc reference is very much out of date, it's been deprecated in 42.0.0 which is from 2017.

The general idea of JDBC connection pool in every driver is a nice intention, but in the end using a dedicated connection pool library like HikariCP or Commons DBCP is probably a much better choice.

Back to your report, i don't really know how it should be tbh. The code is very old and hasn't been touched in years. Given the above, i would not recommend using this to anyone. There are much better alternatives as mentioned.

[tonsky]

That’s what I ended up doing. Thanks! I leave it open anyway, since it’s technically still an issue?

[gotson]

I'll close it. If someone wants to fix it they can send a PR.