From dde7793e17564ca2b7d0fe35932b11b710aaab6b Mon Sep 17 00:00:00 2001 From: Felix Mueller Date: Thu, 10 Jul 2025 09:08:40 +0200 Subject: [PATCH 1/3] Implement Connection#isValid --- .../com/clickhouse/jdbc/ConnectionImpl.java | 9 +++--- .../com/clickhouse/jdbc/ConnectionTest.java | 28 +++++++++++++++++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java index 10fafcae3..5b8240181 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java @@ -15,6 +15,7 @@ import com.clickhouse.jdbc.internal.ParsedPreparedStatement; import com.clickhouse.jdbc.internal.SqlParser; import com.clickhouse.jdbc.metadata.DatabaseMetaDataImpl; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -487,13 +488,13 @@ public SQLXML createSQLXML() throws SQLException { @Override public boolean isValid(int timeout) throws SQLException { - checkOpen(); if (timeout < 0) { throw new SQLException("Timeout must be >= 0", ExceptionUtils.SQL_STATE_CLIENT_ERROR); } - - //TODO: This is a placeholder implementation - return true; + if (isClosed()) { + return false; + } + return client.ping(timeout * 1_000); } @Override diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java index f26d40d9d..8ae0d27a1 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java @@ -13,6 +13,7 @@ import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.common.ConsoleNotifier; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; + import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -34,7 +35,6 @@ import static org.testng.Assert.assertThrows; import static org.testng.Assert.fail; - public class ConnectionTest extends JdbcIntegrationTest { @Test(groups = { "integration" }, enabled = false) @@ -569,8 +569,32 @@ public void testConnectionWithValidDatabaseName(String dbName) throws Exception connCheck.close(); } + @Test(groups = { "integration" }) + public void closedConnectionIsInvalid() throws Exception { + if (isCloud()) { + return; + } + Connection connection = this.getJdbcConnection(); + Assert.assertTrue(connection.isValid(3)); + connection.close(); + Assert.assertFalse(connection.isValid(3)); + } + + @Test(groups = { "integration" }) + public void connectionWithWrongCredentialsIsInvalid() throws Exception { + if (isCloud()) { + return; + } + Connection connection = this.getJdbcConnection(); + Assert.assertTrue(connection.isValid(3)); + Properties properties = new Properties(); + properties.put("password", "invalid"); + connection = this.getJdbcConnection(properties); + Assert.assertFalse(connection.isValid(3)); + } + @DataProvider(name = "validDatabaseNames") - public Object[][] createValidDatabaseNames() { + private static Object[][] createValidDatabaseNames() { return new Object[][] { { "foo" }, { "with-dashes" }, From 32e7a43e0d1f051318fbcf95304d7f4365c12938 Mon Sep 17 00:00:00 2001 From: Felix Mueller Date: Thu, 10 Jul 2025 17:20:26 +0200 Subject: [PATCH 2/3] Convert timeout value using API --- jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java index 5b8240181..d044633f9 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java @@ -36,6 +36,7 @@ import java.sql.Savepoint; import java.sql.Statement; import java.sql.Struct; +import java.time.Duration; import java.util.Arrays; import java.util.Calendar; import java.util.Collections; @@ -494,7 +495,7 @@ public boolean isValid(int timeout) throws SQLException { if (isClosed()) { return false; } - return client.ping(timeout * 1_000); + return client.ping(Duration.ofSeconds(timeout).toMillis()); } @Override From 6af47f42fef32be0865e9956cb048e247fd2ec69 Mon Sep 17 00:00:00 2001 From: Felix Mueller Date: Thu, 10 Jul 2025 17:39:28 +0200 Subject: [PATCH 3/3] handle 0 timeout --- jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java index d044633f9..cdc8ffb06 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java @@ -495,7 +495,9 @@ public boolean isValid(int timeout) throws SQLException { if (isClosed()) { return false; } - return client.ping(Duration.ofSeconds(timeout).toMillis()); + return timeout == 0 + ? client.ping() + : client.ping(Duration.ofSeconds(timeout).toMillis()); } @Override