[jdbc-v2,client-v2] by chernser · Pull Request #2842 · ClickHouse/clickhouse-java

chernser · 2026-04-22T06:05:41Z

Summary

Requests Async operation when query timeout is set
Adds test to verify that timeout is working

Closes #2637

Checklist

Delete items not relevant to your PR:

Closes #
Unit and integration tests covering the common scenarios were added
A human-readable description of the changes was provided to include in CHANGELOG
For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

…rations

sonarqubecloud · 2026-04-22T06:18:16Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2026-04-22T06:18:18Z

Client V2 Coverage

Coverage Report

Package	Coverage	Lines Covered	Total Lines
com.clickhouse.client.api	82.68%	912	1103
com.clickhouse.client.api.command	46.43%	13	28
com.clickhouse.client.api.data_formats	40.29%	141	350
com.clickhouse.client.api.data_formats.internal	74.17%	1645	2218
com.clickhouse.client.api.enums	100.00%	5	5
com.clickhouse.client.api.http	0.00%		1
com.clickhouse.client.api.insert	85.15%	86	101
com.clickhouse.client.api.internal	81.03%	974	1202
com.clickhouse.client.api.metadata	90.74%	49	54
com.clickhouse.client.api.metrics	93.75%	75	80
com.clickhouse.client.api.query	78.88%	127	161
com.clickhouse.client.api.serde	84.21%	48	57
com.clickhouse.client.api.sql	87.50%	28	32
com.clickhouse.client.api.transport	89.29%	50	56

Class Coverage

Class	Coverage	Lines Covered	Total Lines
com.clickhouse.client.api.ClickHouseException	85.71%	12	14
com.clickhouse.client.api.Client	84.72%	377	445
com.clickhouse.client.api.Client.Builder	80.37%	176	219
com.clickhouse.client.api.Client.new DataStreamWriter() {...}	100.00%	8	8
com.clickhouse.client.api.ClientConfigProperties	93.37%	183	196
com.clickhouse.client.api.ClientConfigProperties.new ClientConfigProperties() {...}	100.00%	8	8
com.clickhouse.client.api.ClientException	100.00%	4	4
com.clickhouse.client.api.ClientFaultCause	100.00%	7	7
com.clickhouse.client.api.ClientMisconfigurationException	50.00%	2	4
com.clickhouse.client.api.command.CommandResponse	47.06%	8	17
com.clickhouse.client.api.command.CommandSettings	45.45%	5	11
com.clickhouse.client.api.ConnectionInitiationException	50.00%	3	6
com.clickhouse.client.api.ConnectionReuseStrategy	100.00%	3	3
com.clickhouse.client.api.data_formats.internal.AbstractBinaryFormatReader	72.35%	293	405
com.clickhouse.client.api.data_formats.internal.AbstractBinaryFormatReader.RecordWrapper	50.00%	17	34
com.clickhouse.client.api.data_formats.internal.BinaryReaderBackedRecord	14.77%	13	88
com.clickhouse.client.api.data_formats.internal.BinaryStreamReader	84.63%	369	436
com.clickhouse.client.api.data_formats.internal.BinaryStreamReader.ArrayValue	81.40%	35	43
com.clickhouse.client.api.data_formats.internal.BinaryStreamReader.CachingByteBufferAllocator	100.00%	8	8
com.clickhouse.client.api.data_formats.internal.BinaryStreamReader.DefaultByteBufferAllocator	100.00%	2	2
com.clickhouse.client.api.data_formats.internal.BinaryStreamReader.EnumValue	80.00%	8	10
com.clickhouse.client.api.data_formats.internal.InetAddressConverter	66.67%	18	27
com.clickhouse.client.api.data_formats.internal.MapBackedRecord	45.54%	102	224
com.clickhouse.client.api.data_formats.internal.NumberConverter	88.17%	82	93
com.clickhouse.client.api.data_formats.internal.NumberConverter.NumberType	100.00%	7	7
com.clickhouse.client.api.data_formats.internal.ProcessParser	82.50%	33	40
com.clickhouse.client.api.data_formats.internal.SerializerUtils	82.82%	569	687
com.clickhouse.client.api.data_formats.internal.SerializerUtils.DynamicClassLoader	100.00%	3	3
com.clickhouse.client.api.data_formats.internal.ValueConverters	77.48%	86	111
com.clickhouse.client.api.data_formats.NativeFormatReader	80.77%	42	52
com.clickhouse.client.api.data_formats.NativeFormatReader.Block	66.67%	12	18
com.clickhouse.client.api.data_formats.RowBinaryFormatReader	15.79%	3	19
com.clickhouse.client.api.data_formats.RowBinaryFormatSerializer	19.82%	22	111
com.clickhouse.client.api.data_formats.RowBinaryFormatWriter	27.84%	27	97
com.clickhouse.client.api.data_formats.RowBinaryFormatWriter.InputStreamHolder	0.00%		4
com.clickhouse.client.api.data_formats.RowBinaryFormatWriter.ReaderHolder	0.00%		4
com.clickhouse.client.api.data_formats.RowBinaryWithNamesAndTypesFormatReader	100.00%	22	22
com.clickhouse.client.api.data_formats.RowBinaryWithNamesFormatReader	56.52%	13	23
com.clickhouse.client.api.DataStreamWriter	0.00%		1
com.clickhouse.client.api.DataTransferException	50.00%	2	4
com.clickhouse.client.api.DataTypeUtils	52.50%	63	120
com.clickhouse.client.api.enums.Protocol	100.00%	2	2
com.clickhouse.client.api.enums.ProxyType	100.00%	3	3
com.clickhouse.client.api.http.ClickHouseHttpProto	0.00%		1
com.clickhouse.client.api.insert.InsertResponse	52.94%	9	17
com.clickhouse.client.api.insert.InsertSettings	91.67%	77	84
com.clickhouse.client.api.internal.BaseCollectionConverter	100.00%	28	28
com.clickhouse.client.api.internal.BaseCollectionConverter.BaseArrayWriter	100.00%	6	6
com.clickhouse.client.api.internal.BaseCollectionConverter.BaseCollectionWriter	71.43%	15	21
com.clickhouse.client.api.internal.BaseCollectionConverter.BaseListWriter	100.00%	6	6
com.clickhouse.client.api.internal.BaseCollectionConverter.ListConversionState	100.00%	11	11
com.clickhouse.client.api.internal.BasicObjectsPool	0.00%		11
com.clickhouse.client.api.internal.CachingObjectsSupplier	0.00%		10
com.clickhouse.client.api.internal.ClickHouseLZ4InputStream	89.33%	67	75
com.clickhouse.client.api.internal.ClickHouseLZ4OutputStream	92.31%	60	65
com.clickhouse.client.api.internal.ClientStatisticsHolder	50.00%	7	14
com.clickhouse.client.api.internal.CommonSettings	97.14%	68	70
com.clickhouse.client.api.internal.CompressedEntity	80.00%	28	35
com.clickhouse.client.api.internal.DataTypeConverter	83.93%	94	112
com.clickhouse.client.api.internal.DataTypeConverter.ArrayAsStringWriter	100.00%	18	18
com.clickhouse.client.api.internal.DataTypeConverter.ListAsStringWriter	100.00%	16	16
com.clickhouse.client.api.internal.EnvUtils	0.00%		14
com.clickhouse.client.api.internal.Gauge	66.67%	4	6
com.clickhouse.client.api.internal.HttpAPIClientHelper	89.39%	438	490
com.clickhouse.client.api.internal.HttpAPIClientHelper.CustomSSLConnectionFactory	0.00%		9
com.clickhouse.client.api.internal.HttpAPIClientHelper.DummySSLConnectionSocketFactory	0.00%		3
com.clickhouse.client.api.internal.HttpAPIClientHelper.MeteredManagedHttpClientConnectionFactory	50.00%	7	14
com.clickhouse.client.api.internal.LZ4Entity	82.93%	34	41
com.clickhouse.client.api.internal.MapUtils	35.48%	22	62
com.clickhouse.client.api.internal.ServerSettings	0.00%		1
com.clickhouse.client.api.internal.StopWatch	66.67%	10	15
com.clickhouse.client.api.internal.TableSchemaParser	80.77%	21	26
com.clickhouse.client.api.internal.ValidationUtils	55.00%	11	20
com.clickhouse.client.api.internal.ValidationUtils.SettingsValidationException	100.00%	3	3
com.clickhouse.client.api.metadata.DefaultColumnToMethodMatchingStrategy	100.00%	13	13
com.clickhouse.client.api.metadata.NoSuchColumnException	0.00%		2
com.clickhouse.client.api.metadata.TableSchema	92.31%	36	39
com.clickhouse.client.api.metrics.ClientMetrics	100.00%	7	7
com.clickhouse.client.api.metrics.MicrometerLoader	90.91%	40	44
com.clickhouse.client.api.metrics.OperationMetrics	94.12%	16	17
com.clickhouse.client.api.metrics.ServerMetrics	100.00%	12	12
com.clickhouse.client.api.query.NullValueException	50.00%	2	4
com.clickhouse.client.api.query.QueryResponse	60.98%	25	41
com.clickhouse.client.api.query.QuerySettings	96.43%	81	84
com.clickhouse.client.api.query.QueryStatement	0.00%		4
com.clickhouse.client.api.query.Records	60.87%	14	23
com.clickhouse.client.api.query.Records.new Iterator() {...}	100.00%	5	5
com.clickhouse.client.api.serde.DataSerializationException	0.00%		6
com.clickhouse.client.api.serde.POJOSerDe	97.96%	48	49
com.clickhouse.client.api.serde.SerializerNotFoundException	0.00%		2
com.clickhouse.client.api.ServerException	100.00%	13	13
com.clickhouse.client.api.ServerException.ErrorCodes	100.00%	9	9
com.clickhouse.client.api.Session	100.00%	42	42
com.clickhouse.client.api.sql.SQLUtils	87.50%	28	32
com.clickhouse.client.api.transport.HttpEndpoint	88.00%	44	50
com.clickhouse.client.api.transport.HttpEndpoint.EndpointDetails	100.00%	6	6

mshustov · 2026-04-22T07:59:27Z

@claude review

claude · 2026-04-22T07:59:43Z

Claude finished @mshustov's task in 3m 4s —— View job

Code Review

Gather context and read changed files
Review StatementImpl.java changes
Review PreparedStatementTest.java changes
Check related code for correctness
Post final review

Summary

The fix correctly identifies and addresses the root cause: without ASYNC_OPERATIONS=true, runAsyncOperation wraps the supplier in CompletableFuture.completedFuture(resultSupplier.get()), which executes the HTTP request synchronously before returning the future. Calling .get(timeout, unit) on an already-completed future can never throw TimeoutException. Setting ASYNC_OPERATIONS=true dispatches to a thread pool so the future is genuinely interruptible.

Good approach overall, but there are several issues to address:

Issues

1. Fix is incomplete — `executeUpdateImpl` still has the same bug

executeUpdateImpl at StatementImpl.java:251 uses .get(queryTimeout, TimeUnit.SECONDS) without ASYNC_OPERATIONS=true, so executeUpdate/executeLargeUpdate calls will still never time out:

// StatementImpl.java:251-252 — same problem as the original bug
try (QueryResponse response = queryTimeout == 0 ? connection.getClient().query(lastStatementSql, mergedSettings).get()
        : connection.getClient().query(lastStatementSql, mergedSettings).get(queryTimeout, TimeUnit.SECONDS)) {

The same ASYNC_OPERATIONS flag must be applied here. Fix this →

2. Resource leak when timeout fires

When .get(queryTimeout, TimeUnit.SECONDS) throws TimeoutException, the CompletableFuture reference is discarded and response is null. The background thread continues executing the HTTP request, and the QueryResponse it eventually produces is never closed — leaking the underlying HTTP connection back to the pool.

The fix should keep a reference to the future and attach a cleanup callback:

CompletableFuture<QueryResponse> future = connection.getClient().query(lastStatementSql,
        mergedSettings.setOption(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), true));
try {
    response = future.get(queryTimeout, TimeUnit.SECONDS);
} catch (TimeoutException e) {
    // Clean up the response when the background task eventually finishes
    future.whenComplete((r, ex) -> {
        if (r != null) {
            try { r.close(); } catch (Exception ignore) {}
        }
    });
    throw e;
}

Fix this →

3. Test may be flaky on fast machines

PreparedStatementTest.java:572 — the test relies on MD5 + sum over 1M rows taking longer than 1 second. On fast CI runners this might complete in < 1s, causing assertThrows to fail because no exception is thrown. A deterministic alternative:

// Use sleep() to guarantee the query exceeds the timeout
final String sql = "SELECT sleep(3)";

Or increase the LIMIT significantly (e.g., LIMIT 100000000) to make the query reliably slow.

4. Unused import in test file

PreparedStatementTest.java:3 imports ClientConfigProperties but it is not referenced anywhere in the file. Fix this →

5. No server-side query cancellation on timeout

After the client-side timeout fires, the ClickHouse server continues executing the query indefinitely (until max_execution_time expires server-side, or the connection drops). This is a known trade-off, but it may be worth adding a comment or, optionally, calling KILL QUERY in the timeout path (similar to how cancel() already works). At a minimum, consider documenting this behavior.

Minor: Code style

The inline mergedSettings.setOption(...) passed directly into the query() call is a little subtle since it mutates mergedSettings as a side-effect of passing it:

// Current: mutation hidden inside argument
response = connection.getClient().query(lastStatementSql, mergedSettings
                .setOption(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), true))
        .get(queryTimeout, TimeUnit.SECONDS);

Splitting the mutation out improves readability:

mergedSettings.setOption(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), true);
response = connection.getClient().query(lastStatementSql, mergedSettings)
        .get(queryTimeout, TimeUnit.SECONDS);

chernser added 2 commits April 21, 2026 22:58

added test verifying that setQueryTimeout() is working with ASYNC ope…

7ec63e6

…rations

made async operation when timeout is set to non zero

0e78363

chernser requested a review from mzitnik as a code owner April 22, 2026 06:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jdbc-v2,client-v2]#2842

[jdbc-v2,client-v2]#2842
chernser wants to merge 2 commits intomainfrom
04/21/26/fix_jdbc_query_timeout

chernser commented Apr 22, 2026 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented Apr 22, 2026

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

mshustov commented Apr 22, 2026

Uh oh!

claude Bot commented Apr 22, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

chernser commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

sonarqubecloud Bot commented Apr 22, 2026

Quality Gate passed

Uh oh!

github-actions Bot commented Apr 22, 2026

Client V2 Coverage

Coverage Report

Uh oh!

mshustov commented Apr 22, 2026

Uh oh!

claude Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review

Summary

Issues

1. Fix is incomplete — executeUpdateImpl still has the same bug

2. Resource leak when timeout fires

3. Test may be flaky on fast machines

4. Unused import in test file

5. No server-side query cancellation on timeout

Minor: Code style

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chernser commented Apr 22, 2026 •

edited

Loading

claude Bot commented Apr 22, 2026 •

edited

Loading

1. Fix is incomplete — `executeUpdateImpl` still has the same bug