From 8a090c0029dd6fec3020fbd9eb124d4ae05ae4c5 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Wed, 17 Jun 2026 23:07:40 -0700 Subject: [PATCH 1/8] Added readonly profile tests --- .../clickhouse/jdbc/JdbcDataTypeTests.java | 13 ++- .../clickhouse/jdbc/ReadonlyProfileTest.java | 80 +++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java index e2dd6a025..ed0b00cb9 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java @@ -2863,12 +2863,23 @@ public void testJSONRead(String json, Object expected) throws Exception { assertTrue(rs.next()); Object jsonObj = rs.getObject(1); + assertTrue(jsonObj instanceof Map, "JSON should be read as a Map"); + Map jsonMap = (Map) jsonObj; + if (expected == null) { expected = jsonToClientMap(json); } - assertEquals(jsonObj, expected); + Map expectedMap = (Map) expected; + + assertEquals(jsonMap, expectedMap); + for (Map.Entry entry : expectedMap.entrySet()) { + assertTrue(jsonMap.containsKey(entry.getKey()), "Map should contain key: " + entry.getKey()); + assertEquals(jsonMap.get(entry.getKey()), entry.getValue(), "Values should match for key: " + entry.getKey()); + } + assertTrue(rs.next()); Object emptyJsonObj = rs.getObject(1); + assertTrue(emptyJsonObj instanceof Map, "Empty JSON should be returned as Map"); assertEquals(emptyJsonObj, EMPTY_JSON); assertFalse(rs.next()); } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java new file mode 100644 index 000000000..33125202f --- /dev/null +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java @@ -0,0 +1,80 @@ +package com.clickhouse.jdbc; + +import com.clickhouse.client.api.ClientConfigProperties; +import com.clickhouse.client.api.internal.ServerSettings; +import org.testng.Assert; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Properties; + +public class ReadonlyProfileTest extends JdbcIntegrationTest { + + @BeforeClass(groups = { "integration" }) + public void setup() throws SQLException { + com.clickhouse.client.ClickHouseServerForTest.beforeSuite(); + try (Connection conn = getJdbcConnection(); + Statement stmt = conn.createStatement()) { + stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS profile_readonly_1 SETTINGS readonly=1"); + stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS profile_readonly_2 SETTINGS readonly=2"); + stmt.execute("CREATE USER IF NOT EXISTS user_readonly_1 IDENTIFIED WITH no_password SETTINGS PROFILE profile_readonly_1"); + stmt.execute("CREATE USER IF NOT EXISTS user_readonly_2 IDENTIFIED WITH no_password SETTINGS PROFILE profile_readonly_2"); + stmt.execute("GRANT SELECT ON *.* TO user_readonly_1"); + stmt.execute("GRANT SELECT ON *.* TO user_readonly_2"); + } + } + + @AfterClass(groups = { "integration" }) + public void teardown() throws SQLException { + try (Connection conn = getJdbcConnection(); + Statement stmt = conn.createStatement()) { + stmt.execute("DROP USER IF EXISTS user_readonly_1"); + stmt.execute("DROP USER IF EXISTS user_readonly_2"); + stmt.execute("DROP SETTINGS PROFILE IF EXISTS profile_readonly_1"); + stmt.execute("DROP SETTINGS PROFILE IF EXISTS profile_readonly_2"); + } + } + + @Test(groups = { "integration" }) + public void testReadonly1CannotChangeSettings() throws Exception { + Properties properties = new Properties(); + properties.setProperty("user", "user_readonly_1"); + properties.setProperty("password", ""); + properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); + + try (Connection conn = getJdbcConnection(properties)) { + try (Statement stmt = conn.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT 1")) { + Assert.fail("Should have thrown an exception because readonly=1 prevents changing settings"); + } + } catch (SQLException e) { + Assert.assertTrue(e.getMessage().contains("Cannot modify"), "Exception message should indicate setting cannot be modified: " + e.getMessage()); + } + } + + @Test(groups = { "integration" }) + public void testReadonly2CanChangeSettings() throws Exception { + if (isVersionMatch("(,24.8]")) { + return; // JSON was introduced in 24.10 + } + + Properties properties = new Properties(); + properties.setProperty("user", "user_readonly_2"); + properties.setProperty("password", ""); + properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); + properties.put(ClientConfigProperties.serverSetting("allow_experimental_json_type"), "1"); + + try (Connection conn = getJdbcConnection(properties)) { + try (Statement stmt = conn.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT '{\"key\":\"value\"}'::JSON")) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getString(1), "{\"key\":\"value\"}"); + } + } + } +} From ec56786f142e05b0c5b9f5b162d30c8fe97bf4aa Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 18 Jun 2026 10:51:41 -0700 Subject: [PATCH 2/8] fixed security --- .../clickhouse/jdbc/ReadonlyProfileTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java index 33125202f..6c3e29b34 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java @@ -12,20 +12,25 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.Properties; +import java.util.UUID; public class ReadonlyProfileTest extends JdbcIntegrationTest { + private String password; + @BeforeClass(groups = { "integration" }) public void setup() throws SQLException { + password = UUID.randomUUID().toString(); com.clickhouse.client.ClickHouseServerForTest.beforeSuite(); try (Connection conn = getJdbcConnection(); Statement stmt = conn.createStatement()) { stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS profile_readonly_1 SETTINGS readonly=1"); stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS profile_readonly_2 SETTINGS readonly=2"); - stmt.execute("CREATE USER IF NOT EXISTS user_readonly_1 IDENTIFIED WITH no_password SETTINGS PROFILE profile_readonly_1"); - stmt.execute("CREATE USER IF NOT EXISTS user_readonly_2 IDENTIFIED WITH no_password SETTINGS PROFILE profile_readonly_2"); - stmt.execute("GRANT SELECT ON *.* TO user_readonly_1"); - stmt.execute("GRANT SELECT ON *.* TO user_readonly_2"); + stmt.execute("CREATE USER IF NOT EXISTS user_readonly_1 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE profile_readonly_1"); + stmt.execute("CREATE USER IF NOT EXISTS user_readonly_2 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE profile_readonly_2"); + String db = getDatabase(); + stmt.execute("GRANT SELECT ON " + db + ".* TO user_readonly_1"); + stmt.execute("GRANT SELECT ON " + db + ".* TO user_readonly_2"); } } @@ -44,7 +49,7 @@ public void teardown() throws SQLException { public void testReadonly1CannotChangeSettings() throws Exception { Properties properties = new Properties(); properties.setProperty("user", "user_readonly_1"); - properties.setProperty("password", ""); + properties.setProperty("password", password); properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); try (Connection conn = getJdbcConnection(properties)) { @@ -65,7 +70,7 @@ public void testReadonly2CanChangeSettings() throws Exception { Properties properties = new Properties(); properties.setProperty("user", "user_readonly_2"); - properties.setProperty("password", ""); + properties.setProperty("password", password); properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); properties.put(ClientConfigProperties.serverSetting("allow_experimental_json_type"), "1"); From c02ea43c897443ab3f9cb682dc4a905519e60748 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 18 Jun 2026 11:04:17 -0700 Subject: [PATCH 3/8] updated docs/ai-review.md about creating users in tests and what to verify --- docs/ai-review.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/ai-review.md b/docs/ai-review.md index 82b5350ce..38b829abc 100644 --- a/docs/ai-review.md +++ b/docs/ai-review.md @@ -137,6 +137,9 @@ Prefer the smallest safe optimization that preserves behavior. ## Tests and docs +**Note**: carefully evaluate tests that involve creating database, users and deal with `GRANT` operations. If user is created within test +it must have secure random password. This password should never be logged or accessible externally. User should be deleted in the `finally` block. If `GRANT` is executted then it must be against test database only. Question if `GRANT ALL` should be used. Test can be run against cloud instance and it is crucial to review what is created in database. Flag any unusuall things in queries. + Call out missing focused tests when a change affects: - public API behavior From c8ce9799343a678aad1abfdc305c2162f83983ba Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 18 Jun 2026 11:26:57 -0700 Subject: [PATCH 4/8] fixed created user and profile to cleanup first --- .../clickhouse/jdbc/ReadonlyProfileTest.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java index 6c3e29b34..c14fbd561 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java @@ -24,13 +24,15 @@ public void setup() throws SQLException { com.clickhouse.client.ClickHouseServerForTest.beforeSuite(); try (Connection conn = getJdbcConnection(); Statement stmt = conn.createStatement()) { - stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS profile_readonly_1 SETTINGS readonly=1"); - stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS profile_readonly_2 SETTINGS readonly=2"); - stmt.execute("CREATE USER IF NOT EXISTS user_readonly_1 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE profile_readonly_1"); - stmt.execute("CREATE USER IF NOT EXISTS user_readonly_2 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE profile_readonly_2"); + stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS jdbc_test_profile_readonly_1 SETTINGS readonly=1"); + stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS jdbc_test_profile_readonly_2 SETTINGS readonly=2"); + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_1"); + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_2"); + stmt.execute("CREATE USER jdbc_test_user_readonly_1 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE jdbc_test_profile_readonly_1"); + stmt.execute("CREATE USER jdbc_test_user_readonly_2 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE jdbc_test_profile_readonly_2"); String db = getDatabase(); - stmt.execute("GRANT SELECT ON " + db + ".* TO user_readonly_1"); - stmt.execute("GRANT SELECT ON " + db + ".* TO user_readonly_2"); + stmt.execute("GRANT SELECT ON " + db + ".* TO jdbc_test_user_readonly_1"); + stmt.execute("GRANT SELECT ON " + db + ".* TO jdbc_test_user_readonly_2"); } } @@ -38,17 +40,17 @@ public void setup() throws SQLException { public void teardown() throws SQLException { try (Connection conn = getJdbcConnection(); Statement stmt = conn.createStatement()) { - stmt.execute("DROP USER IF EXISTS user_readonly_1"); - stmt.execute("DROP USER IF EXISTS user_readonly_2"); - stmt.execute("DROP SETTINGS PROFILE IF EXISTS profile_readonly_1"); - stmt.execute("DROP SETTINGS PROFILE IF EXISTS profile_readonly_2"); + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_1"); + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_2"); + stmt.execute("DROP SETTINGS PROFILE IF EXISTS jdbc_test_profile_readonly_1"); + stmt.execute("DROP SETTINGS PROFILE IF EXISTS jdbc_test_profile_readonly_2"); } } @Test(groups = { "integration" }) public void testReadonly1CannotChangeSettings() throws Exception { Properties properties = new Properties(); - properties.setProperty("user", "user_readonly_1"); + properties.setProperty("user", "jdbc_test_user_readonly_1"); properties.setProperty("password", password); properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); @@ -69,7 +71,7 @@ public void testReadonly2CanChangeSettings() throws Exception { } Properties properties = new Properties(); - properties.setProperty("user", "user_readonly_2"); + properties.setProperty("user", "jdbc_test_user_readonly_2"); properties.setProperty("password", password); properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); properties.put(ClientConfigProperties.serverSetting("allow_experimental_json_type"), "1"); From 0c2730972dc67e5c0830f5a317f98d7f6440ecf8 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 18 Jun 2026 11:31:18 -0700 Subject: [PATCH 5/8] fixed typos --- docs/ai-review.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/ai-review.md b/docs/ai-review.md index 38b829abc..717bddc10 100644 --- a/docs/ai-review.md +++ b/docs/ai-review.md @@ -138,7 +138,9 @@ Prefer the smallest safe optimization that preserves behavior. ## Tests and docs **Note**: carefully evaluate tests that involve creating database, users and deal with `GRANT` operations. If user is created within test -it must have secure random password. This password should never be logged or accessible externally. User should be deleted in the `finally` block. If `GRANT` is executted then it must be against test database only. Question if `GRANT ALL` should be used. Test can be run against cloud instance and it is crucial to review what is created in database. Flag any unusuall things in queries. +it must have secure random password. This password should never be logged or accessible externally. User should be deleted in the `finally` block. +If `GRANT` is executed then it must be against test database only. Question if `GRANT ALL` should be used. Test can be run against cloud instance, +and it is crucial to review what is created in database. Flag any unusual things in queries. Call out missing focused tests when a change affects: From 36b80798d4b75523bb64236baf5a5358d2f4e058 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 18 Jun 2026 12:00:09 -0700 Subject: [PATCH 6/8] fixed password for cloud --- .../test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java index c14fbd561..bcc8c0d9a 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java @@ -20,7 +20,9 @@ public class ReadonlyProfileTest extends JdbcIntegrationTest { @BeforeClass(groups = { "integration" }) public void setup() throws SQLException { - password = UUID.randomUUID().toString(); + // Append fixed character classes so the random password satisfies server + // password-complexity policies (uppercase, lowercase, digit, special). + password = UUID.randomUUID().toString() + "Aa1!"; com.clickhouse.client.ClickHouseServerForTest.beforeSuite(); try (Connection conn = getJdbcConnection(); Statement stmt = conn.createStatement()) { From 74d207312bcb639db0f9a9b3c169c4540d6ef562 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 18 Jun 2026 12:24:54 -0700 Subject: [PATCH 7/8] skip tests in cloud because of configuration limitations --- .../java/com/clickhouse/jdbc/ReadonlyProfileTest.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java index bcc8c0d9a..537ee7b92 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java @@ -3,6 +3,7 @@ import com.clickhouse.client.api.ClientConfigProperties; import com.clickhouse.client.api.internal.ServerSettings; import org.testng.Assert; +import org.testng.SkipException; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -51,6 +52,9 @@ public void teardown() throws SQLException { @Test(groups = { "integration" }) public void testReadonly1CannotChangeSettings() throws Exception { + if (isCloud()) { + throw new SkipException("Should not be tested in cloud because creates users and profiles"); + } Properties properties = new Properties(); properties.setProperty("user", "jdbc_test_user_readonly_1"); properties.setProperty("password", password); @@ -68,8 +72,11 @@ public void testReadonly1CannotChangeSettings() throws Exception { @Test(groups = { "integration" }) public void testReadonly2CanChangeSettings() throws Exception { + if (isCloud()) { + throw new SkipException("Should not be tested in cloud because creates users and profiles"); + } if (isVersionMatch("(,24.8]")) { - return; // JSON was introduced in 24.10 + throw new SkipException("Old versions do not support JSON"); } Properties properties = new Properties(); From c170d3dad77366d30f9aa9050c86bae7b639a2f7 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 18 Jun 2026 12:48:34 -0700 Subject: [PATCH 8/8] Fixed setup test for cloud. Sorry. --- .../test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java index 537ee7b92..2c34ed447 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java @@ -21,6 +21,9 @@ public class ReadonlyProfileTest extends JdbcIntegrationTest { @BeforeClass(groups = { "integration" }) public void setup() throws SQLException { + if (isCloud()) { + return; + } // Append fixed character classes so the random password satisfies server // password-complexity policies (uppercase, lowercase, digit, special). password = UUID.randomUUID().toString() + "Aa1!"; @@ -41,6 +44,9 @@ public void setup() throws SQLException { @AfterClass(groups = { "integration" }) public void teardown() throws SQLException { + if (isCloud()) { + return; + } try (Connection conn = getJdbcConnection(); Statement stmt = conn.createStatement()) { stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_1");