fix(jdbc-v2): throw on closed connection in commit, rollback, getCatalog, getNetworkTimeout, setClientInfo

history/latest/3270.md

@@ -0,0 +1,6 @@
+- **[jdbc-v2]** `Connection#commit`, `rollback`, `getCatalog`, `getNetworkTimeout` and both `setClientInfo` overloads
+  now throw an `SQLException` (`SQLClientInfoException` for `setClientInfo`) with SQLState `08000` connection
+  exception "Connection is closed" after the connection was closed, as the JDBC contract requires. Before, they returned a
+  value or silently did nothing, and with `jdbc_ignore_unsupported_values=true` `commit`/`rollback` did not report the
+  closed connection either. `close()`, `isClosed()` and `isValid()` are unchanged.
+  (https://github.com/ClickHouse/clickhouse-java/issues/3270)

jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java

@@ -41,6 +41,7 @@
 import java.time.Duration;
 import java.time.temporal.ChronoUnit;
 import java.util.Calendar;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Properties;
@@ -284,11 +285,13 @@ public boolean getAutoCommit() throws SQLException {
 
     @Override
     public void commit() throws SQLException {
+        ensureOpen();
         featureManager.unsupportedFeatureThrow("commit()");
     }
 
     @Override
     public void rollback() throws SQLException {
+        ensureOpen();
         featureManager.unsupportedFeatureThrow("rollback()");
     }
 
@@ -302,7 +305,7 @@ public void close() throws SQLException {
     }
 
     @Override
-    public boolean isClosed() throws SQLException {
+    public boolean isClosed() {
         return closed;
     }
 
@@ -335,6 +338,7 @@ public void setCatalog(String catalog) throws SQLException {
 
     @Override
     public String getCatalog() throws SQLException {
+        ensureOpen();
         return catalog;
     }
 
@@ -574,6 +578,7 @@ public boolean isValid(int timeout) throws SQLException {
 
     @Override
     public void setClientInfo(String name, String value) throws SQLClientInfoException {
+        ensureOpenForClientInfo();
         if (ClientInfoProperties.APPLICATION_NAME.getKey().equals(name)) {
             config.updateUserClient(value, client);
             appName = value;
@@ -583,6 +588,7 @@ public void setClientInfo(String name, String value) throws SQLClientInfoExcepti
 
     @Override
     public void setClientInfo(Properties properties) throws SQLClientInfoException {
+        ensureOpenForClientInfo();
         Set<String> toSet = new HashSet<>();
         Set<String> toReset = new HashSet<>();
         for (ClientInfoProperties p : ClientInfoProperties .values()) {
@@ -735,6 +741,7 @@ public synchronized void onNetworkTimeout() {
 
     @Override
     public int getNetworkTimeout() throws SQLException {
+        ensureOpen();
         return defaultQuerySettings.getNetworkTimeout().intValue();
     }
 
@@ -754,6 +761,13 @@ public Client getClient() throws SQLException {
         return client;
     }
 
+    private void ensureOpenForClientInfo() throws SQLClientInfoException {
+        if (isClosed()) {
+            throw new SQLClientInfoException("Connection is closed",
+                    ExceptionUtils.SQL_STATE_CONNECTION_EXCEPTION, Collections.emptyMap());
+        }
+    }
+
     private void ensureOpen() throws SQLException {
         if (isClosed()) {
             throw new SQLException("Connection is closed", ExceptionUtils.SQL_STATE_CONNECTION_EXCEPTION);

jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java

@@ -33,6 +33,7 @@
 import java.sql.JDBCType;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
+import java.sql.SQLClientInfoException;
 import java.sql.SQLException;
 import java.sql.SQLFeatureNotSupportedException;
 import java.sql.Statement;
@@ -178,6 +179,28 @@ public void testCommitRollback() throws SQLException {
     }
 
 
+    @Test(groups = { "integration" })
+    public void testClosedConnectionThrows() throws SQLException {
+        for (boolean ignoreUnsupported : new boolean[] { false, true }) {
+            Properties prop = new Properties();
+            prop.setProperty(DriverProperties.IGNORE_UNSUPPORTED_VALUES.getKey(), String.valueOf(ignoreUnsupported));
+            Connection conn = this.getJdbcConnection(prop);
+            conn.close();
+
+            assertThrows(SQLException.class, conn::getNetworkTimeout);
+            assertThrows(SQLException.class, conn::getCatalog);
+            assertThrows(SQLException.class, conn::commit);
+            assertThrows(SQLException.class, conn::rollback);
+            assertThrows(SQLClientInfoException.class, () -> conn.setClientInfo("ApplicationName", "x"));
+            assertThrows(SQLClientInfoException.class, () -> conn.setClientInfo(new Properties()));
+
+            // these stay allowed on a closed connection
+            conn.close();
+            Assert.assertTrue(conn.isClosed());
+            Assert.assertFalse(conn.isValid(1));
+        }
+    }
+
     @Test(groups = { "integration" })
     public void closeTest() throws SQLException {
         Connection localConnection = this.getJdbcConnection();