Skip to content

Commit 34cbe82

Browse files
authored
Merge pull request #2838 from ClickHouse/04/21/26/add_jdbc_cluster_name
Added cluster name property to JDBC
2 parents 32122d9 + b777125 commit 34cbe82

9 files changed

Lines changed: 117 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
## 0.9.9
22

3+
### New Features
4+
5+
- **[jdbc-v2]** Added `cluster_name` configuration property to specify a target cluster for statements like `KILL QUERY` that require an `ON CLUSTER` clause to execute across all nodes. (https://github.com/ClickHouse/clickhouse-java/issues/2837)
6+
37
### Bug Fixes
48

59
- **[client-v2]** Fixed inconsistent use of `executionTimeout` parameter in `Client` component. The timeout was previously set in milliseconds but mistakenly retrieved and used in seconds in some places. Now it correctly uses milliseconds consistently. (https://github.com/ClickHouse/clickhouse-java/issues/2358)

clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/GenericJDBCTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public void connectionTest() throws SQLException {
3131
}
3232
}
3333

34-
@Test
34+
@Test(enabled = false) // skipped to be removed after reviewing tests.
3535
public void connectionWithPropertiesTest() throws SQLException {
3636
Properties properties = new Properties();
3737
properties.setProperty("user", "default");

docs/ai-review.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ Use this guide across Cursor, Claude, Copilot, and other AI assistants when revi
44

55
This repository contains Java libraries and drivers for ClickHouse, including shared data types, HTTP clients, the v2 client, JDBC drivers, and R2DBC integration.
66

7+
For a reusable review write-up format, use `docs/review-template.md`.
8+
79
## Role
810

911
Act as an experienced maintainer and reviewer of a public Java library.
@@ -169,6 +171,8 @@ When responding to a review request:
169171
3. Use file or symbol references where helpful.
170172
4. Keep any change summary brief and secondary.
171173
5. If no issues are found, say so explicitly and mention residual risk or testing gaps.
174+
6. Use `docs/review-template.md` when a structured review summary is helpful.
175+
7. End with exactly one verdict: `ready to merge`, `ready for human review`, or `need changes`.
172176

173177
In the final summary, state:
174178

docs/features.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ Compatibility-sensitive traits:
4949
- Schema and database context: Supports database selection through URL, `setSchema`, `USE`, and statement-level settings.
5050
- Non-transactional operation: Exposes ClickHouse-appropriate transaction behavior with auto-commit semantics and unsupported transactional features.
5151
- Statement execution: Supports `execute`, `executeQuery`, `executeUpdate`, large update counts, and forward-only/read-only statements.
52-
- Query cancellation and timeout: Supports JDBC query timeout handling and query cancellation through server-side `KILL QUERY`.
52+
- Query cancellation and timeout: Supports JDBC query timeout handling and query cancellation through server-side `KILL QUERY`, with optional JDBC `cluster_name` property support to add `ON CLUSTER '<name>'` for cluster-wide cancellation.
5353
- Batch execution: Supports batched statements and prepared-statement batches, including multi-row rewrite for eligible `INSERT ... VALUES` statements.
5454
- Prepared statements: Supports `?` parameters through client-side SQL rendering and validates that all parameters are bound before execution.
5555
- SQL parsing and classification: Classifies SQL to distinguish queries, updates, inserts, `USE`, and role-changing statements, with selectable parser backends.
@@ -67,6 +67,7 @@ Compatibility-sensitive traits:
6767
Compatibility-sensitive traits:
6868

6969
- Prepared statements are client-side SQL rendering, not server-side prepared statements. Changes to literal encoding or placeholder parsing are externally visible behavior.
70+
- JDBC `cluster_name` is compatibility-sensitive for cancellation behavior: when configured, `Statement.cancel()` issues `KILL QUERY ON CLUSTER '<name>'`, and when omitted it falls back to a local `KILL QUERY`.
7071
- String parameters are escaped with backslash-based escaping: backslashes are doubled and single quotes are backslash-escaped before values are wrapped in single quotes.
7172
- `?` placeholder detection is SQL-aware and should not treat question marks inside quoted strings, quoted identifiers, comments, casts, or similar syntax as bind parameters.
7273
- String-like ClickHouse values have stable JDBC expectations: `String`, `FixedString`, and `Enum` values are returned as strings, while `UUID` is available both as `getString()` and `getObject(..., UUID.class)`.

docs/review-template.md

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Review Template
2+
3+
Use this template when writing a review summary for a pull request or patch.
4+
5+
## Findings
6+
7+
List findings first and order them by severity.
8+
9+
### High
10+
11+
- None.
12+
13+
### Medium
14+
15+
- None.
16+
17+
### Low
18+
19+
- None.
20+
21+
## Human Review Instructions for Important Changes
22+
23+
Ask a human reviewer to inspect important changes directly when the diff touches any of the following:
24+
25+
- public API surface such as public classes, interfaces, methods, constructors, fields, or enums
26+
- behavioral compatibility such as defaults, retries, timeouts, parsing, serialization, formatting, or exception behavior
27+
- JDBC or R2DBC semantics, driver metadata, or type mappings
28+
- documented features in `docs/features.md`
29+
- security-sensitive code, authentication, credentials, or network-facing behavior
30+
- cross-module contracts where the same concept exists in multiple modules
31+
32+
When human review is needed, call out the exact risk and what should be checked, for example:
33+
34+
- verify that no public signature changed
35+
- verify that output format and serialized values are unchanged
36+
- verify that existing callers keep the same default behavior
37+
- verify that focused tests cover compatibility-sensitive paths
38+
- verify whether `docs/features.md` or other docs need updates
39+
40+
## Verdict
41+
42+
Choose exactly one verdict:
43+
44+
- `ready to merge`: no blocking issues found and no additional human sign-off is needed beyond normal review flow
45+
- `ready for human review`: no blocking issues found by AI, but the change is important enough that a human should confirm compatibility, behavior, or product intent
46+
- `need changes`: blocking issues, regressions, compatibility risk, or missing validation were found and should be addressed before merge
47+
48+
## Copy/Paste Template
49+
50+
```md
51+
## Findings
52+
53+
### High
54+
- None.
55+
56+
### Medium
57+
- None.
58+
59+
### Low
60+
- None.
61+
62+
## Human Review Instructions for Important Changes
63+
- None.
64+
65+
## Verdict
66+
`ready to merge`
67+
```

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ public class ConnectionImpl implements Connection, JdbcV2Wrapper {
5252
protected final JdbcConfiguration config;
5353

5454
private boolean closed = false;
55-
protected boolean onCluster;//TODO: Placeholder for cluster support
56-
protected String cluster;
55+
protected final boolean onCluster;
56+
protected final String cluster;
5757
private String catalog;
5858
private String schema;
5959
private String appName;
@@ -74,8 +74,9 @@ public ConnectionImpl(String url, Properties info) throws SQLException {
7474
try {
7575
this.url = url;//Raw URL
7676
this.config = new JdbcConfiguration(url, info);
77-
this.onCluster = false;
78-
this.cluster = null;
77+
final String tmpClusterName = config.getDriverProperty(DriverProperties.CLUSTER_NAME.getKey(), DriverProperties.CLUSTER_NAME.getDefaultValue());
78+
this.cluster = tmpClusterName == null ? null : tmpClusterName.trim();
79+
this.onCluster = this.cluster != null && !this.cluster.isEmpty();
7980
this.appName = "";
8081
this.readOnly = false;
8182
this.holdability = ResultSet.HOLD_CURSORS_OVER_COMMIT;

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ public enum DriverProperties {
124124
@Deprecated
125125
HTTP_CONNECTION_PROVIDER("http_connection_provider", null),
126126

127+
/**
128+
* Define cluster name for statement executions like KILL
129+
*/
130+
CLUSTER_NAME("jdbc_cluster_name", null),
131+
127132
;
128133

129134

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ public void cancel() throws SQLException {
334334
}
335335

336336
try (QueryResponse response = connection.getClient().query(String.format("KILL QUERY%sWHERE query_id = '%s'",
337-
connection.onCluster ? " ON CLUSTER " + connection.cluster + " " : " ",
337+
connection.onCluster ? " ON CLUSTER " + SQLUtils.enquoteIdentifier(connection.cluster, true) + ' ' : ' ',
338338
lastQueryId), connection.getDefaultQuerySettings()).get()){
339339
LOG.debug("Query {} was killed by {}", lastQueryId, response.getQueryId());
340340
} catch (Exception e) {

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.slf4j.Logger;
1010
import org.slf4j.LoggerFactory;
1111
import org.testng.Assert;
12+
import org.testng.SkipException;
1213
import org.testng.annotations.DataProvider;
1314
import org.testng.annotations.Test;
1415

@@ -579,6 +580,33 @@ public void testConnectionExhaustion() throws Exception {
579580
}
580581
}
581582

583+
@Test(groups = {"integration"})
584+
public void testCancelOnCluster() throws Exception {
585+
// Generate non-existing cluster name to cause error
586+
String testCluster = "test_cluster_" + RandomStringUtils.randomAlphanumeric(10);
587+
Properties p = new Properties();
588+
p.setProperty(DriverProperties.CLUSTER_NAME.getKey(), testCluster);
589+
try (Connection conn = getJdbcConnection(p)) {
590+
try (StatementImpl stmt = (StatementImpl) conn.createStatement();
591+
ResultSet rs = stmt.executeQuery("SELECT 1")) { // to fill queryId
592+
try {
593+
stmt.cancel();
594+
fail("Should have thrown an exception for missing cluster");
595+
} catch (SQLException e) {
596+
assertTrue(e.getMessage().contains(testCluster), "Exception should mention the missing cluster");
597+
}
598+
}
599+
}
600+
601+
// no cluster set - check no exception. actual cancelation is tested elsewhere
602+
try (Connection conn = getJdbcConnection()) {
603+
try (StatementImpl stmt = (StatementImpl) conn.createStatement();
604+
ResultSet rs = stmt.executeQuery("SELECT 1")) { // to fill queryId
605+
stmt.cancel();
606+
}
607+
}
608+
}
609+
582610
@Test(groups = {"integration"})
583611
public void testConcurrentCancel() throws Exception {
584612
int maxNumConnections = 3;

0 commit comments

Comments
 (0)