diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ceae046d..c728260de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,15 @@ ### Bug Fixes +- **[client-v2, jdbc-v2]** Fixed `Client.getTableSchema(...)` and `Client.getTableSchemaFromQuery(...)` failing with + `Failed to parse column null defined by type 'null'` against ClickHouse `26.8+` (and `jdbc-v2` failing with it, + because `Connection`/`PreparedStatement` metadata calls use them). Both methods asked for `TSKV` with a `FORMAT` + clause in the `DESCRIBE` query, while the client sends the requested format of the operation in the + `X-ClickHouse-Format` header on every request. A server before `26.8` used the format from the query, but since + `26.8` the header wins, so the server answered with `RowBinaryWithNamesAndTypes` and the `TSKV` parser read binary + data. The internal queries of the client (the two schema calls and `ping()`) now request their format through the + settings of the operation only, so the header and the query always agree. + (https://github.com/ClickHouse/clickhouse-java/issues/3068) - **[jdbc-v2]** Fixed an `INSERT` whose values list holds a function call the bundled `ANTLR4` grammar cannot match - such as `hex(x'AB')`, valid ClickHouse the grammar has no hex string literal for - being reported to hold no function call when an `ANTLR4` parser backend is selected (`jdbc_sql_parser=ANTLR4` / `ANTLR4_PARAMS_PARSER`). Function calls in diff --git a/client-v2/src/main/java/com/clickhouse/client/api/Client.java b/client-v2/src/main/java/com/clickhouse/client/api/Client.java index 3e5764adb..884aa1f3f 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/Client.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/Client.java @@ -1374,7 +1374,8 @@ public boolean ping() { public boolean ping(long timeout) { long startTime = System.nanoTime(); try { - CompletableFuture future = query("SELECT 1 FORMAT TabSeparated"); + CompletableFuture future = + query("SELECT 1", new QuerySettings().setFormat(ClickHouseFormat.TabSeparated)); try (QueryResponse response = timeout > 0 ? future.get(timeout, TimeUnit.MILLISECONDS) : future.get()) { return true; } @@ -1804,8 +1805,9 @@ public CompletableFuture query(String sqlQuery) { *

Sends SQL query to server.

* Notes: *
    - *
  • Server response format can be specified thru `settings` or in SQL query.
    • - *
  • If specified in both, the `sqlQuery` will take precedence.
    • + *
  • Server response format should be specified thru `settings` and not with a FORMAT clause in the SQL query.
    • + *
  • If specified in both, the format that wins depends on the server version: a server before v26.8 uses the + * format from the `sqlQuery`, a server since v26.8 uses the format from the `settings`.
    • *
* @param sqlQuery - complete SQL query. * @param settings - query operation settings. @@ -1834,8 +1836,10 @@ public CompletableFuture query(String sqlQuery, QuerySettings set * * Notes: *
    - *
  • Server response format can be specified through {@code settings} or in SQL query.
    • - *
  • If specified in both, the {@code sqlQuery} will take precedence.
    • + *
  • Server response format should be specified through {@code settings} and not with a FORMAT clause in the + * SQL query.
    • + *
  • If specified in both, the format that wins depends on the server version: a server before v26.8 uses the + * format from the {@code sqlQuery}, a server since v26.8 uses the format from the {@code settings}.
    • *
* * @param sqlQuery - complete SQL query. @@ -2201,7 +2205,7 @@ public TableSchema getTableSchema(String table) { * @return {@code TableSchema} - Schema of the table */ public TableSchema getTableSchema(String table, String database) { - final String sql = "DESCRIBE TABLE " + table + " FORMAT " + ClickHouseFormat.TSKV.name(); + final String sql = "DESCRIBE TABLE " + table; return getTableSchemaImpl(sql, table, null, database, null); } @@ -2215,7 +2219,7 @@ public TableSchema getTableSchemaFromQuery(String sql) { } public TableSchema getTableSchemaFromQuery(String sql, Map params) { - final String describeQuery = "DESC (" + sql + ") FORMAT " + ClickHouseFormat.TSKV.name(); + final String describeQuery = "DESC (" + sql + ")"; return getTableSchemaImpl(describeQuery, null, sql, getDefaultDatabase(), params); } @@ -2223,7 +2227,10 @@ private TableSchema getTableSchemaImpl( String describeQuery, String name, String originalQuery, String database, Map queryParams) { int operationTimeout = getOperationTimeout(); - QuerySettings settings = new QuerySettings().setDatabase(database); + // The format is requested thru settings (the X-ClickHouse-Format header) and not with a FORMAT clause: + // since v26.8 the server lets the header override the format written in the query, so a query that asks + // for one format while the client sends another in the header returns data the caller cannot parse. + QuerySettings settings = new QuerySettings().setDatabase(database).setFormat(ClickHouseFormat.TSKV); try (QueryResponse response = operationTimeout == 0 ? query(describeQuery, queryParams, settings).get() : query(describeQuery, queryParams, settings).get(operationTimeout, TimeUnit.MILLISECONDS)) { diff --git a/client-v2/src/test/java/com/clickhouse/client/api/RequestFormatUnitTest.java b/client-v2/src/test/java/com/clickhouse/client/api/RequestFormatUnitTest.java new file mode 100644 index 000000000..4f8acd053 --- /dev/null +++ b/client-v2/src/test/java/com/clickhouse/client/api/RequestFormatUnitTest.java @@ -0,0 +1,103 @@ +package com.clickhouse.client.api; + +import com.clickhouse.client.api.query.QueryResponse; +import com.clickhouse.client.api.query.QuerySettings; +import com.clickhouse.data.ClickHouseFormat; +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.github.tomakehurst.wiremock.verification.LoggedRequest; +import org.testng.Assert; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; + +public class RequestFormatUnitTest { + + private static final String TSKV_RESPONSE = + "name=id\ttype=Int32\tdefault_type=\tdefault_expression=\tcomment=\tcodec_expression=\tttl_expression=\n"; + + private WireMockServer mockServer; + + private Client client; + + @BeforeMethod + public void setUp() { + mockServer = new WireMockServer(WireMockConfiguration.options().dynamicPort()); + mockServer.start(); + mockServer.stubFor(WireMock.post(WireMock.anyUrl()) + .willReturn(WireMock.aResponse().withStatus(200) + .withHeader("Content-Type", "text/plain") + .withBody(TSKV_RESPONSE))); + client = new Client.Builder() + .addEndpoint("http://localhost:" + mockServer.port()) + .setUsername("default") + .setPassword("") + .setDefaultDatabase("default") + .compressServerResponse(false) + .build(); + } + + @AfterMethod + public void tearDown() { + if (client != null) { + client.close(); + } + if (mockServer != null) { + mockServer.stop(); + } + } + + @Test(dataProvider = "requestFormatData") + public void testFormatIsRequestedWithHeaderOnly(Consumer operation, String expectedStatement, + ClickHouseFormat expectedFormat) { + operation.accept(client); + + LoggedRequest request = findRequest(expectedStatement); + Assert.assertEquals(request.getBodyAsString().trim(), expectedStatement); + Assert.assertEquals(request.getHeader("X-ClickHouse-Format"), expectedFormat.name()); + } + + @DataProvider(name = "requestFormatData") + public static Object[][] requestFormatData() { + return new Object[][]{ + {(Consumer) c -> Assert.assertEquals( + c.getTableSchema("test_table", "test_db").getColumns().size(), 1), + "DESCRIBE TABLE test_table", ClickHouseFormat.TSKV}, + {(Consumer) c -> Assert.assertEquals( + c.getTableSchemaFromQuery("SELECT id FROM test_table").getColumns().size(), 1), + "DESC (SELECT id FROM test_table)", ClickHouseFormat.TSKV}, + {(Consumer) c -> Assert.assertTrue(c.ping()), + "SELECT 1", ClickHouseFormat.TabSeparated}, + // Formats a caller asks for keep flowing through unchanged + {(Consumer) c -> runQuery(c, "SELECT 2", null), + "SELECT 2", ClickHouseFormat.RowBinaryWithNamesAndTypes}, + {(Consumer) c -> runQuery(c, "SELECT 3", + new QuerySettings().setFormat(ClickHouseFormat.JSONEachRow)), + "SELECT 3", ClickHouseFormat.JSONEachRow}, + }; + } + + private static void runQuery(Client client, String sql, QuerySettings settings) { + try (QueryResponse response = client.query(sql, settings).get(10, TimeUnit.SECONDS)) { + Assert.assertNotNull(response); + } catch (Exception e) { + throw new AssertionError("query failed: " + sql, e); + } + } + + private LoggedRequest findRequest(String statement) { + List requests = mockServer.findAll(WireMock.postRequestedFor(WireMock.anyUrl())); + for (LoggedRequest request : requests) { + if (request.getBodyAsString().trim().equals(statement)) { + return request; + } + } + throw new AssertionError("no request was sent with statement '" + statement + "', sent: " + requests); + } +} diff --git a/client-v2/src/test/java/com/clickhouse/client/observability/SpanRecorderTest.java b/client-v2/src/test/java/com/clickhouse/client/observability/SpanRecorderTest.java index 19d0ad10e..49b671eae 100644 --- a/client-v2/src/test/java/com/clickhouse/client/observability/SpanRecorderTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/observability/SpanRecorderTest.java @@ -116,7 +116,7 @@ public void testPingSpan() { // implements on top of a query CapturedSpan operationSpan = recorder.operationSpan(); Assert.assertEquals(operationSpan.getName(), "query " + database); - Assert.assertEquals(operationSpan.getAttribute(SpanAttribute.DB_QUERY_TEXT), "SELECT 1 FORMAT TabSeparated"); + Assert.assertEquals(operationSpan.getAttribute(SpanAttribute.DB_QUERY_TEXT), "SELECT 1"); Assert.assertNull(operationSpan.getAttribute(SpanAttribute.DB_OPERATION_NAME)); Assert.assertNull(operationSpan.getAttribute(SpanAttribute.DB_COLLECTION_NAME)); Assert.assertEquals(operationSpan.getEndCount(), 1); @@ -143,7 +143,7 @@ public void testTableSchemaSpanIsReportedAsQuery() { CapturedSpan operationSpan = recorder.operationSpan(); Assert.assertEquals(operationSpan.getName(), "query " + database); Assert.assertEquals(operationSpan.getAttribute(SpanAttribute.DB_QUERY_TEXT), - "DESCRIBE TABLE " + TABLE + " FORMAT TSKV"); + "DESCRIBE TABLE " + TABLE); Assert.assertNull(operationSpan.getAttribute(SpanAttribute.DB_OPERATION_NAME)); Assert.assertNull(operationSpan.getAttribute(SpanAttribute.DB_COLLECTION_NAME)); Assert.assertEquals(operationSpan.getEndCount(), 1); @@ -157,7 +157,7 @@ public void testTableSchemaFromQuerySpanIsReportedAsQuery() { CapturedSpan operationSpan = recorder.operationSpan(); Assert.assertEquals(operationSpan.getName(), "query " + database); Assert.assertEquals(operationSpan.getAttribute(SpanAttribute.DB_QUERY_TEXT), - "DESC (SELECT id FROM " + TABLE + ") FORMAT TSKV"); + "DESC (SELECT id FROM " + TABLE + ")"); Assert.assertNull(operationSpan.getAttribute(SpanAttribute.DB_OPERATION_NAME)); Assert.assertEquals(operationSpan.getEndCount(), 1); }