From dfb721da764839f24b0697aa63ad6f1834c4f181 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 19 Mar 2024 15:42:02 -0700 Subject: [PATCH 01/18] parsed roles from stmt --- .../internal/ClickHouseStatementImpl.java | 10 +++ .../clickhouse/jdbc/AccessManagementTest.java | 62 +++++++++++++++++++ .../jdbc/parser/ClickHouseSqlParserTest.java | 22 +++++++ 3 files changed, 94 insertions(+) create mode 100644 clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java diff --git a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java index 20871b6f6..11b6637c4 100644 --- a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java +++ b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java @@ -377,6 +377,16 @@ protected ClickHouseSqlStatement parseSqlStatements(String sql) { throw new IllegalArgumentException("Failed to parse given SQL: " + sql); } + for (ClickHouseSqlStatement stmt : parsedStmts) { + if (stmt.getStatementType() == StatementType.SET) { + try { + connection.getClientInfo().put("_ROLES", stmt.getSettings().get("_ROLES")); + } catch (Exception e) { + //TODO: fix + throw new RuntimeException(e); + } + } + } return getLastStatement(); } diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java new file mode 100644 index 000000000..d21f9714d --- /dev/null +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -0,0 +1,62 @@ +package com.clickhouse.jdbc; + +import com.clickhouse.client.ClickHouseProtocol; +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Arrays; + +public class AccessManagementTest extends JdbcIntegrationTest { + + @Test(groups = "integration") + public void testSetRoleDifferentConnections() throws SQLException { + String httpEndpoint = "http://" + getServerAddress(ClickHouseProtocol.HTTP) + "/"; + String url = String.format("jdbc:ch:%s", httpEndpoint); + ClickHouseDataSource dataSource = new ClickHouseDataSource(url); + + + + try (Connection connection = dataSource.getConnection("default", "")) { + Statement st = connection.createStatement(); + + st.execute("create role ROL1; create role ROL2;"); + st.execute("create user some_user IDENTIFIED WITH no_password"); + st.execute("grant ROL1 to some_user"); + st.execute("grant ROL2 to some_user"); + + } catch (Exception e) { + Assert.fail("Failed", e); + } + + try (Connection connection = dataSource.getConnection("some_user", "")) { + assertRolesEquals(connection, "ROL1", "ROL2"); + + Statement st = connection.createStatement(); + st.execute("set role ROL1"); + assertRolesEquals(connection, "ROL1"); + + } catch (Exception e) { + Assert.fail("Failed", e); + } + } + + private void assertRolesEquals(Connection connection, String ...expected) { + try { + Statement st = connection.createStatement(); + ResultSet resultSet = st.executeQuery("select currentRoles()"); + + Assert.assertTrue(resultSet.next()); + String[] roles = (String[]) resultSet.getArray(1).getArray(); + Arrays.sort(roles); + Arrays.sort(expected); + Assert.assertEquals(roles, expected); + + } catch (Exception e) { + Assert.fail("Failed", e); + } + } +} diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java index 73ac543bf..4bfffefbc 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java @@ -843,6 +843,28 @@ public void testTcl() { assertEquals(stmts[2].getSQL(), "rollback"); } + @Test + public void testSETRoleStatements() { + + final String simpleStmt = "SET ROLE ROL1, ROL2"; + ClickHouseSqlStatement[] stmts = parse(simpleStmt); + Assert.assertEquals(stmts.length, 1); + Assert.assertEquals(stmts[0].getStatementType(), StatementType.SET); + Assert.assertEquals(stmts[0].getOperationType(), OperationType.UNKNOWN); + Assert.assertEquals(stmts[0].getSQL(), simpleStmt); + + final String compositeStmt = "SET ROLE ROL1; SET ROLE ROL2;"; + stmts = parse(compositeStmt); + Assert.assertEquals(stmts.length, 2); + for (ClickHouseSqlStatement stmt : stmts) { + Assert.assertEquals(stmt.getStatementType(), StatementType.SET); + + + } + + + } + // known issue public void testTernaryOperator() { String sql = "select x > 2 ? 'a' : 'b' from (select number as x from system.numbers limit ?)"; From 6f0f17f7aa6328dafbfe0bb5eac34f9d2f03db2a Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 19 Mar 2024 21:20:07 -0700 Subject: [PATCH 02/18] recorded some cases --- .../java/com/clickhouse/jdbc/AccessManagementTest.java | 8 ++++++++ .../clickhouse/jdbc/parser/ClickHouseSqlParserTest.java | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index d21f9714d..33272985b 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -14,6 +14,14 @@ public class AccessManagementTest extends JdbcIntegrationTest { @Test(groups = "integration") public void testSetRoleDifferentConnections() throws SQLException { + /* + Tests: + * Simple expressions + * Composite expressions with multiple roles + * Composite expressions with mixed statements: + - update table1 SET a = 1; set role ROL1; update table2 SET b = 2; set role NONE; + */ + String httpEndpoint = "http://" + getServerAddress(ClickHouseProtocol.HTTP) + "/"; String url = String.format("jdbc:ch:%s", httpEndpoint); ClickHouseDataSource dataSource = new ClickHouseDataSource(url); diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java index 4bfffefbc..f7553c386 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserTest.java @@ -852,14 +852,14 @@ public void testSETRoleStatements() { Assert.assertEquals(stmts[0].getStatementType(), StatementType.SET); Assert.assertEquals(stmts[0].getOperationType(), OperationType.UNKNOWN); Assert.assertEquals(stmts[0].getSQL(), simpleStmt); + Assert.assertNotNull(stmts[0].getSettings().get("_ROLES")); final String compositeStmt = "SET ROLE ROL1; SET ROLE ROL2;"; stmts = parse(compositeStmt); Assert.assertEquals(stmts.length, 2); for (ClickHouseSqlStatement stmt : stmts) { Assert.assertEquals(stmt.getStatementType(), StatementType.SET); - - + Assert.assertNotNull(stmts[0].getSettings().get("_ROLES")); } From 4d4df8a7494367e0db828f34f43ffd86c6914fd0 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Wed, 20 Mar 2024 16:18:02 -0700 Subject: [PATCH 03/18] passed client roles as query parameter --- .../client/http/ClickHouseHttpConnection.java | 5 +++ .../internal/ClickHouseStatementImpl.java | 39 ++++++++++++------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java index 6ef61c976..ce4705284 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java @@ -134,6 +134,11 @@ static String buildQueryParams(ClickHouseRequest request) { appendQueryParameter(builder, settingKey, "0"); } + settingKey = "custom_client_roles"; // TODO: remove custom_ prefix + if (!settings.containsKey(settingKey)) { + appendQueryParameter(builder, settingKey, ""); + } + Optional optionalValue = request.getSessionId(); if (optionalValue.isPresent()) { appendQueryParameter(builder, ClickHouseClientOption.SESSION_ID.getKey(), optionalValue.get()); diff --git a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java index 11b6637c4..4e4343edc 100644 --- a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java +++ b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java @@ -10,11 +10,7 @@ import java.sql.SQLFeatureNotSupportedException; import java.sql.SQLWarning; import java.sql.Statement; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -93,6 +89,28 @@ public class ClickHouseStatementImpl extends JdbcWrapper protected ClickHouseSqlStatement[] parsedStmts; + + private void updateClientRolesAndApply(ClickHouseSqlStatement stmt) { + + try { + + if (stmt.getSettings().get("_ROLES_COUNT") != null) { + ArrayList clientRoles = new ArrayList<>(); + + int i = 0; + while (stmt.getSettings().get("_ROLE_" + i) != null) { + clientRoles.add(stmt.getSettings().get("_ROLE_" + i)); + i++; + } + + request.set("custom_client_roles", String.join(",", clientRoles)); + } + } catch (Exception e) { + //TODO: fix + throw new RuntimeException(e); + } + } + private ClickHouseResponse getLastResponse(Map options, List tables, Map settings) throws SQLException { boolean autoTx = connection.getAutoCommit() && connection.isTransactionSupported(); @@ -104,6 +122,7 @@ private ClickHouseResponse getLastResponse(Map o ClickHouseResponse response = null; for (int i = 0, len = parsedStmts.length; i < len; i++) { ClickHouseSqlStatement stmt = parsedStmts[i]; + updateClientRolesAndApply(stmt); response = processSqlStatement(stmt); if (response != null) { updateResult(stmt, response); @@ -377,16 +396,6 @@ protected ClickHouseSqlStatement parseSqlStatements(String sql) { throw new IllegalArgumentException("Failed to parse given SQL: " + sql); } - for (ClickHouseSqlStatement stmt : parsedStmts) { - if (stmt.getStatementType() == StatementType.SET) { - try { - connection.getClientInfo().put("_ROLES", stmt.getSettings().get("_ROLES")); - } catch (Exception e) { - //TODO: fix - throw new RuntimeException(e); - } - } - } return getLastStatement(); } From 0586afb80a75c9d8c2c0c519ea637af72be862e5 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 21 Mar 2024 15:53:24 -0700 Subject: [PATCH 04/18] updated jj code with role parsing functions. --- .../src/main/javacc/ClickHouseSqlParser.jj | 45 ++++++++++++++++++- .../clickhouse/jdbc/AccessManagementTest.java | 3 +- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj index c9d857280..5c09ab4df 100644 --- a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj +++ b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj @@ -37,6 +37,8 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.HashSet; +import java.util.Collection; import com.clickhouse.client.ClickHouseConfig; import com.clickhouse.logging.Logger; @@ -51,6 +53,7 @@ public class ClickHouseSqlParser { private ClickHouseConfig config; private ParseHandler handler; + private int anyArgsListStart = -1; private boolean tokenIn(int tokenIndex, int... tokens) { boolean matched = false; @@ -121,6 +124,34 @@ public class ClickHouseSqlParser { token_source.reset(); } } + + + private void rememberSetStmtArgsStart() { + if (token.next != null && "ROLE".equalsIgnoreCase(token.next.image)) { + anyArgsListStart = token_source.input_stream.tokenBegin; + } + } + + private void rememberRolesIfSetStmt() { + if (anyArgsListStart > 0) { + HashSet roles = new HashSet<>(); + token_source.attachedAttributes.put("_ROLES", roles); + int stmtLength = token_source.builder.length(); + + StringBuilder roleBuff = new StringBuilder(); + // read until the end of the statement or the beginning of the role list - so 2 chars before the role list + for (int i = stmtLength - 1; i > 0 && i > anyArgsListStart - 2; i--) { + char ch = token_source.builder.charAt(i); + if (Character.isLetterOrDigit(ch)) { + roleBuff.append(ch); + } else if (!roleBuff.isEmpty()) { + roles.add(roleBuff.reverse().toString()); + roleBuff.setLength(0); + } + } + } + anyArgsListStart = -1; + } } PARSER_END(ClickHouseSqlParser) @@ -150,6 +181,7 @@ TOKEN_MGR_DECLS: { final Map positions = new HashMap<>(); final Map settings = new LinkedHashMap<>(); final Set tempTables = new LinkedHashSet<>(); + final Map attachedAttributes = new HashMap<>(); public void CommonTokenAction(Token t) { if (t.kind != ClickHouseSqlParserConstants.SEMICOLON) { @@ -234,6 +266,16 @@ TOKEN_MGR_DECLS: { ClickHouseSqlStatement build(ParseHandler handler) { String sqlStmt = builder.toString(); ClickHouseSqlStatement s = null; + if (attachedAttributes.get("_ROLES") != null && attachedAttributes.get("_ROLES") instanceof Collection){ + Collection roles = (Collection) attachedAttributes.get("_ROLES"); + settings.put("_ROLES_COUNT", String.valueOf(roles.size())); + int i = 0; + for (String role : roles) { + settings.put("_ROLE_" + i, role); + i++; + } + } + if (handler != null) { s = handler.handleStatement( sqlStmt, stmtType, cluster, database, table, input, compressAlgorithm, compressLevel, format, file, parameters, positions, settings, tempTables); @@ -598,7 +640,8 @@ void tableClause(boolean record): { Token t; } { // https://clickhouse.tech/docs/en/sql-reference/statements/set/ // https://clickhouse.tech/docs/en/sql-reference/statements/set-role/ void setStmt(): {} { // not interested - (LOOKAHEAD(2) settingExprList() | anyExprList()) + + (LOOKAHEAD(2) settingExprList() | ( {rememberSetStmtArgsStart(); } anyExprList() {rememberRolesIfSetStmt();})) } // https://clickhouse.tech/docs/en/sql-reference/statements/show/ diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index 33272985b..b11c46a33 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -44,7 +44,8 @@ public void testSetRoleDifferentConnections() throws SQLException { assertRolesEquals(connection, "ROL1", "ROL2"); Statement st = connection.createStatement(); - st.execute("set role ROL1"); +// st.execute("set role ROL1"); + st.execute("set\n role\n ROL1, ROl2"); assertRolesEquals(connection, "ROL1"); } catch (Exception e) { From a4b63bb9578173185dba1dee26c0240d02c74e63 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Fri, 22 Mar 2024 13:31:26 -0700 Subject: [PATCH 05/18] implemented sending roles thru headers --- .../client/http/ClickHouseHttpClient.java | 2 +- .../client/http/ClickHouseHttpConnection.java | 12 +++-- .../internal/ClickHouseStatementImpl.java | 44 ++++++++++++------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java index 2c6f757ac..eb442b399 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java @@ -118,7 +118,7 @@ protected ClickHouseResponse send(ClickHouseRequest sealedRequest) throws Cli httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null), sealedRequest.getExternalTables(), sealedRequest.getOutputStream().orElse(null), ClickHouseHttpConnection.buildUrl(server.getBaseUri(), sealedRequest), - ClickHouseHttpConnection.createDefaultHeaders(config, server, conn.getUserAgent()), + ClickHouseHttpConnection.createDefaultHeaders(config, server, conn.getUserAgent(), sealedRequest), postAction); } else { httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null), diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java index ce4705284..9a7f3c156 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java @@ -134,11 +134,6 @@ static String buildQueryParams(ClickHouseRequest request) { appendQueryParameter(builder, settingKey, "0"); } - settingKey = "custom_client_roles"; // TODO: remove custom_ prefix - if (!settings.containsKey(settingKey)) { - appendQueryParameter(builder, settingKey, ""); - } - Optional optionalValue = request.getSessionId(); if (optionalValue.isPresent()) { appendQueryParameter(builder, ClickHouseClientOption.SESSION_ID.getKey(), optionalValue.get()); @@ -194,10 +189,13 @@ static String buildUrl(String baseUrl, ClickHouseRequest request) { } protected static Map createDefaultHeaders(ClickHouseConfig config, ClickHouseNode server, - String userAgent) { + String userAgent, ClickHouseRequest request) { Map map = new LinkedHashMap<>(); boolean hasAuthorizationHeader = false; // add customer headers + if (request.hasSetting("custom_run_with_roles")) { + map.put("X-ClickHouse-User-Roles", request.getSetting( "custom_run_with_roles", "")); + } for (Entry header : ClickHouseOption .toKeyValuePairs(config.getStrOption(ClickHouseHttpOption.CUSTOM_HEADERS)).entrySet()) { String name = header.getKey().toLowerCase(Locale.ROOT); @@ -368,7 +366,7 @@ protected ClickHouseHttpConnection(ClickHouseNode server, ClickHouseRequest r ClickHouseConfig c = request.getConfig(); this.config = c; - this.defaultHeaders = Collections.unmodifiableMap(createDefaultHeaders(c, server, getUserAgent())); + this.defaultHeaders = Collections.unmodifiableMap(createDefaultHeaders(c, server, getUserAgent(), request)); this.url = buildUrl(server.getBaseUri(), request); log.debug("url [%s]", this.url); } diff --git a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java index 4e4343edc..e263dfe2b 100644 --- a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java +++ b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java @@ -90,24 +90,32 @@ public class ClickHouseStatementImpl extends JdbcWrapper protected ClickHouseSqlStatement[] parsedStmts; - private void updateClientRolesAndApply(ClickHouseSqlStatement stmt) { + private Set getRequestRoles(ClickHouseSqlStatement stmt) { + Set roles = new HashSet<>(); - try { - - if (stmt.getSettings().get("_ROLES_COUNT") != null) { - ArrayList clientRoles = new ArrayList<>(); + for (Map settings : Arrays.asList(connection.getConfig().getCustomSettings(), + stmt.getSettings())) { + int i = 0; + String role; + while ((role = settings.get("_ROLE_" + i)) != null) { + roles.add(role); + i++; + } + } - int i = 0; - while (stmt.getSettings().get("_ROLE_" + i) != null) { - clientRoles.add(stmt.getSettings().get("_ROLE_" + i)); - i++; - } + return roles; + } - request.set("custom_client_roles", String.join(",", clientRoles)); - } - } catch (Exception e) { - //TODO: fix - throw new RuntimeException(e); + private void rememberRoles(Set roles) { + Map settings = connection.getConfig().getCustomSettings(); + int i = 0; + while (settings.remove("_ROLE_" + i) != null) { + i++; + } + i = 0; + for (String role : roles) { + settings.put("_ROLE_" + i, role); + i++; } } @@ -122,7 +130,6 @@ private ClickHouseResponse getLastResponse(Map o ClickHouseResponse response = null; for (int i = 0, len = parsedStmts.length; i < len; i++) { ClickHouseSqlStatement stmt = parsedStmts[i]; - updateClientRolesAndApply(stmt); response = processSqlStatement(stmt); if (response != null) { updateResult(stmt, response); @@ -132,11 +139,16 @@ private ClickHouseResponse getLastResponse(Map o if (stmt.hasFormat()) { request.format(ClickHouseFormat.valueOf(stmt.getFormat())); } + final Set requestRoles = getRequestRoles(stmt); + if (!requestRoles.isEmpty()) { + request.set("custom_run_with_roles", String.join(",", requestRoles)); + } request.query(stmt.getSQL(), queryId = connection.newQueryId()); // TODO skip useless queries to reduce network calls and server load try { response = autoTx ? request.executeWithinTransaction(connection.isImplicitTransactionSupported()) : request.transaction(connection.getTransaction()).executeAndWait(); + rememberRoles(requestRoles); } catch (Exception e) { throw SqlExceptionUtils.handle(e); } finally { From 258729484f04daae025b58cec5a41f47d8d70e77 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Mon, 25 Mar 2024 13:52:22 -0700 Subject: [PATCH 06/18] implemented saving in http client. useing qparams --- .../client/http/ClickHouseHttpClient.java | 27 ++++++++++++- .../client/http/ClickHouseHttpConnection.java | 35 ++++++++++++----- .../http/ClickHouseHttpConnectionTest.java | 5 ++- .../internal/ClickHouseStatementImpl.java | 38 ++++++------------- .../clickhouse/jdbc/AccessManagementTest.java | 2 +- 5 files changed, 66 insertions(+), 41 deletions(-) diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java index eb442b399..b83c14af1 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java @@ -1,13 +1,17 @@ package com.clickhouse.client.http; import java.io.IOException; +import java.io.Serializable; import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletionException; +import java.util.concurrent.ConcurrentSkipListSet; import com.clickhouse.client.AbstractClient; import com.clickhouse.client.ClickHouseConfig; @@ -28,6 +32,8 @@ public class ClickHouseHttpClient extends AbstractClient SUPPORTED = Collections.singletonList(ClickHouseProtocol.HTTP); + private ConcurrentSkipListSet roles = new ConcurrentSkipListSet<>(); + @Override protected boolean checkConnection(ClickHouseHttpConnection connection, ClickHouseNode requestServer, ClickHouseNode currentServer, ClickHouseRequest request) { @@ -113,18 +119,30 @@ protected ClickHouseResponse send(ClickHouseRequest sealedRequest) throws Cli } } : null; + Map additionalParams = null; + if (sealedRequest.hasSetting("_set_roles_stmt")) { + additionalParams = Collections.singletonMap("_roles", sealedRequest.getSettings().get("_set_roles_stmt")); + } else if (!roles.isEmpty()) { + additionalParams = Collections.singletonMap("_roles", roles); + } if (conn.isReusable()) { ClickHouseNode server = sealedRequest.getServer(); httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null), sealedRequest.getExternalTables(), sealedRequest.getOutputStream().orElse(null), - ClickHouseHttpConnection.buildUrl(server.getBaseUri(), sealedRequest), - ClickHouseHttpConnection.createDefaultHeaders(config, server, conn.getUserAgent(), sealedRequest), + ClickHouseHttpConnection.buildUrl(server.getBaseUri(), sealedRequest, additionalParams), + ClickHouseHttpConnection.createDefaultHeaders(config, server, conn.getUserAgent()), postAction); } else { httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null), sealedRequest.getExternalTables(), sealedRequest.getOutputStream().orElse(null), null, null, postAction); } + + // At this point only successful responses are expected + if (sealedRequest.hasSetting("_set_roles_stmt")) { + rememberRoles((Set) sealedRequest.getSettings().get("_set_roles_stmt")); + } + return ClickHouseStreamResponse.of(httpResponse.getConfig(sealedRequest), httpResponse.getInputStream(), sealedRequest.getSettings(), null, httpResponse.summary); } @@ -133,4 +151,9 @@ protected ClickHouseResponse send(ClickHouseRequest sealedRequest) throws Cli public final Class getOptionClass() { return ClickHouseHttpOption.class; } + + private void rememberRoles(Set requestedRoles) { + roles.clear(); + roles.addAll(requestedRoles); + } } diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java index 9a7f3c156..623be2f7e 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java @@ -10,12 +10,14 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Map.Entry; +import java.util.Set; import com.clickhouse.client.ClickHouseClient; import com.clickhouse.client.ClickHouseConfig; @@ -64,6 +66,8 @@ private static StringBuilder appendQueryParameter(StringBuilder builder, String .append(urlEncode(value, StandardCharsets.UTF_8)).append('&'); } + + static String urlEncode(String str, Charset charset) { if (charset == null) { charset = StandardCharsets.UTF_8; @@ -77,11 +81,15 @@ static String urlEncode(String str, Charset charset) { } } - static String buildQueryParams(ClickHouseRequest request) { + static String buildQueryParams(ClickHouseRequest request, Map additionalParams) { if (request == null) { return ""; } + if (additionalParams == null) { + additionalParams = Collections.emptyMap(); + } + ClickHouseConfig config = request.getConfig(); StringBuilder builder = new StringBuilder(); @@ -134,6 +142,13 @@ static String buildQueryParams(ClickHouseRequest request) { appendQueryParameter(builder, settingKey, "0"); } + // Handle additional parameters + if (additionalParams.containsKey("_roles")) { + Serializable value = additionalParams.get("_roles"); + Set roles = !(value instanceof Set) ? Collections.emptySet() : (Set) value; + roles.forEach(role -> appendQueryParameter(builder, "custom_role", role)); + } + Optional optionalValue = request.getSessionId(); if (optionalValue.isPresent()) { appendQueryParameter(builder, ClickHouseClientOption.SESSION_ID.getKey(), optionalValue.get()); @@ -154,6 +169,10 @@ static String buildQueryParams(ClickHouseRequest request) { } for (Map.Entry entry : settings.entrySet()) { + // Skip internal settings + if (entry.getKey().equalsIgnoreCase("_set_roles_stmt")) { + continue; + } appendQueryParameter(builder, entry.getKey(), String.valueOf(entry.getValue())); } @@ -163,7 +182,7 @@ static String buildQueryParams(ClickHouseRequest request) { return builder.toString(); } - static String buildUrl(String baseUrl, ClickHouseRequest request) { + static String buildUrl(String baseUrl, ClickHouseRequest request, Map additionalParams) { StringBuilder builder = new StringBuilder().append(baseUrl); // TODO: Using default until we will remove String context = "/"; @@ -180,7 +199,7 @@ static String buildUrl(String baseUrl, ClickHouseRequest request) { } } - String query = buildQueryParams(request); + String query = buildQueryParams(request, additionalParams); if (!query.isEmpty()) { builder.append('?').append(query); } @@ -189,13 +208,9 @@ static String buildUrl(String baseUrl, ClickHouseRequest request) { } protected static Map createDefaultHeaders(ClickHouseConfig config, ClickHouseNode server, - String userAgent, ClickHouseRequest request) { + String userAgent) { Map map = new LinkedHashMap<>(); boolean hasAuthorizationHeader = false; - // add customer headers - if (request.hasSetting("custom_run_with_roles")) { - map.put("X-ClickHouse-User-Roles", request.getSetting( "custom_run_with_roles", "")); - } for (Entry header : ClickHouseOption .toKeyValuePairs(config.getStrOption(ClickHouseHttpOption.CUSTOM_HEADERS)).entrySet()) { String name = header.getKey().toLowerCase(Locale.ROOT); @@ -366,8 +381,8 @@ protected ClickHouseHttpConnection(ClickHouseNode server, ClickHouseRequest r ClickHouseConfig c = request.getConfig(); this.config = c; - this.defaultHeaders = Collections.unmodifiableMap(createDefaultHeaders(c, server, getUserAgent(), request)); - this.url = buildUrl(server.getBaseUri(), request); + this.defaultHeaders = Collections.unmodifiableMap(createDefaultHeaders(c, server, getUserAgent())); + this.url = buildUrl(server.getBaseUri(), request, Collections.emptyMap()); log.debug("url [%s]", this.url); } diff --git a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java index ca01c30b4..2f67162ef 100644 --- a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java +++ b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java @@ -1,6 +1,7 @@ package com.clickhouse.client.http; import java.io.IOException; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -46,7 +47,7 @@ public void testBuildUrl() { ClickHouseNode server = ClickHouseNode.builder().port(ClickHouseProtocol.HTTP).build(); ClickHouseRequest request = ClickHouseClient.newInstance().read(server); ClickHouseNode s = request.getServer(); - Assert.assertEquals(ClickHouseHttpConnection.buildUrl(server.getBaseUri(), request), + Assert.assertEquals(ClickHouseHttpConnection.buildUrl(server.getBaseUri(), request, Collections.emptyMap()), "http://localhost:8123/?compress=1&extremes=0"); } @@ -67,7 +68,7 @@ public void testDefaultHeaders() { public void testGetBaseUrl() { ClickHouseNode server = ClickHouseNode.of("https://localhost/db1"); ClickHouseRequest request = ClickHouseClient.newInstance().read(server); - Assert.assertEquals(ClickHouseHttpConnection.buildUrl(server.getBaseUri(), request), + Assert.assertEquals(ClickHouseHttpConnection.buildUrl(server.getBaseUri(), request, Collections.emptyMap()), "https://localhost:8443/?compress=1&extremes=0"); try (SimpleHttpConnection c = new SimpleHttpConnection(server, request)) { Assert.assertEquals(c.getBaseUrl(), "https://localhost:8443/"); diff --git a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java index e263dfe2b..eb7a80e05 100644 --- a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java +++ b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseStatementImpl.java @@ -90,33 +90,18 @@ public class ClickHouseStatementImpl extends JdbcWrapper protected ClickHouseSqlStatement[] parsedStmts; - private Set getRequestRoles(ClickHouseSqlStatement stmt) { - Set roles = new HashSet<>(); + private HashSet getRequestRoles(ClickHouseSqlStatement stmt) { + HashSet roles = new HashSet<>(); - for (Map settings : Arrays.asList(connection.getConfig().getCustomSettings(), - stmt.getSettings())) { - int i = 0; - String role; - while ((role = settings.get("_ROLE_" + i)) != null) { - roles.add(role); - i++; - } - } - - return roles; - } - - private void rememberRoles(Set roles) { - Map settings = connection.getConfig().getCustomSettings(); + Map settings = stmt.getSettings(); int i = 0; - while (settings.remove("_ROLE_" + i) != null) { - i++; - } - i = 0; - for (String role : roles) { - settings.put("_ROLE_" + i, role); + String role; + while ((role = settings.get("_ROLE_" + i)) != null) { + roles.add(role); i++; } + + return roles; } private ClickHouseResponse getLastResponse(Map options, @@ -139,16 +124,17 @@ private ClickHouseResponse getLastResponse(Map o if (stmt.hasFormat()) { request.format(ClickHouseFormat.valueOf(stmt.getFormat())); } - final Set requestRoles = getRequestRoles(stmt); + + final HashSet requestRoles = getRequestRoles(stmt); if (!requestRoles.isEmpty()) { - request.set("custom_run_with_roles", String.join(",", requestRoles)); + request.set("_set_roles_stmt", requestRoles); } + request.query(stmt.getSQL(), queryId = connection.newQueryId()); // TODO skip useless queries to reduce network calls and server load try { response = autoTx ? request.executeWithinTransaction(connection.isImplicitTransactionSupported()) : request.transaction(connection.getTransaction()).executeAndWait(); - rememberRoles(requestRoles); } catch (Exception e) { throw SqlExceptionUtils.handle(e); } finally { diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index b11c46a33..83b36805e 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -45,7 +45,7 @@ public void testSetRoleDifferentConnections() throws SQLException { Statement st = connection.createStatement(); // st.execute("set role ROL1"); - st.execute("set\n role\n ROL1, ROl2"); + st.execute("set\n role\n ROL1, ROL2"); assertRolesEquals(connection, "ROL1"); } catch (Exception e) { From 7a5ce8d6aa1a99ce46d711b9c0a86bc0321d7317 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Mon, 25 Mar 2024 16:13:58 -0700 Subject: [PATCH 07/18] added tests for different exotic role names --- .../http/ClickHouseHttpConnectionTest.java | 41 +++++++++++++++++++ .../clickhouse/jdbc/AccessManagementTest.java | 9 ++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java index 2f67162ef..a1be0dd1b 100644 --- a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java +++ b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java @@ -1,7 +1,13 @@ package com.clickhouse.client.http; import java.io.IOException; +import java.io.Serializable; +import java.net.URI; +import java.net.URL; +import java.net.URLDecoder; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; @@ -16,8 +22,12 @@ import com.clickhouse.data.ClickHouseInputStream; import com.clickhouse.data.ClickHouseOutputStream; +import org.apache.hc.core5.net.URIBuilder; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import org.testng.collections.Sets; +import org.testng.internal.invokers.Arguments; public class ClickHouseHttpConnectionTest { static class SimpleHttpConnection extends ClickHouseHttpConnection { @@ -74,4 +84,35 @@ public void testGetBaseUrl() { Assert.assertEquals(c.getBaseUrl(), "https://localhost:8443/"); } } + + @Test(groups = { "unit" }, dataProvider = "roles") + public void testRolesQParam(String role) { + ClickHouseNode server = ClickHouseNode.of("https://localhost/db1"); + ClickHouseRequest request = ClickHouseClient.newInstance().read(server); + Map additionalParams = Collections.singletonMap("_roles", (Serializable) Sets.newHashSet(role)); + String url = ClickHouseHttpConnection.buildUrl(server.getBaseUri(), request, additionalParams); + + try { + URIBuilder uriBuilder = new URIBuilder(new URI(url)); + String queryRole = uriBuilder.getQueryParams().stream() + .filter(p -> "custom_role".equals(p.getName())) + .findFirst() + .map(p -> p.getValue()) + .orElse(null); + Assert.assertEquals(role, queryRole); + } catch (Exception e) { + Assert.fail("Failed to build URL with roles query parameter", e); + } + } + + @DataProvider(name = "roles") + private static Object[][] getRolesQParamArguments() { + return new Object[][] { + { "ROLE1" }, + { "ROl2,," }, + { "role☺," }, + { "ROL3∕" }, + + }; + } } diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index 83b36805e..577a1794d 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -31,21 +31,21 @@ public void testSetRoleDifferentConnections() throws SQLException { try (Connection connection = dataSource.getConnection("default", "")) { Statement st = connection.createStatement(); - st.execute("create role ROL1; create role ROL2;"); + st.execute("create role ROL1; create role \"role☺,\";"); st.execute("create user some_user IDENTIFIED WITH no_password"); st.execute("grant ROL1 to some_user"); - st.execute("grant ROL2 to some_user"); + st.execute("grant \"role☺,\" to some_user"); } catch (Exception e) { Assert.fail("Failed", e); } try (Connection connection = dataSource.getConnection("some_user", "")) { - assertRolesEquals(connection, "ROL1", "ROL2"); + assertRolesEquals(connection, "ROL1", "role☺,"); Statement st = connection.createStatement(); // st.execute("set role ROL1"); - st.execute("set\n role\n ROL1, ROL2"); + st.execute("set\n role\n ROL1, \"role☺,\""); assertRolesEquals(connection, "ROL1"); } catch (Exception e) { @@ -62,6 +62,7 @@ private void assertRolesEquals(Connection connection, String ...expected) { String[] roles = (String[]) resultSet.getArray(1).getArray(); Arrays.sort(roles); Arrays.sort(expected); + System.out.println("currentRoles = " + Arrays.asList(roles)); Assert.assertEquals(roles, expected); } catch (Exception e) { From c33c79b2b12745d8e09e2c7b4f2d7c6a5bb7e5b7 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 26 Mar 2024 15:36:51 -0700 Subject: [PATCH 08/18] fixed parsing role names. Using final role qparam name --- .../client/http/ClickHouseHttpConnection.java | 2 +- .../src/main/javacc/ClickHouseSqlParser.jj | 20 ++++++++++++------- .../clickhouse/jdbc/AccessManagementTest.java | 15 ++++++++------ 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java index 623be2f7e..50d6c1a32 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java @@ -146,7 +146,7 @@ static String buildQueryParams(ClickHouseRequest request, Map roles = !(value instanceof Set) ? Collections.emptySet() : (Set) value; - roles.forEach(role -> appendQueryParameter(builder, "custom_role", role)); + roles.forEach(role -> appendQueryParameter(builder, "role", role)); } Optional optionalValue = request.getSessionId(); diff --git a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj index 5c09ab4df..223b8833e 100644 --- a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj +++ b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj @@ -132,21 +132,27 @@ public class ClickHouseSqlParser { } } - private void rememberRolesIfSetStmt() { +private void rememberRolesIfSetStmt() { if (anyArgsListStart > 0) { HashSet roles = new HashSet<>(); token_source.attachedAttributes.put("_ROLES", roles); int stmtLength = token_source.builder.length(); StringBuilder roleBuff = new StringBuilder(); - // read until the end of the statement or the beginning of the role list - so 2 chars before the role list - for (int i = stmtLength - 1; i > 0 && i > anyArgsListStart - 2; i--) { + boolean isQuoted = false; + for (int i = anyArgsListStart; i < stmtLength; i++) { char ch = token_source.builder.charAt(i); - if (Character.isLetterOrDigit(ch)) { - roleBuff.append(ch); - } else if (!roleBuff.isEmpty()) { - roles.add(roleBuff.reverse().toString()); + if (ch == '"' && isQuoted ) { + roles.add(roleBuff.toString()); + roleBuff.setLength(0); + isQuoted = false; + } else if (ch == '"' && !isQuoted) { + isQuoted = true; + } else if (ch == ',' && !isQuoted) { + roles.add(roleBuff.toString()); roleBuff.setLength(0); + } else { + roleBuff.append(ch); } } } diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index 577a1794d..86358a998 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -31,22 +31,25 @@ public void testSetRoleDifferentConnections() throws SQLException { try (Connection connection = dataSource.getConnection("default", "")) { Statement st = connection.createStatement(); - st.execute("create role ROL1; create role \"role☺,\";"); +// st.execute("DROP ROLE IF EXISTS ROL1, \"role☺,\""); + st.execute("DROP ROLE IF EXISTS ROL1, \"ROL2,☺\""); + st.execute("DROP USER IF EXISTS some_user"); + st.execute("create role ROL1; create role \"ROL2,☺\";"); st.execute("create user some_user IDENTIFIED WITH no_password"); st.execute("grant ROL1 to some_user"); - st.execute("grant \"role☺,\" to some_user"); + st.execute("grant \"ROL2,☺\" to some_user"); } catch (Exception e) { Assert.fail("Failed", e); } try (Connection connection = dataSource.getConnection("some_user", "")) { - assertRolesEquals(connection, "ROL1", "role☺,"); + assertRolesEquals(connection, "ROL1", "ROL2,☺"); Statement st = connection.createStatement(); -// st.execute("set role ROL1"); - st.execute("set\n role\n ROL1, \"role☺,\""); - assertRolesEquals(connection, "ROL1"); + st.execute("set role \"ROL2,☺\""); +// st.execute("set\n role\n ROL1, \"ROL2,\""); + assertRolesEquals(connection, "ROL2,☺"); } catch (Exception e) { Assert.fail("Failed", e); From 7e4c8068a2d900c733422bb8e7247b827555730d Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 26 Mar 2024 22:32:47 -0700 Subject: [PATCH 09/18] added new setting to switch remembering roles --- .../client/ClickHouseException.java | 1 + .../client/http/ClickHouseHttpClient.java | 23 ++++-- .../http/config/ClickHouseHttpOption.java | 9 ++- .../src/main/javacc/ClickHouseSqlParser.jj | 8 +- .../clickhouse/jdbc/AccessManagementTest.java | 78 ++++++++++++------- 5 files changed, 80 insertions(+), 39 deletions(-) diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseException.java b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseException.java index fe2cbdf5d..c86bc283c 100644 --- a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseException.java +++ b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseException.java @@ -16,6 +16,7 @@ public class ClickHouseException extends Exception { */ private static final long serialVersionUID = -2417038200885554382L; + public static final int ERROR_UNKNOWN_SETTING = 115; public static final int ERROR_ABORTED = 236; public static final int ERROR_CANCELLED = 394; public static final int ERROR_NETWORK = 210; diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java index b83c14af1..2297d8bdc 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java @@ -120,11 +120,17 @@ protected ClickHouseResponse send(ClickHouseRequest sealedRequest) throws Cli } : null; Map additionalParams = null; - if (sealedRequest.hasSetting("_set_roles_stmt")) { - additionalParams = Collections.singletonMap("_roles", sealedRequest.getSettings().get("_set_roles_stmt")); - } else if (!roles.isEmpty()) { - additionalParams = Collections.singletonMap("_roles", roles); + + if (config.getBoolOption(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES)) { + if (sealedRequest.hasSetting("_set_roles_stmt")) { + additionalParams = Collections.singletonMap("_roles", sealedRequest.getSettings().get("_set_roles_stmt")); + } else if (!roles.isEmpty()) { + additionalParams = Collections.singletonMap("_roles", roles); + } + } else { + additionalParams = Collections.emptyMap(); } + if (conn.isReusable()) { ClickHouseNode server = sealedRequest.getServer(); httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null), @@ -138,11 +144,14 @@ protected ClickHouseResponse send(ClickHouseRequest sealedRequest) throws Cli postAction); } - // At this point only successful responses are expected - if (sealedRequest.hasSetting("_set_roles_stmt")) { - rememberRoles((Set) sealedRequest.getSettings().get("_set_roles_stmt")); + if (config.getBoolOption(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES)) { + // At this point only successful responses are expected + if (sealedRequest.hasSetting("_set_roles_stmt")) { + rememberRoles((Set) sealedRequest.getSettings().get("_set_roles_stmt")); + } } + return ClickHouseStreamResponse.of(httpResponse.getConfig(sealedRequest), httpResponse.getInputStream(), sealedRequest.getSettings(), null, httpResponse.summary); } diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/config/ClickHouseHttpOption.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/config/ClickHouseHttpOption.java index 4630dbc64..af7736e81 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/config/ClickHouseHttpOption.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/config/ClickHouseHttpOption.java @@ -42,13 +42,20 @@ public enum ClickHouseHttpOption implements ClickHouseOption { * headers. */ RECEIVE_QUERY_PROGRESS("receive_query_progress", true, - "Whether to receive information about the progress of a query in response headers."); + "Whether to receive information about the progress of a query in response headers."), // SEND_PROGRESS("send_progress_in_http_headers", false, // "Enables or disables X-ClickHouse-Progress HTTP response headers in // clickhouse-server responses."), // SEND_PROGRESS_INTERVAL("http_headers_progress_interval_ms", 3000, ""), // WAIT_END_OF_QUERY("wait_end_of_query", false, ""), + /** + * Whether to remember last set role and send them in every next requests as query parameters. + * Only one role can be set at a time. + */ + REMEMBER_LAST_SET_ROLES("remember_last_set_roles", false, + "Whether to remember last set role and send them in every next requests as query parameters."); + private final String key; private final Serializable defaultValue; private final Class clazz; diff --git a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj index 223b8833e..73880cebf 100644 --- a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj +++ b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj @@ -132,7 +132,7 @@ public class ClickHouseSqlParser { } } -private void rememberRolesIfSetStmt() { + private void rememberRolesIfSetStmt() { if (anyArgsListStart > 0) { HashSet roles = new HashSet<>(); token_source.attachedAttributes.put("_ROLES", roles); @@ -146,12 +146,12 @@ private void rememberRolesIfSetStmt() { roles.add(roleBuff.toString()); roleBuff.setLength(0); isQuoted = false; - } else if (ch == '"' && !isQuoted) { + } else if (ch == '"') { isQuoted = true; - } else if (ch == ',' && !isQuoted) { + } else if (ch == ',' && !isQuoted && !roleBuff.isEmpty()) { roles.add(roleBuff.toString()); roleBuff.setLength(0); - } else { + } else if (!Character.isWhitespace(ch)) { roleBuff.append(ch); } } diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index 86358a998..20a7c415c 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -1,7 +1,12 @@ package com.clickhouse.jdbc; +import com.clickhouse.client.ClickHouseClient; +import com.clickhouse.client.ClickHouseException; import com.clickhouse.client.ClickHouseProtocol; +import com.clickhouse.client.http.config.ClickHouseHttpOption; +import com.clickhouse.data.ClickHouseVersion; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.sql.Connection; @@ -9,53 +14,56 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.Arrays; +import java.util.Properties; public class AccessManagementTest extends JdbcIntegrationTest { - @Test(groups = "integration") - public void testSetRoleDifferentConnections() throws SQLException { - /* - Tests: - * Simple expressions - * Composite expressions with multiple roles - * Composite expressions with mixed statements: - - update table1 SET a = 1; set role ROL1; update table2 SET b = 2; set role NONE; - */ + @Test(groups = "integration", dataProvider = "setRolesArgsForTestSetRole") + public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, String[] activeRoles) throws SQLException { String httpEndpoint = "http://" + getServerAddress(ClickHouseProtocol.HTTP) + "/"; String url = String.format("jdbc:ch:%s", httpEndpoint); - ClickHouseDataSource dataSource = new ClickHouseDataSource(url); - - + Properties properties = new Properties(); + properties.setProperty(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES.getKey(), "true"); + ClickHouseDataSource dataSource = new ClickHouseDataSource(url, properties); try (Connection connection = dataSource.getConnection("default", "")) { Statement st = connection.createStatement(); - -// st.execute("DROP ROLE IF EXISTS ROL1, \"role☺,\""); - st.execute("DROP ROLE IF EXISTS ROL1, \"ROL2,☺\""); st.execute("DROP USER IF EXISTS some_user"); - st.execute("create role ROL1; create role \"ROL2,☺\";"); - st.execute("create user some_user IDENTIFIED WITH no_password"); - st.execute("grant ROL1 to some_user"); - st.execute("grant \"ROL2,☺\" to some_user"); - + st.execute("DROP ROLE IF EXISTS " + String.join(", ", roles)); + st.execute("CREATE ROLE " + String.join(", ", roles)); + st.execute("CREATE USER some_user IDENTIFIED WITH no_password"); + st.execute("GRANT " + String.join(", ", roles) + " TO some_user"); } catch (Exception e) { Assert.fail("Failed", e); } try (Connection connection = dataSource.getConnection("some_user", "")) { - assertRolesEquals(connection, "ROL1", "ROL2,☺"); - Statement st = connection.createStatement(); - st.execute("set role \"ROL2,☺\""); -// st.execute("set\n role\n ROL1, \"ROL2,\""); - assertRolesEquals(connection, "ROL2,☺"); - + st.execute(setRoleExpr); + assertRolesEquals(connection, activeRoles); + } catch (SQLException e) { + if (e.getErrorCode() == ClickHouseException.ERROR_UNKNOWN_SETTING) { + String serverVersion = getServerVersion(dataSource.getConnection()); + if (ClickHouseVersion.of(serverVersion).check("[24.3,)")) { + return; + } + } + Assert.fail("Failed", e); } catch (Exception e) { Assert.fail("Failed", e); } } + @DataProvider(name = "setRolesArgsForTestSetRole") + private static Object[][] setRolesArgsForTestSetRole() { + return new Object[][]{ + {new String[]{"ROL1", "ROL2"}, "set role ROL2, ROL1", new String[]{"ROL2"}}, + {new String[]{"ROL1", "ROL2"}, "set role ROL1,ROL2", new String[]{"ROL1"}}, + {new String[]{"ROL1", "\"ROL2,☺\""}, "set role \"ROL2,☺\", ROL1", new String[]{"ROL2,☺"}}, + }; + } + private void assertRolesEquals(Connection connection, String ...expected) { try { Statement st = connection.createStatement(); @@ -65,11 +73,27 @@ private void assertRolesEquals(Connection connection, String ...expected) { String[] roles = (String[]) resultSet.getArray(1).getArray(); Arrays.sort(roles); Arrays.sort(expected); - System.out.println("currentRoles = " + Arrays.asList(roles)); + System.out.print("Roles: "); + for (String role : roles) { + System.out.print("'" + role+ "', "); + } + System.out.println(); Assert.assertEquals(roles, expected); } catch (Exception e) { Assert.fail("Failed", e); } } + + private String getServerVersion(Connection connection) { + try (Statement stmt = connection.createStatement()) { + ResultSet rs = stmt.executeQuery("SELECT version()"); + if (rs.next()) { + return rs.getString(1); + } + } catch (SQLException e) { + Assert.fail("Failed to get server version", e); + } + return null; + } } From b24547fa8c25c635621bf4f246968669a11175c6 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 26 Mar 2024 23:00:43 -0700 Subject: [PATCH 10/18] fixed tests --- .../clickhouse/client/http/ClickHouseHttpConnectionTest.java | 2 +- .../src/test/java/com/clickhouse/jdbc/AccessManagementTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java index a1be0dd1b..743628499 100644 --- a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java +++ b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java @@ -95,7 +95,7 @@ public void testRolesQParam(String role) { try { URIBuilder uriBuilder = new URIBuilder(new URI(url)); String queryRole = uriBuilder.getQueryParams().stream() - .filter(p -> "custom_role".equals(p.getName())) + .filter(p -> "role".equals(p.getName())) .findFirst() .map(p -> p.getValue()) .orElse(null); diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index 20a7c415c..c2a4d64a1 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -45,7 +45,7 @@ public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, } catch (SQLException e) { if (e.getErrorCode() == ClickHouseException.ERROR_UNKNOWN_SETTING) { String serverVersion = getServerVersion(dataSource.getConnection()); - if (ClickHouseVersion.of(serverVersion).check("[24.3,)")) { + if (ClickHouseVersion.of(serverVersion).check("(,24.2]")) { return; } } From 2bd31532ca2d4e553671e3c469d0dc32728807b7 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 26 Mar 2024 23:53:49 -0700 Subject: [PATCH 11/18] fixed tests --- clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj index 73880cebf..f1ce5e87f 100644 --- a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj +++ b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj @@ -148,7 +148,7 @@ public class ClickHouseSqlParser { isQuoted = false; } else if (ch == '"') { isQuoted = true; - } else if (ch == ',' && !isQuoted && !roleBuff.isEmpty()) { + } else if (ch == ',' && !isQuoted && roleBuff.length() > 0) { roles.add(roleBuff.toString()); roleBuff.setLength(0); } else if (!Character.isWhitespace(ch)) { From cd79cf14f6b7e59d4fbe5d47a3f4f1f29d0b22ad Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Wed, 27 Mar 2024 11:48:58 -0700 Subject: [PATCH 12/18] remove dropping user as temp fix --- .../test/java/com/clickhouse/jdbc/AccessManagementTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index c2a4d64a1..82b95c8f1 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -29,8 +29,7 @@ public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, try (Connection connection = dataSource.getConnection("default", "")) { Statement st = connection.createStatement(); - st.execute("DROP USER IF EXISTS some_user"); - st.execute("DROP ROLE IF EXISTS " + String.join(", ", roles)); + st.execute("CREATE ROLE " + String.join(", ", roles)); st.execute("CREATE USER some_user IDENTIFIED WITH no_password"); st.execute("GRANT " + String.join(", ", roles) + " TO some_user"); From bae46874cd893cdd4e0a2907da61041ad2830e5c Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Wed, 27 Mar 2024 13:20:38 -0700 Subject: [PATCH 13/18] fix test user permissions --- .../clickhouse-server/users.d/users.xml | 15 +++++++++++++++ .../com/clickhouse/jdbc/AccessManagementTest.java | 6 ++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml b/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml index cf57c6907..64aa4d25a 100644 --- a/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml +++ b/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml @@ -67,5 +67,20 @@ me + + default + + ::/0 + + 123 + default + + GRANT ROLE ADMIN ON *.* + GRANT CREATE ROLE ON *.* + GRANT CREATE USER ON *.* + GRANT DROP USER ON *.* + GRANT DROP ROLE ON *.* + + \ No newline at end of file diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index 82b95c8f1..2081448fe 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -27,9 +27,11 @@ public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, properties.setProperty(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES.getKey(), "true"); ClickHouseDataSource dataSource = new ClickHouseDataSource(url, properties); - try (Connection connection = dataSource.getConnection("default", "")) { + try (Connection connection = dataSource.getConnection("access_dba", "123")) { Statement st = connection.createStatement(); + st.execute("DROP ROLE IF EXISTS " + String.join(", ", roles)); + st.execute("DROP USER IF EXISTS some_user"); st.execute("CREATE ROLE " + String.join(", ", roles)); st.execute("CREATE USER some_user IDENTIFIED WITH no_password"); st.execute("GRANT " + String.join(", ", roles) + " TO some_user"); @@ -44,7 +46,7 @@ public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, } catch (SQLException e) { if (e.getErrorCode() == ClickHouseException.ERROR_UNKNOWN_SETTING) { String serverVersion = getServerVersion(dataSource.getConnection()); - if (ClickHouseVersion.of(serverVersion).check("(,24.2]")) { + if (ClickHouseVersion.of(serverVersion).check("(,24.3]")) { return; } } From 8eadd9760fa766a30f0da1bbb38cdc5e22089c31 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Wed, 27 Mar 2024 16:05:33 -0700 Subject: [PATCH 14/18] fix test user access --- .../containers/clickhouse-server/users.d/users.xml | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml b/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml index 64aa4d25a..66bd3f3e8 100644 --- a/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml +++ b/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml @@ -74,13 +74,8 @@ 123 default - - GRANT ROLE ADMIN ON *.* - GRANT CREATE ROLE ON *.* - GRANT CREATE USER ON *.* - GRANT DROP USER ON *.* - GRANT DROP ROLE ON *.* - + 1 + \ No newline at end of file From 39aebc5d0fc7cba7a97cc86a89da4bfe2d7b85e0 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 2 Apr 2024 23:56:10 -0700 Subject: [PATCH 15/18] Updated a change log --- CHANGELOG.md | 8 ++++++++ .../containers/clickhouse-server/users.d/users.xml | 1 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bdabcc71..497a11da3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## Latest + +### New Features +- [HTTP] Persistence of a role after it is set by `SET ROLE ` + +### Bug Fixes +- Fix param error in ByteUtils#equals in java9 [#1551](https://github.com/ClickHouse/clickhouse-java/issues/1551) + ## 0.6.0-patch3 ### Bug Fixes diff --git a/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml b/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml index 66bd3f3e8..efe565b20 100644 --- a/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml +++ b/clickhouse-client/src/test/resources/containers/clickhouse-server/users.d/users.xml @@ -75,7 +75,6 @@ 123 default 1 - \ No newline at end of file From 77b8024c3032470402b5207ae9d77ec7fa0388d2 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 4 Apr 2024 14:04:24 -0700 Subject: [PATCH 16/18] fix build --- .../client/http/ClickHouseHttpClient.java | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java index c633d36d7..874584b1e 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java @@ -1,5 +1,20 @@ package com.clickhouse.client.http; +import com.clickhouse.client.AbstractClient; +import com.clickhouse.client.ClickHouseConfig; +import com.clickhouse.client.ClickHouseException; +import com.clickhouse.client.ClickHouseNode; +import com.clickhouse.client.ClickHouseProtocol; +import com.clickhouse.client.ClickHouseRequest; +import com.clickhouse.client.ClickHouseResponse; +import com.clickhouse.client.ClickHouseStreamResponse; +import com.clickhouse.client.ClickHouseTransaction; +import com.clickhouse.client.http.config.ClickHouseHttpOption; +import com.clickhouse.config.ClickHouseOption; +import com.clickhouse.data.ClickHouseChecker; +import com.clickhouse.logging.Logger; +import com.clickhouse.logging.LoggerFactory; + import java.io.IOException; import java.io.Serializable; import java.io.UncheckedIOException; @@ -9,29 +24,13 @@ import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; +import java.util.Enumeration; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentSkipListSet; -import com.clickhouse.client.AbstractClient; -import com.clickhouse.client.ClickHouseConfig; -import com.clickhouse.client.ClickHouseException; -import com.clickhouse.client.ClickHouseNode; -import com.clickhouse.client.ClickHouseProtocol; -import com.clickhouse.client.ClickHouseRequest; -import com.clickhouse.client.ClickHouseResponse; -import com.clickhouse.client.ClickHouseTransaction; -import com.clickhouse.client.ClickHouseStreamResponse; -import com.clickhouse.client.config.ClickHouseClientOption; -import com.clickhouse.client.http.config.ClickHouseHttpOption; -import com.clickhouse.config.ClickHouseOption; -import com.clickhouse.data.ClickHouseChecker; -import com.clickhouse.logging.Logger; -import com.clickhouse.logging.LoggerFactory; - public class ClickHouseHttpClient extends AbstractClient { private static final Logger log = LoggerFactory.getLogger(ClickHouseHttpClient.class); From 6ce419c5879a9ca52524d7f8d71be4bb8d245c08 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 23 Apr 2024 20:15:24 -0700 Subject: [PATCH 17/18] fixed for HttpURLConnection. Fixed bugwith one role --- .../client/http/ApacheHttpConnectionImpl.java | 3 +- .../client/http/ClickHouseHttpClient.java | 28 +++++++++++-------- .../client/http/ClickHouseHttpConnection.java | 5 ++-- .../http/ClickHouseHttpConnectionFactory.java | 14 ++++++++-- .../client/http/HttpUrlConnectionImpl.java | 17 ++++++----- .../http/ClickHouseHttpConnectionTest.java | 22 ++++++--------- .../src/main/javacc/ClickHouseSqlParser.jj | 3 ++ .../clickhouse/jdbc/AccessManagementTest.java | 23 ++++++++++----- 8 files changed, 68 insertions(+), 47 deletions(-) diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java index 3d39f26b5..9289833a3 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java @@ -60,6 +60,7 @@ import java.net.Socket; import java.net.StandardSocketOptions; import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.TimeZone; @@ -76,7 +77,7 @@ public class ApacheHttpConnectionImpl extends ClickHouseHttpConnection { protected ApacheHttpConnectionImpl(ClickHouseNode server, ClickHouseRequest request, ExecutorService executor) throws IOException { - super(server, request); + super(server, request, Collections.emptyMap()); client = newConnection(config); } diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java index 874584b1e..b7170951e 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpClient.java @@ -114,7 +114,8 @@ protected ClickHouseHttpConnection newConnection(ClickHouseHttpConnection connec } try { - return ClickHouseHttpConnectionFactory.createConnection(server, request, getExecutor()); + + return ClickHouseHttpConnectionFactory.createConnection(server, request, getExecutor(), buildAdditionalReqParams(request)); } catch (IOException e) { throw new CompletionException(e); } @@ -146,6 +147,18 @@ protected String buildQueryParams(Map params) { return builder.toString(); } + private Map buildAdditionalReqParams(ClickHouseRequest sealedRequest) { + ClickHouseConfig config = sealedRequest.getConfig(); + if (config.getBoolOption(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES)) { + if (sealedRequest.hasSetting("_set_roles_stmt")) { + return Collections.singletonMap("_roles", sealedRequest.getSettings().get("_set_roles_stmt")); + } else if (!roles.isEmpty()) { + return Collections.singletonMap("_roles", roles); + } + } + return Collections.emptyMap(); + } + @Override protected ClickHouseResponse send(ClickHouseRequest sealedRequest) throws ClickHouseException, IOException { ClickHouseHttpConnection conn = getConnection(sealedRequest); @@ -174,19 +187,10 @@ protected ClickHouseResponse send(ClickHouseRequest sealedRequest) throws Cli } } : null; - Map additionalParams = null; - - if (config.getBoolOption(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES)) { - if (sealedRequest.hasSetting("_set_roles_stmt")) { - additionalParams = Collections.singletonMap("_roles", sealedRequest.getSettings().get("_set_roles_stmt")); - } else if (!roles.isEmpty()) { - additionalParams = Collections.singletonMap("_roles", roles); - } - } else { - additionalParams = Collections.emptyMap(); - } if (conn.isReusable()) { + Map additionalParams = buildAdditionalReqParams(sealedRequest); + ClickHouseNode server = sealedRequest.getServer(); httpResponse = conn.post(config, sql, sealedRequest.getInputStream().orElse(null), sealedRequest.getExternalTables(), sealedRequest.getOutputStream().orElse(null), diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java index 4f074769d..91192606c 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java @@ -374,7 +374,8 @@ protected static void postData(ClickHouseConfig config, byte[] boundary, String protected final Map defaultHeaders; protected final String url; - protected ClickHouseHttpConnection(ClickHouseNode server, ClickHouseRequest request) { + protected ClickHouseHttpConnection(ClickHouseNode server, ClickHouseRequest request, + Map additionalParams) { if (server == null || request == null) { throw new IllegalArgumentException("Non-null server and request are required"); } @@ -385,7 +386,7 @@ protected ClickHouseHttpConnection(ClickHouseNode server, ClickHouseRequest r ClickHouseConfig c = request.getConfig(); this.config = c; this.defaultHeaders = Collections.unmodifiableMap(createDefaultHeaders(c, server, getUserAgent(), ClickHouseHttpClient.getReferer(config))); - this.url = buildUrl(server.getBaseUri(), request, Collections.emptyMap()); + this.url = buildUrl(server.getBaseUri(), request, additionalParams); log.debug("url [%s]", this.url); } diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java index dc6247093..c5eb8852a 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java @@ -1,6 +1,9 @@ package com.clickhouse.client.http; import java.io.IOException; +import java.io.Serializable; +import java.util.Collections; +import java.util.Map; import java.util.concurrent.ExecutorService; import com.clickhouse.client.ClickHouseNode; @@ -14,7 +17,14 @@ public final class ClickHouseHttpConnectionFactory { private static final Logger log = LoggerFactory.getLogger(ClickHouseHttpConnectionFactory.class); public static ClickHouseHttpConnection createConnection(ClickHouseNode server, ClickHouseRequest request, - ExecutorService executor) throws IOException { + ExecutorService executor) throws IOException + { + return createConnection(server, request, executor, Collections.emptyMap()); + } + + public static ClickHouseHttpConnection createConnection(ClickHouseNode server, ClickHouseRequest request, + ExecutorService executor, Map additionalRequestParams) throws IOException { HttpConnectionProvider provider = request.getConfig().getOption(ClickHouseHttpOption.CONNECTION_PROVIDER, HttpConnectionProvider.class); if (provider == HttpConnectionProvider.APACHE_HTTP_CLIENT) { @@ -27,7 +37,7 @@ public static ClickHouseHttpConnection createConnection(ClickHouseNode server, C log.warn("HTTP_CLIENT is only supported in JDK 11 or above, fall back to HTTP_URL_CONNECTION"); } - return new HttpUrlConnectionImpl(server, request, executor); + return new HttpUrlConnectionImpl(server, request, executor, additionalRequestParams); } private ClickHouseHttpConnectionFactory() { diff --git a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/HttpUrlConnectionImpl.java b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/HttpUrlConnectionImpl.java index 0eff6ee3b..946f49d4b 100644 --- a/clickhouse-http-client/src/main/java/com/clickhouse/client/http/HttpUrlConnectionImpl.java +++ b/clickhouse-http-client/src/main/java/com/clickhouse/client/http/HttpUrlConnectionImpl.java @@ -17,6 +17,9 @@ import com.clickhouse.logging.Logger; import com.clickhouse.logging.LoggerFactory; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLContext; import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -24,8 +27,8 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.io.Serializable; import java.io.UncheckedIOException; - import java.net.ConnectException; import java.net.HttpURLConnection; import java.net.Proxy; @@ -35,15 +38,11 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.TimeZone; -import java.util.Map.Entry; import java.util.concurrent.ExecutorService; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLContext; - public class HttpUrlConnectionImpl extends ClickHouseHttpConnection { private static final Logger log = LoggerFactory.getLogger(HttpUrlConnectionImpl.class); @@ -205,9 +204,9 @@ private void checkResponse(HttpURLConnection conn) throws IOException { } } - protected HttpUrlConnectionImpl(ClickHouseNode server, ClickHouseRequest request, ExecutorService executor) - throws IOException { - super(server, request); + protected HttpUrlConnectionImpl(ClickHouseNode server, ClickHouseRequest request, ExecutorService executor, + Map additionalParams) throws IOException { + super(server, request, additionalParams); conn = newConnection(url, true); } diff --git a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java index 75525b0bf..05413ac44 100644 --- a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java +++ b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpConnectionTest.java @@ -1,16 +1,5 @@ package com.clickhouse.client.http; -import java.io.IOException; -import java.io.Serializable; -import java.net.URI; -import java.net.URL; -import java.net.URLDecoder; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; - import com.clickhouse.client.ClickHouseClient; import com.clickhouse.client.ClickHouseConfig; import com.clickhouse.client.ClickHouseNode; @@ -21,18 +10,23 @@ import com.clickhouse.data.ClickHouseFormat; import com.clickhouse.data.ClickHouseInputStream; import com.clickhouse.data.ClickHouseOutputStream; - import org.apache.hc.core5.net.URIBuilder; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import org.testng.collections.Sets; -import org.testng.internal.invokers.Arguments; + +import java.io.IOException; +import java.io.Serializable; +import java.net.URI; +import java.util.Collections; +import java.util.List; +import java.util.Map; public class ClickHouseHttpConnectionTest { static class SimpleHttpConnection extends ClickHouseHttpConnection { protected SimpleHttpConnection(ClickHouseNode server, ClickHouseRequest request) { - super(server, request); + super(server, request, Collections.emptyMap()); } @Override diff --git a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj index f1ce5e87f..0ad0e83c6 100644 --- a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj +++ b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj @@ -155,6 +155,9 @@ public class ClickHouseSqlParser { roleBuff.append(ch); } } + if (roleBuff.length() > 0) { + roles.add(roleBuff.toString()); + } } anyArgsListStart = -1; } diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index 2081448fe..f202e831c 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -1,9 +1,9 @@ package com.clickhouse.jdbc; -import com.clickhouse.client.ClickHouseClient; import com.clickhouse.client.ClickHouseException; import com.clickhouse.client.ClickHouseProtocol; import com.clickhouse.client.http.config.ClickHouseHttpOption; +import com.clickhouse.client.http.config.HttpConnectionProvider; import com.clickhouse.data.ClickHouseVersion; import org.testng.Assert; import org.testng.annotations.DataProvider; @@ -19,12 +19,14 @@ public class AccessManagementTest extends JdbcIntegrationTest { @Test(groups = "integration", dataProvider = "setRolesArgsForTestSetRole") - public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, String[] activeRoles) throws SQLException { + public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, String[] activeRoles, + String connectionProvider) throws SQLException { String httpEndpoint = "http://" + getServerAddress(ClickHouseProtocol.HTTP) + "/"; String url = String.format("jdbc:ch:%s", httpEndpoint); Properties properties = new Properties(); properties.setProperty(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES.getKey(), "true"); + properties.setProperty(ClickHouseHttpOption.CONNECTION_PROVIDER.getKey(), connectionProvider); ClickHouseDataSource dataSource = new ClickHouseDataSource(url, properties); try (Connection connection = dataSource.getConnection("access_dba", "123")) { @@ -59,13 +61,20 @@ public void testSetRoleDifferentConnections(String[] roles, String setRoleExpr, @DataProvider(name = "setRolesArgsForTestSetRole") private static Object[][] setRolesArgsForTestSetRole() { return new Object[][]{ - {new String[]{"ROL1", "ROL2"}, "set role ROL2, ROL1", new String[]{"ROL2"}}, - {new String[]{"ROL1", "ROL2"}, "set role ROL1,ROL2", new String[]{"ROL1"}}, - {new String[]{"ROL1", "\"ROL2,☺\""}, "set role \"ROL2,☺\", ROL1", new String[]{"ROL2,☺"}}, + {new String[]{"ROL1", "ROL2"}, "set role ROL2", new String[]{"ROL2"}, + HttpConnectionProvider.HTTP_URL_CONNECTION.name()}, + {new String[]{"ROL1", "ROL2"}, "set role ROL2", new String[]{"ROL2"}, + HttpConnectionProvider.APACHE_HTTP_CLIENT.name()}, + {new String[]{"ROL1", "ROL2"}, "set role ROL2, ROL1", new String[]{"ROL1", "ROL2"}, + HttpConnectionProvider.APACHE_HTTP_CLIENT.name()}, + {new String[]{"ROL1", "\"ROL2,☺\""}, "set role \"ROL2,☺\", ROL1", new String[]{"ROL2,☺", "ROL1"}, + HttpConnectionProvider.APACHE_HTTP_CLIENT.name()}, + {new String[]{"ROL1", "ROL2"}, "set role ROL2 , ROL1, ", new String[]{"ROL", "ROL1"}, + HttpConnectionProvider.APACHE_HTTP_CLIENT.name()}, }; } - private void assertRolesEquals(Connection connection, String ...expected) { + private void assertRolesEquals(Connection connection, String... expected) { try { Statement st = connection.createStatement(); ResultSet resultSet = st.executeQuery("select currentRoles()"); @@ -76,7 +85,7 @@ private void assertRolesEquals(Connection connection, String ...expected) { Arrays.sort(expected); System.out.print("Roles: "); for (String role : roles) { - System.out.print("'" + role+ "', "); + System.out.print("'" + role + "', "); } System.out.println(); Assert.assertEquals(roles, expected); From 5d48f520f95ddf3b721995af5d67de665d46793a Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Tue, 23 Apr 2024 22:06:33 -0700 Subject: [PATCH 18/18] update java 11 src --- .../http/ClickHouseHttpConnectionFactory.java | 16 +++++++++++++--- .../client/http/HttpClientConnectionImpl.java | 8 +++++--- .../src/main/javacc/ClickHouseSqlParser.jj | 2 -- .../clickhouse/jdbc/AccessManagementTest.java | 2 +- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java b/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java index 3e49d4154..42cb65180 100644 --- a/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java +++ b/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/ClickHouseHttpConnectionFactory.java @@ -2,6 +2,10 @@ import java.io.IOException; import java.util.concurrent.ExecutorService; +import java.util.Collections; +import java.io.Serializable; +import java.util.Map; + import com.clickhouse.client.ClickHouseNode; import com.clickhouse.client.ClickHouseRequest; @@ -14,7 +18,13 @@ public final class ClickHouseHttpConnectionFactory { private static final Logger log = LoggerFactory.getLogger(ClickHouseHttpConnectionFactory.class); public static ClickHouseHttpConnection createConnection(ClickHouseNode server, ClickHouseRequest request, - ExecutorService executor) throws IOException { + ExecutorService executor) throws IOException + { + return createConnection(server, request, executor, Collections.emptyMap()); + } + + public static ClickHouseHttpConnection createConnection(ClickHouseNode server, ClickHouseRequest request, + ExecutorService executor, Map additionalRequestParams) throws IOException { HttpConnectionProvider provider = request.getConfig().getOption(ClickHouseHttpOption.CONNECTION_PROVIDER, HttpConnectionProvider.class); if (provider == HttpConnectionProvider.APACHE_HTTP_CLIENT) { @@ -24,10 +34,10 @@ public static ClickHouseHttpConnection createConnection(ClickHouseNode server, C log.warn("Error when creating %s, fall back to HTTP_URL_CONNECTION", provider, t); } } else if (provider == HttpConnectionProvider.HTTP_CLIENT) { - return new HttpClientConnectionImpl(server, request, executor); + return new HttpClientConnectionImpl(server, request, executor, additionalRequestParams); } - return new HttpUrlConnectionImpl(server, request, executor); + return new HttpUrlConnectionImpl(server, request, executor, additionalRequestParams); } private ClickHouseHttpConnectionFactory() { diff --git a/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/HttpClientConnectionImpl.java b/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/HttpClientConnectionImpl.java index a3006ba8d..25c088fea 100644 --- a/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/HttpClientConnectionImpl.java +++ b/clickhouse-http-client/src/main/java11/com/clickhouse/client/http/HttpClientConnectionImpl.java @@ -52,6 +52,8 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.atomic.AtomicBoolean; +import java.io.Serializable; +import java.util.Map; import javax.net.ssl.SSLContext; @@ -173,9 +175,9 @@ private HttpRequest newRequest(String url) { .timeout(Duration.ofMillis(config.getSocketTimeout())).build(); } - protected HttpClientConnectionImpl(ClickHouseNode server, ClickHouseRequest request, ExecutorService executor) - throws IOException { - super(server, request); + protected HttpClientConnectionImpl(ClickHouseNode server, ClickHouseRequest request, ExecutorService executor, + Map additionalParams) throws IOException { + super(server, request, additionalParams); HttpClient.Builder builder = HttpClient.newBuilder().version(Version.HTTP_1_1) .connectTimeout(Duration.ofMillis(config.getConnectionTimeout())).followRedirects(Redirect.NORMAL); diff --git a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj index 0ad0e83c6..e3eadced5 100644 --- a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj +++ b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj @@ -143,8 +143,6 @@ public class ClickHouseSqlParser { for (int i = anyArgsListStart; i < stmtLength; i++) { char ch = token_source.builder.charAt(i); if (ch == '"' && isQuoted ) { - roles.add(roleBuff.toString()); - roleBuff.setLength(0); isQuoted = false; } else if (ch == '"') { isQuoted = true; diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java index f202e831c..1946dddee 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/AccessManagementTest.java @@ -69,7 +69,7 @@ private static Object[][] setRolesArgsForTestSetRole() { HttpConnectionProvider.APACHE_HTTP_CLIENT.name()}, {new String[]{"ROL1", "\"ROL2,☺\""}, "set role \"ROL2,☺\", ROL1", new String[]{"ROL2,☺", "ROL1"}, HttpConnectionProvider.APACHE_HTTP_CLIENT.name()}, - {new String[]{"ROL1", "ROL2"}, "set role ROL2 , ROL1, ", new String[]{"ROL", "ROL1"}, + {new String[]{"ROL1", "ROL2"}, "set role ROL2 , ROL1 ", new String[]{"ROL2", "ROL1"}, HttpConnectionProvider.APACHE_HTTP_CLIENT.name()}, }; }