From d1a5c6af90ee1e22c93a20b8464a000de4d07a74 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:12:39 +0000 Subject: [PATCH 1/7] [client-v2, jdbc-v2] Fix bad and noisy logging - Parser no longer logs the full user SQL at WARN (can contain passwords/PII); the failure warns without the statement and the SQL is logged only at DEBUG. - jdbc-v2 metadata unknown-type fallbacks (mapping to JDBCType.OTHER) log at DEBUG instead of ERROR, and use the com.clickhouse.logging %s format so the type name actually renders. - Remove the per-conversion DEBUG stack trace in ExceptionUtils.toSqlState (often contained SQL) and the now-unused slf4j logger field. - Remove the per-element TRACE in PreparedStatement parameter encoding and the two duplicate per-execution SQL TRACEs (the query is still traced once). - Consolidate client-v2 retry logging into a single WARN carrying operation, attempt, endpoint, query id and cause; downgrade the duplicate transport connection-failure WARNs to DEBUG so a retried failure is not double-logged. - Remove the two redundant per-operation compression DEBUGs (already logged at init) and the jdbc-v2 driver static-init banner. - LZ4 in/out stream logs now report the algorithm and buffer size instead of an unusable object identity. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/2970 --- CHANGELOG.md | 13 +++++++++++++ .../main/java/com/clickhouse/client/api/Client.java | 6 +++--- .../api/internal/ClickHouseLZ4InputStream.java | 3 +-- .../api/internal/ClickHouseLZ4OutputStream.java | 2 +- .../client/api/internal/HttpAPIClientHelper.java | 10 ++-------- .../java/com/clickhouse/jdbc/ConnectionImpl.java | 4 +--- .../src/main/java/com/clickhouse/jdbc/Driver.java | 3 --- .../com/clickhouse/jdbc/PreparedStatementImpl.java | 2 -- .../java/com/clickhouse/jdbc/StatementImpl.java | 2 -- .../clickhouse/jdbc/internal/ExceptionUtils.java | 5 ----- .../jdbc/metadata/DatabaseMetaDataImpl.java | 6 +++--- jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj | 3 ++- 12 files changed, 26 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca4088730..d166e186c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,19 @@ ### Bug Fixes +- **[client-v2, jdbc-v2]** Cleaned up noisy and potentially sensitive logging in `jdbc-v2` and `client-v2` + (https://github.com/ClickHouse/clickhouse-java/issues/2970). SQL that fails to parse is no longer logged + in full at `WARN` (it could contain passwords/PII); the parser now warns without the statement and logs + the raw SQL only at `DEBUG`. In `jdbc-v2`: unknown-type metadata fallbacks (mapping to `JDBCType.OTHER`) + log at `DEBUG` instead of `ERROR`; `ExceptionUtils.toSqlState` no longer emits a per-conversion `DEBUG` + stack trace that often included SQL; the per-element `TRACE` in prepared-statement parameter encoding and + the duplicate per-execution SQL `TRACE`s were removed (the query is still traced once); and the + static-init driver banner was dropped. In `client-v2`: a retried request failure is now logged once as a + single consolidated retry `WARN` (operation, attempt, endpoint, query id and cause) rather than a + transport `WARN` followed by a stack-trace `WARN`; the two per-operation compression `DEBUG`s were removed + (compression settings are already logged once at initialization); and the LZ4 compressor/decompressor + `DEBUG`s now report the algorithm and buffer size instead of an unusable object identity. + - **[client-v2]** Fixed scalar `String` query parameters containing a tab (`0x09`), newline (`0x0a`) or backslash being mishandled through the server's `param_` interface. A `{name:String}` parameter value is parsed by the server with `deserializeTextEscaped`, which treated a raw tab or 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 c8fd9eee0..8309bbf71 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 @@ -1499,7 +1499,7 @@ public CompletableFuture insert(String tableName, List data, lastException = httpClientHelper.wrapException(msg, e, requestSettings.getQueryId()); if (httpClientHelper.shouldRetry(e, requestSettings.getAllSettings()) && requestIsNotCancelled(queryId)) { if (i < maxAttempts) { - LOG.warn("Retrying.", e); + LOG.warn("{}, endpoint: {}, cause: {}. Retrying.", msg, selectedEndpoint, e.getMessage()); selectedEndpoint = nodeSelector.getNextAliveNode(selectedEndpoint); } else { nodeSelector.getNextAliveNode(selectedEndpoint); @@ -1705,7 +1705,7 @@ public CompletableFuture insert(String tableName, lastException = httpClientHelper.wrapException(msg, e, requestSettings.getQueryId()); if (httpClientHelper.shouldRetry(e, requestSettings.getAllSettings()) && requestIsNotCancelled(requestSettings.getQueryId())) { if (i < maxAttempts) { - LOG.warn("Retrying.", e); + LOG.warn("{}, endpoint: {}, cause: {}. Retrying.", msg, selectedEndpoint, e.getMessage()); selectedEndpoint = nodeSelector.getNextAliveNode(selectedEndpoint); } else { nodeSelector.getNextAliveNode(selectedEndpoint); @@ -1849,7 +1849,7 @@ public CompletableFuture query(String sqlQuery, Map boolean useHttpCompression = ClientConfigProperties.USE_HTTP_COMPRESSION.getOrDefault(requestConfig); boolean appCompressedData = ClientConfigProperties.APP_COMPRESSED_DATA.getOrDefault(requestConfig); - LOG.debug("wrapRequestEntity: client compression: {}, http compression: {}, content encoding: {}", - clientCompression, useHttpCompression, httpEntity.getContentEncoding()); - if (httpEntity.getContentEncoding() != null && !appCompressedData) { // http header is set and data is not compressed return new CompressedEntity(httpEntity, false, CompressorStreamFactory.getSingleton()); @@ -937,9 +934,6 @@ private HttpEntity wrapResponseEntity(HttpEntity httpEntity, int httpStatus, Map boolean serverCompression = ClientConfigProperties.COMPRESS_SERVER_RESPONSE.getOrDefault(requestConfig); boolean useHttpCompression = ClientConfigProperties.USE_HTTP_COMPRESSION.getOrDefault(requestConfig); - LOG.debug("wrapResponseEntity: server compression: {}, http compression: {}, content encoding: {}", - serverCompression, useHttpCompression, httpEntity.getContentEncoding()); - if (httpEntity.getContentEncoding() != null) { // http compressed response return new CompressedEntity(httpEntity, true, CompressorStreamFactory.getSingleton()); diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java index 791ee0f49..4ce471126 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java @@ -96,9 +96,7 @@ public ConnectionImpl(String url, Properties info) throws SQLException { clientName = this.appName + " " + clientName; // Use the application name as client name } - if (this.config.isDisableFrameworkDetection()) { - LOG.debug("Framework detection is disabled."); - } else { + if (!this.config.isDisableFrameworkDetection()) { String detectedFrameworks = Driver.FrameworksDetection.getFrameworksDetected(); LOG.debug("Detected frameworks: {}", detectedFrameworks); if (!detectedFrameworks.trim().isEmpty()) { diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java index ed00fb650..4620dc917 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java @@ -62,10 +62,7 @@ public static String getFrameworksDetected() { public static final String DRIVER_CLIENT_NAME = "jdbc-v2/"; static { - log.debug("Initializing ClickHouse JDBC driver V2"); - driverVersion = ClickHouseClientOption.readVersionFromResource("jdbc-v2-version.properties"); - log.debug("ClickHouse JDBC driver version: {}", driverVersion); int[] versions = parseVersion(driverVersion); diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java index 4a93f5bbb..83a518831 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java @@ -748,8 +748,6 @@ private String encodeObject(Object x) throws SQLException { private static final char C_BRACKET = ']'; private String encodeObject(Object x, Long length) throws SQLException { - LOG.trace("Encoding object: {}", x); - try { if (x == null) { return "NULL"; diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java index 9bb3db82e..c103fd958 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java @@ -78,11 +78,9 @@ protected void ensureOpen() throws SQLException { } private String parseJdbcEscapeSyntax(String sql) { - LOG.trace("Original SQL: {}", sql); if (escapeProcessingEnabled) { sql = escapedSQLToNative(sql); } - LOG.trace("Escaped SQL: {}", sql); return sql; } diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/ExceptionUtils.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/ExceptionUtils.java index 81067ab7d..d0947d735 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/ExceptionUtils.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/ExceptionUtils.java @@ -4,8 +4,6 @@ import com.clickhouse.client.api.ClientMisconfigurationException; import com.clickhouse.client.api.ConnectionInitiationException; import com.clickhouse.client.api.ServerException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.net.MalformedURLException; import java.sql.SQLDataException; @@ -15,7 +13,6 @@ * Helper class for building {@link SQLException}. */ public final class ExceptionUtils { - private static final Logger log = LoggerFactory.getLogger(ExceptionUtils.class); public static final String SQL_STATE_CLIENT_ERROR = "HY000"; public static final String SQL_STATE_OPERATION_CANCELLED = "HY008"; public static final String SQL_STATE_CONNECTION_EXCEPTION = "08000"; @@ -50,8 +47,6 @@ public static SQLException toSqlState(Exception cause) { * @return Converted {@link SQLException} */ public static SQLException toSqlState(String message, String debugMessage, Exception cause) { - log.debug("Exception Message: {}, Debug message: {}", message, debugMessage, cause); - if (cause == null) { return new SQLException(message == null ? "Unknown client error" : message, SQL_STATE_CLIENT_ERROR); } diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java index 5f61faf34..9ad3933c8 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java @@ -1132,7 +1132,7 @@ private static String generateSqlTypeSizes(String columnName) { ClickHouseDataType dt = c.getDataType(); type = JdbcUtils.convertToSqlType(dt); } catch (Exception e) { - log.error("Failed to convert column data type to SQL type: {}", typeName, e); + log.debug("Failed to convert column data type to SQL type: %s", typeName, e); type = JDBCType.OTHER; // In case of error, return SQL type 0 } } @@ -1339,10 +1339,10 @@ public ResultSet getTypeInfo() throws SQLException { try { type = JdbcUtils.convertToSqlType(ClickHouseDataType.valueOf(typeName)); } catch (IllegalArgumentException e) { - log.error("Unknown type: " + typeName + ". Please check for a new version of the client."); + log.debug("Unknown type: %s. Please check for a new version of the client.", typeName); type = JDBCType.OTHER; } catch (Exception e) { - log.error("Failed to get SQL type for type: " + typeName, e); + log.debug("Failed to get SQL type for type: %s", typeName, e); type = JDBCType.OTHER; } } diff --git a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj index 3140e04e8..fa245c77a 100644 --- a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj +++ b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj @@ -98,7 +98,8 @@ public class ClickHouseSqlParser { if (DEBUG) { throw new IllegalArgumentException(e); } else { - log.warn("%s. If you believe the SQL is valid, please feel free to open an issue on Github with this warning and the following SQL attached.\n%s", e.getMessage(), sql); + log.warn("%s. If you believe the SQL is valid, please feel free to open an issue on Github with this warning.", e.getMessage()); + log.debug("Unparsable SQL: %s", sql); } } From 3e493d7edda112b10ad0c7e567fd7d20747f79b1 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 05:12:49 +0000 Subject: [PATCH 2/7] Address review (#2970): migrate jdbc-v2 metadata & parser logging to SLF4J @chernser asked for SLF4J `{}` placeholders on DatabaseMetaDataImpl and ClickHouseSqlParser. Those two classes used the deprecated `com.clickhouse.logging.Logger` facade, which formats via `java.util.Formatter` (`%s`), so a bare `%s`->`{}` swap would have rendered a literal `{}`. Migrate both classes to `org.slf4j.Logger`/`LoggerFactory` (matching the rest of jdbc-v2) and switch the placeholders to `{}`, so they render correctly. This also fixes a pre-existing latent bug: DatabaseMetaData.getTables logged `catalog={}, schemaPattern={}, ...` literally under the old facade. Added a regression test that captures the getTables DEBUG line and asserts the placeholders are substituted. --- .../jdbc/metadata/DatabaseMetaDataImpl.java | 10 +++--- .../src/main/javacc/ClickHouseSqlParser.jj | 8 ++--- .../jdbc/metadata/DatabaseMetaDataTest.java | 35 +++++++++++++++++++ 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java index 9ad3933c8..715c6ac00 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java @@ -11,9 +11,9 @@ import com.clickhouse.jdbc.internal.DetachedResultSet; import com.clickhouse.jdbc.internal.ExceptionUtils; import com.clickhouse.jdbc.internal.JdbcUtils; -import com.clickhouse.logging.Logger; -import com.clickhouse.logging.LoggerFactory; import com.google.common.collect.ImmutableMap; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.sql.Connection; import java.sql.JDBCType; @@ -1132,7 +1132,7 @@ private static String generateSqlTypeSizes(String columnName) { ClickHouseDataType dt = c.getDataType(); type = JdbcUtils.convertToSqlType(dt); } catch (Exception e) { - log.debug("Failed to convert column data type to SQL type: %s", typeName, e); + log.debug("Failed to convert column data type to SQL type: {}", typeName, e); type = JDBCType.OTHER; // In case of error, return SQL type 0 } } @@ -1339,10 +1339,10 @@ public ResultSet getTypeInfo() throws SQLException { try { type = JdbcUtils.convertToSqlType(ClickHouseDataType.valueOf(typeName)); } catch (IllegalArgumentException e) { - log.debug("Unknown type: %s. Please check for a new version of the client.", typeName); + log.debug("Unknown type: {}. Please check for a new version of the client.", typeName); type = JDBCType.OTHER; } catch (Exception e) { - log.debug("Failed to get SQL type for type: %s", typeName, e); + log.debug("Failed to get SQL type for type: {}", typeName, e); type = JDBCType.OTHER; } } diff --git a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj index fa245c77a..5d82adaf4 100644 --- a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj +++ b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj @@ -41,8 +41,8 @@ import java.util.HashSet; import java.util.Collection; import com.clickhouse.client.ClickHouseConfig; -import com.clickhouse.logging.Logger; -import com.clickhouse.logging.LoggerFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class ClickHouseSqlParser { private static final boolean DEBUG = false; @@ -98,8 +98,8 @@ public class ClickHouseSqlParser { if (DEBUG) { throw new IllegalArgumentException(e); } else { - log.warn("%s. If you believe the SQL is valid, please feel free to open an issue on Github with this warning.", e.getMessage()); - log.debug("Unparsable SQL: %s", sql); + log.warn("{}. If you believe the SQL is valid, please feel free to open an issue on Github with this warning.", e.getMessage()); + log.debug("Unparsable SQL: {}", sql); } } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java index 7ea68db19..1050439a9 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java @@ -12,6 +12,8 @@ import org.testng.SkipException; import org.testng.annotations.Test; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.ResultSet; @@ -521,6 +523,39 @@ public void testGetTables() throws Exception { } } + /** + * Regression test for issue #2970. {@code DatabaseMetaDataImpl} logged through the deprecated + * {@code com.clickhouse.logging} facade, which formats with {@link java.util.Formatter} ({@code %s}). + * Its SLF4J-style {@code "{}"} placeholders (such as the {@code getTables} entry log) therefore rendered + * literally instead of substituting the arguments. After migrating the class to SLF4J the placeholders + * are substituted. Captures {@code System.err} (the slf4j-simple output target) around a {@code getTables} + * call and asserts the emitted DEBUG line substitutes its arguments rather than printing a literal "{}". + */ + @Test(groups = { "integration" }) + public void testGetTablesDebugSubstitutesPlaceholders() throws Exception { + final String schemaProbe = "log_placeholder_probe_" + System.nanoTime(); + final ByteArrayOutputStream captured = new ByteArrayOutputStream(); + final PrintStream originalErr = System.err; + final String logged; + try (Connection conn = getJdbcConnection()) { + DatabaseMetaData dbmd = conn.getMetaData(); + // slf4j-simple (the jdbc-v2 test binding) writes to System.err, and simplelogger.properties + // enables DEBUG for com.clickhouse.jdbc, so the getTables entry log is emitted and captured here. + System.setErr(new PrintStream(captured, true, "UTF-8")); + try (ResultSet rs = dbmd.getTables(null, schemaProbe, "no_such_table%", null)) { + // getTables logs its four arguments at DEBUG on entry; the lookup itself matches nothing. + } finally { + System.err.flush(); + System.setErr(originalErr); + } + logged = captured.toString("UTF-8"); + } + assertFalse(logged.contains("catalog={}"), + "getTables logged a literal '{}' placeholder instead of substituting its arguments:\n" + logged); + assertTrue(logged.contains(schemaProbe), + "getTables did not substitute the schemaPattern argument into its DEBUG log:\n" + logged); + } + @Test(groups = { "integration" }) public void testGetPrimaryKeys() throws Exception { From f3c397b5133e5fa3fd915cc1ac081c8a05452078 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:21:22 +0000 Subject: [PATCH 3/7] [client-v2, jdbc-v2] Satisfy SonarCloud new-code gate on the logging cleanup Consolidate the three byte-identical retry warnings in Client.insert/query into a single logRetryAndSelectNextNode(...) helper with a per-operation label. The retry is now logged in exactly one place and the previously duplicated new lines become textually distinct, clearing the new-code duplication SonarCloud flagged. Add DatabaseMetaDataImplMutatorTest covering the unknown/null type-name fallback to JDBCType.OTHER in the getColumns and getTypeInfo result-set mutators, so the DEBUG fallback logs (previously uncovered new lines) are exercised and new-code coverage is restored. No behaviour change: the retry branch still logs then advances to the next node, and unresolved type names still degrade to JDBCType.OTHER. --- .../com/clickhouse/client/api/Client.java | 21 +++++-- .../DatabaseMetaDataImplMutatorTest.java | 58 +++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImplMutatorTest.java 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 8309bbf71..15f4a0d93 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 @@ -1499,8 +1499,7 @@ public CompletableFuture insert(String tableName, List data, lastException = httpClientHelper.wrapException(msg, e, requestSettings.getQueryId()); if (httpClientHelper.shouldRetry(e, requestSettings.getAllSettings()) && requestIsNotCancelled(queryId)) { if (i < maxAttempts) { - LOG.warn("{}, endpoint: {}, cause: {}. Retrying.", msg, selectedEndpoint, e.getMessage()); - selectedEndpoint = nodeSelector.getNextAliveNode(selectedEndpoint); + selectedEndpoint = logRetryAndSelectNextNode("Insert", i + 1, maxAttempts + 1, requestSettings.getQueryId(), selectedEndpoint, e); } else { nodeSelector.getNextAliveNode(selectedEndpoint); } @@ -1705,8 +1704,7 @@ public CompletableFuture insert(String tableName, lastException = httpClientHelper.wrapException(msg, e, requestSettings.getQueryId()); if (httpClientHelper.shouldRetry(e, requestSettings.getAllSettings()) && requestIsNotCancelled(requestSettings.getQueryId())) { if (i < maxAttempts) { - LOG.warn("{}, endpoint: {}, cause: {}. Retrying.", msg, selectedEndpoint, e.getMessage()); - selectedEndpoint = nodeSelector.getNextAliveNode(selectedEndpoint); + selectedEndpoint = logRetryAndSelectNextNode("Insert (stream)", i + 1, maxAttempts + 1, requestSettings.getQueryId(), selectedEndpoint, e); } else { nodeSelector.getNextAliveNode(selectedEndpoint); } @@ -1849,8 +1847,7 @@ public CompletableFuture query(String sqlQuery, Map query(String sqlQuery, Map> mutator(String fieldName) throws Exception { + Field field = DatabaseMetaDataImpl.class.getDeclaredField(fieldName); + field.setAccessible(true); + return (Consumer>) field.get(null); + } + + private static Object resolveDataType(String fieldName, String typeName) throws Exception { + Map row = new HashMap<>(); + row.put("TYPE_NAME", typeName); + mutator(fieldName).accept(row); + return row.get("DATA_TYPE"); + } + + @DataProvider(name = "typeNameMutators") + public Object[][] typeNameMutators() { + return new Object[][] { + {"DATA_TYPE_VALUE_FUNCTION"}, + {"TYPE_INFO_VALUE_FUNCTION"}, + }; + } + + @Test(groups = {"unit"}, dataProvider = "typeNameMutators") + public void testUnknownTypeNameFallsBackToOther(String mutatorField) throws Exception { + Object dataType = resolveDataType(mutatorField, "ThisTypeDoesNotExist_" + System.nanoTime()); + assertEquals(dataType, JDBCType.OTHER.getVendorTypeNumber(), + mutatorField + " must map an unrecognised type name to JDBCType.OTHER"); + } + + @Test(groups = {"unit"}, dataProvider = "typeNameMutators") + public void testNullTypeNameFallsBackToOther(String mutatorField) throws Exception { + Object dataType = resolveDataType(mutatorField, null); + assertEquals(dataType, JDBCType.OTHER.getVendorTypeNumber(), + mutatorField + " must map a null type name to JDBCType.OTHER"); + } +} From 6fb6546a38533040937d33ef7451c1878d76d2c3 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:22:13 +0000 Subject: [PATCH 4/7] [jdbc-v2] Make getTables slf4j-placeholder test deterministic under coverage testGetTablesDebugSubstitutesPlaceholders captured System.err and asserted the getTables DEBUG line substituted its {} placeholders. It relied on simplelogger.properties enabling DEBUG for com.clickhouse.jdbc, but the Coverage workflow (analysis.yml) deletes simplelogger.* before `mvn -Pcoverage verify`, so DEBUG was off, nothing was captured, and the assertion failed. That single failsafe failure aborted the jdbc-v2 module before jacoco-merge/jacoco-aggregate ran, so SonarCloud saw no coverage for any jdbc-v2 file and the new-code coverage gate failed (71.4%). Pin the SLF4J migration deterministically by asserting the class logger is an org.slf4j.Logger (the old com.clickhouse.logging facade rendered {} literally), and only attempt the System.err capture when DEBUG is actually enabled. The module now builds under coverage, so the existing DatabaseMetaDataImplMutatorTest unit coverage of the debug fallback lines is reported. --- .../jdbc/metadata/DatabaseMetaDataTest.java | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java index 1050439a9..670e84c9b 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java @@ -14,6 +14,7 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; +import java.lang.reflect.Field; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.ResultSet; @@ -524,23 +525,44 @@ public void testGetTables() throws Exception { } /** - * Regression test for issue #2970. {@code DatabaseMetaDataImpl} logged through the deprecated - * {@code com.clickhouse.logging} facade, which formats with {@link java.util.Formatter} ({@code %s}). - * Its SLF4J-style {@code "{}"} placeholders (such as the {@code getTables} entry log) therefore rendered - * literally instead of substituting the arguments. After migrating the class to SLF4J the placeholders - * are substituted. Captures {@code System.err} (the slf4j-simple output target) around a {@code getTables} - * call and asserts the emitted DEBUG line substitutes its arguments rather than printing a literal "{}". + * Regression test for issue #2970. {@code DatabaseMetaDataImpl} previously logged through the + * deprecated {@code com.clickhouse.logging} facade, which formats with {@link java.util.Formatter} + * ({@code %s}); its SLF4J-style {@code "{}"} placeholders therefore rendered literally instead of + * substituting the arguments. The class was migrated to SLF4J so the placeholders substitute. + * + *

The migration is pinned deterministically by asserting the logger facade is an + * {@link org.slf4j.Logger} (independent of the active logging backend or level): the old + * {@code com.clickhouse.logging.Logger} is a different type, so a revert fails this assertion. + * When DEBUG is enabled for this logger — the default test binding via {@code simplelogger.properties} + * — the test additionally captures {@code System.err} around a {@code getTables} call and confirms the + * emitted DEBUG line substitutes its arguments at runtime. Some CI jobs (e.g. the coverage job) run + * without {@code simplelogger.properties}, disabling DEBUG; there the capture is skipped and the facade + * assertion still guards the regression. */ @Test(groups = { "integration" }) public void testGetTablesDebugSubstitutesPlaceholders() throws Exception { + Field logField = DatabaseMetaDataImpl.class.getDeclaredField("log"); + logField.setAccessible(true); + Object logger = logField.get(null); + assertTrue(logger instanceof org.slf4j.Logger, + "DatabaseMetaDataImpl must log via org.slf4j.Logger so '{}' placeholders substitute, was: " + + (logger == null ? "null" : logger.getClass().getName())); + + // slf4j-simple derives a logger's level at creation time from simplelogger.properties / system + // properties; the coverage CI job strips simplelogger.properties, so DEBUG is off there and there + // is nothing to capture. Only attempt the runtime capture when DEBUG is actually emitted. + if (!((org.slf4j.Logger) logger).isDebugEnabled()) { + return; + } + final String schemaProbe = "log_placeholder_probe_" + System.nanoTime(); final ByteArrayOutputStream captured = new ByteArrayOutputStream(); final PrintStream originalErr = System.err; final String logged; try (Connection conn = getJdbcConnection()) { DatabaseMetaData dbmd = conn.getMetaData(); - // slf4j-simple (the jdbc-v2 test binding) writes to System.err, and simplelogger.properties - // enables DEBUG for com.clickhouse.jdbc, so the getTables entry log is emitted and captured here. + // slf4j-simple (the jdbc-v2 test binding) writes to System.err, so the getTables entry log + // is emitted and captured here. System.setErr(new PrintStream(captured, true, "UTF-8")); try (ResultSet rs = dbmd.getTables(null, schemaProbe, "no_such_table%", null)) { // getTables logs its four arguments at DEBUG on entry; the lookup itself matches nothing. From f77500dc2f06b3344f132f43be37ab0d83e39688 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:59:03 +0000 Subject: [PATCH 5/7] Address review (#2970): keep parser WARN generic, name retry-log cause class - ClickHouseSqlParser: the parse-failure WARN no longer includes the parser exception message (a JavaCC parse error embeds offending SQL token text, so it could expose credentials/PII). WARN is now a generic message; the raw SQL and the parse exception are logged only at DEBUG (throwable attached). Fixed the "Github" -> "GitHub" wording. - Client.logRetryAndSelectNextNode: the consolidated retry WARN now names the failure cause's exception class in addition to its message, so it stays informative when getMessage() is null (still no stack trace attached). - DatabaseMetaDataTest: the getTables SLF4J-substitution assertions only run when System.err capture actually produced output; the logger-facade assertion remains the deterministic regression guard. - Added regression tests: ClickHouseSqlParserLoggingTest (a parse-failure WARN must not expose the SQL or the parser error) and ClientFailoverUnitTest.testRetryWarnNamesExceptionClass. --- CHANGELOG.md | 9 ++-- .../com/clickhouse/client/api/Client.java | 5 ++- .../client/api/ClientFailoverUnitTest.java | 41 +++++++++++++++++++ .../src/main/javacc/ClickHouseSqlParser.jj | 4 +- .../jdbc/metadata/DatabaseMetaDataTest.java | 6 +++ 5 files changed, 57 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e28999d37..0ed54f940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,14 +21,15 @@ - **[client-v2, jdbc-v2]** Cleaned up noisy and potentially sensitive logging in `jdbc-v2` and `client-v2` (https://github.com/ClickHouse/clickhouse-java/issues/2970). SQL that fails to parse is no longer logged - in full at `WARN` (it could contain passwords/PII); the parser now warns without the statement and logs - the raw SQL only at `DEBUG`. In `jdbc-v2`: unknown-type metadata fallbacks (mapping to `JDBCType.OTHER`) + in full at `WARN` (it could contain passwords/PII); the parser now warns without the statement or the + parser error detail (which can also echo statement tokens), and logs the raw SQL and the parse exception + only at `DEBUG`. In `jdbc-v2`: unknown-type metadata fallbacks (mapping to `JDBCType.OTHER`) log at `DEBUG` instead of `ERROR`; `ExceptionUtils.toSqlState` no longer emits a per-conversion `DEBUG` stack trace that often included SQL; the per-element `TRACE` in prepared-statement parameter encoding and the duplicate per-execution SQL `TRACE`s were removed (the query is still traced once); and the static-init driver banner was dropped. In `client-v2`: a retried request failure is now logged once as a - single consolidated retry `WARN` (operation, attempt, endpoint, query id and cause) rather than a - transport `WARN` followed by a stack-trace `WARN`; the two per-operation compression `DEBUG`s were removed + single consolidated retry `WARN` (operation, attempt, endpoint, query id, and the failure cause's exception + class and message) rather than a transport `WARN` followed by a stack-trace `WARN`; the two per-operation compression `DEBUG`s were removed (compression settings are already logged once at initialization); and the LZ4 compressor/decompressor `DEBUG`s now report the algorithm and buffer size instead of an unusable object identity. The `jdbc-v2` `DatabaseMetaDataImpl` and `ClickHouseSqlParser` were also migrated from the deprecated 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 15f4a0d93..9d24c5377 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 @@ -1875,8 +1875,9 @@ public CompletableFuture query(String sqlQuery, Map connection refused on every attempt + .setUsername("default") + .setPassword("") + .setDefaultDatabase("default") + .setMaxRetries(2) + .build()) { + try { + client.query("SELECT 1").get(30, TimeUnit.SECONDS); + Assert.fail("a query against a dead endpoint should fail after exhausting retries"); + } catch (Exception expected) { + // every attempt hits the dead endpoint + } + } finally { + System.err.flush(); + System.setErr(originalErr); + } + + StringBuilder retryWarns = new StringBuilder(); + for (String line : captured.toString("UTF-8").split("\\R")) { + if (line.contains(" WARN ") && line.contains("Retrying.")) { + retryWarns.append(line).append('\n'); + } + } + String warn = retryWarns.toString(); + Assert.assertFalse(warn.isEmpty(), + "expected a consolidated retry WARN to be emitted:\n" + captured.toString("UTF-8")); + // The cause must name the exception class (informative even when getMessage() is null), + // not just its message. + Assert.assertTrue(Pattern.compile("cause: [\\w$.]+(Exception|Error):").matcher(warn).find(), + "retry WARN should name the exception class in the cause, was:\n" + warn); + } } diff --git a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj index 5d82adaf4..23c650aa6 100644 --- a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj +++ b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj @@ -98,8 +98,8 @@ public class ClickHouseSqlParser { if (DEBUG) { throw new IllegalArgumentException(e); } else { - log.warn("{}. If you believe the SQL is valid, please feel free to open an issue on Github with this warning.", e.getMessage()); - log.debug("Unparsable SQL: {}", sql); + log.warn("Failed to parse SQL. If you believe the SQL is valid, please feel free to open an issue on GitHub with this warning. Enable DEBUG logging for details."); + log.debug("Unparsable SQL: {}", sql, e); } } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java index 670e84c9b..c9ca30edd 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java @@ -572,6 +572,12 @@ public void testGetTablesDebugSubstitutesPlaceholders() throws Exception { } logged = captured.toString("UTF-8"); } + // The logger-facade assertion above is the deterministic guard; the System.err capture is + // best-effort (a cached stream or a non-slf4j-simple binding could yield nothing), so the + // substitution assertions only run when output was actually captured. + if (logged.isEmpty()) { + return; + } assertFalse(logged.contains("catalog={}"), "getTables logged a literal '{}' placeholder instead of substituting its arguments:\n" + logged); assertTrue(logged.contains(schemaProbe), From d87463c95e1e2c902150a0b9b01688411f58098c Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:01:01 +0000 Subject: [PATCH 6/7] Add parser-logging regression test (#2970) Pins that a parse failure does not expose the SQL statement or the parser error detail at WARN. Placed in com.clickhouse.jdbc.internal (the parser package com.clickhouse.jdbc.internal.parser.javacc is .gitignored for generated sources). --- .../ClickHouseSqlParserLoggingTest.java | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/ClickHouseSqlParserLoggingTest.java diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/ClickHouseSqlParserLoggingTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/ClickHouseSqlParserLoggingTest.java new file mode 100644 index 000000000..4fd88c07b --- /dev/null +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/ClickHouseSqlParserLoggingTest.java @@ -0,0 +1,70 @@ +package com.clickhouse.jdbc.internal; + +import com.clickhouse.jdbc.internal.parser.javacc.ClickHouseSqlParser; +import com.clickhouse.jdbc.internal.parser.javacc.ClickHouseSqlStatement; +import org.testng.annotations.Test; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; + +/** + * Verifies that a parse failure does not expose the SQL statement or the parser error detail at + * {@code WARN}. The statement that failed to parse can carry credentials or PII, so the {@code WARN} + * stays a generic message; the raw SQL and the parser exception are only acceptable at {@code DEBUG}. + */ +public class ClickHouseSqlParserLoggingTest { + + @Test(groups = { "unit" }) + public void testParseFailureWarnDoesNotExposeSql() throws Exception { + String secret = "s3cret_db_" + System.nanoTime(); + // A trailing identifier after USE is unexpected, so parsing fails with the token echoed + // in the parser error message. + String sql = "USE " + secret + " " + secret; + + // Precondition: the raw parser error embeds the offending SQL token, so logging it (or the + // SQL) at WARN would expose statement contents. + String rawError = null; + try { + new ClickHouseSqlParser(sql, null).sql(); + fail("expected the invalid SQL to fail parsing"); + } catch (Exception e) { + rawError = e.getMessage(); + } + assertNotNull(rawError, "parser error should carry a message"); + assertTrue(rawError.contains(secret), "the parser error is expected to embed the SQL token"); + + ByteArrayOutputStream captured = new ByteArrayOutputStream(); + PrintStream originalErr = System.err; + System.setErr(new PrintStream(captured, true, "UTF-8")); + ClickHouseSqlStatement[] stmts; + try { + stmts = ClickHouseSqlParser.parse(sql, null); + } finally { + System.err.flush(); + System.setErr(originalErr); + } + + // A parse failure degrades to a single fallback statement rather than throwing. + assertNotNull(stmts); + assertTrue(stmts.length >= 1); + + // WARN is always enabled, so the parse-failure warning is emitted here regardless of the + // DEBUG configuration. Isolate the WARN-level line(s) and assert they neither echo the parser + // error nor expose the SQL token (both belong only at DEBUG). + StringBuilder warnLines = new StringBuilder(); + for (String line : captured.toString("UTF-8").split("\\R")) { + if (line.contains(" WARN ") && line.contains("ClickHouseSqlParser")) { + warnLines.append(line).append('\n'); + } + } + String warn = warnLines.toString(); + assertFalse(warn.isEmpty(), "expected a parse-failure WARN to be emitted"); + assertFalse(warn.contains(secret), "parse-failure WARN exposed the SQL token:\n" + warn); + assertFalse(warn.contains("Encountered"), "parse-failure WARN echoed the parser error detail:\n" + warn); + } +} From 350f880c837eeda966b93716754a49f9da45c993 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:42:20 +0000 Subject: [PATCH 7/7] Address review (#2970): string-concat exception log sites, move retry +1 into helper Per @chernser's review on #2973: - SLF4J placeholders do not bind a Throwable passed as an argument (that is a separate (String, Throwable) overload), so the log sites that pass an exception alongside {} now use string concatenation with the throwable as the trailing argument: ClickHouseSqlParser parse-failure DEBUG and the two DatabaseMetaDataImpl unknown-type DEBUG fallbacks (getColumns / getTypeInfo). The one remaining {} site with no exception argument is left as-is. - Moved the display '+1' arithmetic (1-based attempt, total attempts) out of the three retry call sites and into logRetryAndSelectNextNode; the emitted WARN is unchanged. Strengthened ClientFailoverUnitTest to pin 'attempt 1 of 3'. --- .../main/java/com/clickhouse/client/api/Client.java | 10 +++++----- .../clickhouse/client/api/ClientFailoverUnitTest.java | 5 +++++ .../clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java | 4 ++-- jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj | 2 +- 4 files changed, 13 insertions(+), 8 deletions(-) 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 9d24c5377..3d61576ac 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 @@ -1499,7 +1499,7 @@ public CompletableFuture insert(String tableName, List data, lastException = httpClientHelper.wrapException(msg, e, requestSettings.getQueryId()); if (httpClientHelper.shouldRetry(e, requestSettings.getAllSettings()) && requestIsNotCancelled(queryId)) { if (i < maxAttempts) { - selectedEndpoint = logRetryAndSelectNextNode("Insert", i + 1, maxAttempts + 1, requestSettings.getQueryId(), selectedEndpoint, e); + selectedEndpoint = logRetryAndSelectNextNode("Insert", i, maxAttempts, requestSettings.getQueryId(), selectedEndpoint, e); } else { nodeSelector.getNextAliveNode(selectedEndpoint); } @@ -1704,7 +1704,7 @@ public CompletableFuture insert(String tableName, lastException = httpClientHelper.wrapException(msg, e, requestSettings.getQueryId()); if (httpClientHelper.shouldRetry(e, requestSettings.getAllSettings()) && requestIsNotCancelled(requestSettings.getQueryId())) { if (i < maxAttempts) { - selectedEndpoint = logRetryAndSelectNextNode("Insert (stream)", i + 1, maxAttempts + 1, requestSettings.getQueryId(), selectedEndpoint, e); + selectedEndpoint = logRetryAndSelectNextNode("Insert (stream)", i, maxAttempts, requestSettings.getQueryId(), selectedEndpoint, e); } else { nodeSelector.getNextAliveNode(selectedEndpoint); } @@ -1847,7 +1847,7 @@ public CompletableFuture query(String sqlQuery, Map query(String sqlQuery, Map