Skip to content

Commit 46d6fa2

Browse files
authored
Merge branch 'main' into polyglot/jdbc-v2-new-clickhouse-keywords
2 parents 56c6d0c + 2b2ac01 commit 46d6fa2

4 files changed

Lines changed: 145 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,21 @@
158158
keywords `IDLE TIMEOUT` and `RECENT SAMPLES`) to the list of keywords allowed in identifier positions. The server
159159
accepts all of them as a column or table alias, so a query using one of them as an identifier must parse.
160160
(https://github.com/ClickHouse/clickhouse-java/issues/3113)
161+
- **[client-v2]** Fixed a query with statement parameters sent in the request body
162+
(`client.http.use_form_request_for_query=true`) failing with `LZ4 decompression failed ... (LZ4_DECODER_FAILED)`
163+
when client request compression and HTTP compression were both enabled. The multipart body is always sent
164+
uncompressed, but the request still declared `Content-Encoding: lz4`; ClickHouse `26.8+` honours that header for
165+
multipart requests and tried to decompress a plain body. The header is now omitted for multipart requests, like
166+
the `decompress` query parameter already was. Response compression (`Accept-Encoding`,
167+
`enable_http_compression`) is unchanged. (https://github.com/ClickHouse/clickhouse-java/issues/3075)
168+
- **[client-v1]** Fixed the `DateTime64` case of `testReadWriteSimpleTypes` failing against ClickHouse 26.8. From 26.8
169+
an unquoted number written to a `DateTime64` column in the `Values`/`Quoted` and `JSON` paths is a Unix timestamp in
170+
seconds instead of the raw scaled value - the server setting `input_format_read_datetime_number_as_raw_value`
171+
changed its default from `1` to `0` - so the `insert into ... values(1)` of the test stored `1970-01-01 00:00:01`
172+
instead of the expected `1970-01-01 00:00:00.001`. The test now writes a quoted date-time literal for `DateTime64`,
173+
as it already does for `FixedString` and `UUID`, so the written value means the same on every server version and the
174+
sub-second round-trip stays covered. No client code is affected: both clients quote a date-time value in a text
175+
statement or send it in `RowBinary`. (https://github.com/ClickHouse/clickhouse-java/issues/3114)
161176
- **[jdbc-v2, client-v2]** Fixes issue with `FORMAT` in query unable to override format set by client when used with
162177
ClickHouse 26.8+. Default format is `RowBinaryWithNamesAndTypes` set at client level. For JDBC, recommend using
163178
`format=JSONEachRow` to query JSON. Setting `format=` (empty or `null`) omits the format request header so explicit

‎clickhouse-client/src/test/java/com/clickhouse/client/ClientIntegrationTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,10 @@ public void testReadWriteSimpleTypes(String dataType, String zero, String negati
12061206
negativeOneValue = ClickHouseUtils.format("'%s'", ClickHouseIntegerValue.of(-1).asUuid());
12071207
zeroValue = ClickHouseUtils.format("'%s'", ClickHouseIntegerValue.of(0).asUuid());
12081208
positiveOneValue = ClickHouseUtils.format("'%s'", ClickHouseIntegerValue.of(1).asUuid());
1209+
} else if (dataType.startsWith(ClickHouseDataType.DateTime64.name())) {
1210+
negativeOneValue = ClickHouseUtils.format("'%s'", negativeOne);
1211+
zeroValue = ClickHouseUtils.format("'%s'", zero);
1212+
positiveOneValue = ClickHouseUtils.format("'%s'", positiveOne);
12091213
}
12101214

12111215
try {

‎client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,11 @@ public TransportRequest createRequest(Endpoint server, Map<String, Object> reque
624624

625625
final HttpEntity httpEntity;
626626
if (useMultipart) {
627+
// a multipart body is always sent as-is, so the request must not declare a content encoding - the
628+
// server would fail to decompress the plain body. Removed after addHeaders() to also drop an
629+
// encoding set by the application with `http_header_*`.
630+
req.removeHeaders(HttpHeaders.CONTENT_ENCODING);
631+
627632
MultipartEntityBuilder multipartEntityBuilder = MultipartEntityBuilder.create();
628633
addStatementParams(requestConfig, multipartEntityBuilder::addTextBody);
629634
multipartEntityBuilder.addTextBody(ClickHouseHttpProto.QPARAM_QUERY_STMT, body);

‎client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
1414
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
1515
import org.apache.hc.core5.http.ClassicHttpResponse;
16+
import org.apache.hc.core5.http.Header;
1617
import org.apache.hc.core5.http.HttpEntity;
18+
import org.apache.hc.core5.http.HttpHeaders;
1719
import org.apache.hc.core5.http.message.BasicHeader;
1820
import org.mockito.ArgumentCaptor;
1921
import org.mockito.MockedConstruction;
@@ -327,6 +329,125 @@ public void testShouldRetryUsesServerExceptionFromCause(Throwable ex, boolean ex
327329
assertEquals(helper.shouldRetry(ex, new HashMap<>()), expectedRetry);
328330
}
329331

332+
/**
333+
* A multipart body (statement parameters sent as form data) is never compressed, so the request must not
334+
* declare a content encoding - the server would try to decompress the plain body and fail with
335+
* LZ4_DECODER_FAILED. A request that is not multipart, and response compression, keep their signalling.
336+
*/
337+
@DataProvider(name = "requestCompressionSignalling")
338+
public static Object[][] requestCompressionSignalling() {
339+
return new Object[][] {
340+
// clientCompression, useHttpCompression, sendParamsInBody, withParams,
341+
// contentEncoding, acceptEncoding, decompressParam
342+
{true, true, true, true, null, "lz4", false},
343+
{true, true, true, false, "lz4", "lz4", false}, // no parameters -> not a multipart request
344+
{true, true, false, true, "lz4", "lz4", false},
345+
{false, true, true, true, null, "lz4", false},
346+
{true, false, true, true, null, null, false},
347+
{true, false, false, true, null, null, true},
348+
};
349+
}
350+
351+
@Test(dataProvider = "requestCompressionSignalling")
352+
public void testRequestCompressionSignalling(boolean clientCompression, boolean useHttpCompression,
353+
boolean sendParamsInBody, boolean withParams,
354+
String expectedContentEncoding, String expectedAcceptEncoding,
355+
boolean expectDecompressParam) {
356+
Map<String, Object> reqConfig = compressionConfig(clientCompression, useHttpCompression, sendParamsInBody);
357+
if (withParams) {
358+
reqConfig.put(HttpAPIClientHelper.KEY_STATEMENT_PARAMS, Collections.singletonMap("p1", "1"));
359+
}
360+
361+
HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig,
362+
"SELECT {p1:Int32}").getDelegate();
363+
364+
String setup = "clientCompression=" + clientCompression + ", useHttpCompression=" + useHttpCompression
365+
+ ", sendParamsInBody=" + sendParamsInBody + ", withParams=" + withParams;
366+
assertEquals(headerValue(req, HttpHeaders.CONTENT_ENCODING), expectedContentEncoding,
367+
"unexpected " + HttpHeaders.CONTENT_ENCODING + " for " + setup);
368+
assertEquals(req.getEntity().getContentEncoding(), expectedContentEncoding,
369+
"the request body entity must declare the same encoding as the request for " + setup);
370+
assertEquals(headerValue(req, HttpHeaders.ACCEPT_ENCODING), expectedAcceptEncoding,
371+
"response compression signalling must not depend on the request body form");
372+
373+
String query = req.getRequestUri();
374+
assertEquals(query.contains(ClickHouseHttpProto.QPARAM_DECOMPRESS + "=1"), expectDecompressParam,
375+
"unexpected " + ClickHouseHttpProto.QPARAM_DECOMPRESS + " parameter in " + query);
376+
assertEquals(query.contains(ClickHouseHttpProto.QPARAM_ENABLE_HTTP_COMPRESSION + "=1"), useHttpCompression,
377+
"unexpected " + ClickHouseHttpProto.QPARAM_ENABLE_HTTP_COMPRESSION + " parameter in " + query);
378+
}
379+
380+
@DataProvider(name = "contentEncodingHeaderNames")
381+
public static Object[][] contentEncodingHeaderNames() {
382+
return new Object[][] {{HttpHeaders.CONTENT_ENCODING}, {"content-encoding"}};
383+
}
384+
385+
/**
386+
* A content encoding set by the application through {@code http_header_*} cannot make the plain multipart
387+
* body compressed either, so it must not reach the server, whatever the header is spelled like.
388+
*/
389+
@Test(dataProvider = "contentEncodingHeaderNames")
390+
public void testCustomContentEncodingHeaderRemovedForMultipartRequest(String headerName) {
391+
Map<String, Object> reqConfig = compressionConfig(false, false, true);
392+
reqConfig.put(HttpAPIClientHelper.KEY_STATEMENT_PARAMS, Collections.singletonMap("p1", "1"));
393+
reqConfig.put(ClientConfigProperties.HTTP_HEADER_PREFIX + headerName, "lz4");
394+
395+
HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig,
396+
"SELECT {p1:Int32}").getDelegate();
397+
398+
assertNull(headerValue(req, HttpHeaders.CONTENT_ENCODING),
399+
"a custom " + headerName + " must be removed from a multipart request");
400+
}
401+
402+
/**
403+
* A request that is not multipart is unaffected: a content encoding set by the application through
404+
* {@code http_header_*} still reaches the server.
405+
*/
406+
@Test(dataProvider = "contentEncodingHeaderNames")
407+
public void testCustomContentEncodingHeaderKeptForRequestWithoutParams(String headerName) {
408+
Map<String, Object> reqConfig = compressionConfig(false, false, true);
409+
reqConfig.put(ClientConfigProperties.HTTP_HEADER_PREFIX + headerName, "lz4");
410+
411+
HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig,
412+
"SELECT 1").getDelegate();
413+
414+
assertEquals(headerValue(req, HttpHeaders.CONTENT_ENCODING), "lz4",
415+
"a custom " + headerName + " must be kept on a request that is not multipart");
416+
}
417+
418+
/**
419+
* Data is streamed into the request body, so an insert is never a multipart request and keeps compressing
420+
* its body even when the client is configured to send statement parameters in the body.
421+
*/
422+
@Test
423+
public void testDataRequestKeepsContentEncodingWhenParamsInBodyEnabled() {
424+
Map<String, Object> reqConfig = compressionConfig(true, true, true);
425+
426+
HttpPost req = newHelper().createRequest(new HttpEndpoint("localhost", 8123, false, "/"), reqConfig,
427+
out -> out.write(1)).getDelegate();
428+
429+
assertEquals(headerValue(req, HttpHeaders.CONTENT_ENCODING), "lz4",
430+
"an insert body is compressed, so the request must declare the content encoding");
431+
}
432+
433+
private static HttpAPIClientHelper newHelper() {
434+
return HttpAPIClientHelperFactory.newHelper(new HashMap<>(), LZ4Factory.fastestInstance());
435+
}
436+
437+
private static Map<String, Object> compressionConfig(boolean clientCompression, boolean useHttpCompression,
438+
boolean sendParamsInBody) {
439+
Map<String, Object> reqConfig = new HashMap<>();
440+
reqConfig.put(ClientConfigProperties.COMPRESS_CLIENT_REQUEST.getKey(), clientCompression);
441+
reqConfig.put(ClientConfigProperties.USE_HTTP_COMPRESSION.getKey(), useHttpCompression);
442+
reqConfig.put(ClientConfigProperties.HTTP_SEND_PARAMS_IN_BODY.getKey(), sendParamsInBody);
443+
return reqConfig;
444+
}
445+
446+
private static String headerValue(HttpPost req, String name) {
447+
Header header = req.getFirstHeader(name);
448+
return header == null ? null : header.getValue();
449+
}
450+
330451
/**
331452
* A server error is logged at WARN only for an unknown status code (the switch's default branch). Known
332453
* error paths emit no server-error WARN: readError surfaces an exception-code error, a mapped code (502)

0 commit comments

Comments
 (0)