Fix client-v2: request the format of internal queries thru settings, not a FORMAT clause

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

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<QueryResponse> future = query("SELECT 1 FORMAT TabSeparated");
+            CompletableFuture<QueryResponse> 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<QueryResponse> query(String sqlQuery) {
      * <p>Sends SQL query to server.</p>
      * <b>Notes:</b>
      * <ul>
-     * <li>Server response format can be specified thru `settings` or in SQL query.</li>
-     * <li>If specified in both, the `sqlQuery` will take precedence.</li>
+     * <li>Server response format should be specified thru `settings` and not with a FORMAT clause in the SQL query.</li>
+     * <li>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`.</li>
      * </ul>
      * @param sqlQuery - complete SQL query.
      * @param settings - query operation settings.
@@ -1834,8 +1836,10 @@ public CompletableFuture<QueryResponse> query(String sqlQuery, QuerySettings set
      *
      * <b>Notes:</b>
      * <ul>
-     * <li>Server response format can be specified through {@code settings} or in SQL query.</li>
-     * <li>If specified in both, the {@code sqlQuery} will take precedence.</li>
+     * <li>Server response format should be specified through {@code settings} and not with a FORMAT clause in the
+     * SQL query.</li>
+     * <li>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}.</li>
      * </ul>
      *
      * @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,15 +2219,18 @@ public TableSchema getTableSchemaFromQuery(String sql) {
     }
 
     public TableSchema getTableSchemaFromQuery(String sql, Map<String, Object> params) {
-        final String describeQuery = "DESC (" + sql + ") FORMAT " + ClickHouseFormat.TSKV.name();
+        final String describeQuery = "DESC (" + sql + ")";
         return getTableSchemaImpl(describeQuery, null, sql, getDefaultDatabase(), params);
     }
 
     private TableSchema getTableSchemaImpl(
             String describeQuery, String name, String originalQuery, String database, Map<String, Object> 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)) {

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<Client> 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<Client>) c -> Assert.assertEquals(
+                        c.getTableSchema("test_table", "test_db").getColumns().size(), 1),
+                        "DESCRIBE TABLE test_table", ClickHouseFormat.TSKV},
+                {(Consumer<Client>) c -> Assert.assertEquals(
+                        c.getTableSchemaFromQuery("SELECT id FROM test_table").getColumns().size(), 1),
+                        "DESC (SELECT id FROM test_table)", ClickHouseFormat.TSKV},
+                {(Consumer<Client>) c -> Assert.assertTrue(c.ping()),
+                        "SELECT 1", ClickHouseFormat.TabSeparated},
+                // Formats a caller asks for keep flowing through unchanged
+                {(Consumer<Client>) c -> runQuery(c, "SELECT 2", null),
+                        "SELECT 2", ClickHouseFormat.RowBinaryWithNamesAndTypes},
+                {(Consumer<Client>) 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<LoggedRequest> 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);
+    }
+}

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);
     }