diff --git a/jdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4/ClickHouseParser.g4 b/jdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4/ClickHouseParser.g4 index 2a422e3b9..cda734421 100644 --- a/jdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4/ClickHouseParser.g4 +++ b/jdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4/ClickHouseParser.g4 @@ -668,7 +668,7 @@ setStmt // SET ROLE statement setRoleStmt - : SET (DEFAULT)? ROLE (setRolesList | NONE | ALL (EXCEPT setRolesList)) (TO identifier | CURRENT_USER (COMMA identifier | CURRENT_USER)*)? + : SET (DEFAULT)? ROLE ( NONE | setRolesList | ALL (EXCEPT setRolesList)) (TO identifier | CURRENT_USER (COMMA identifier | CURRENT_USER)*)? ; setRolesList diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java index b3238e872..b8354f7ec 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java @@ -119,7 +119,7 @@ public ConnectionImpl(String url, Properties info) throws SQLException { this.sqlParser = SqlParserFacade.getParser(config.getDriverProperty(DriverProperties.SQL_PARSER.getKey(), - DriverProperties.SQL_PARSER.getDefaultValue())); + DriverProperties.SQL_PARSER.getDefaultValue()), config); this.featureManager = new FeatureManager(this.config); } catch (SQLException e) { throw e; diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java index 872ec4608..7027c95c1 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java @@ -78,6 +78,52 @@ public enum DriverProperties { */ QUERY_ID_GENERATOR("jdbc_query_id_generator", null), + /** + * Controls logic of saving roles that were set using {@code SET } statement. + * Default: true - save roles + */ + REMEMBER_LAST_SET_ROLES("remember_last_set_roles", String.valueOf(Boolean.TRUE)), + + /** + * Deprecated and will be removed. + * This property is here to keep backward compatibility with {@code com.clickhouse.client.http.config.ClickHouseHttpOption#CUSTOM_PARAMS}. + * Original property is deprecated for {@code com.clickhouse.client.config.ClickHouseClientOption#CUSTOM_SETTINGS} + * This property is expected to be a comma separated list of key-value pair that should be sent to server. + * Pairs will be converted to new properties for client config. + * Use {@link ClientConfigProperties#serverSetting(String)} instead + * + */ + @Deprecated + CUSTOM_HTTP_PARAMS("custom_http_params", null), + + + /** + * Deprecated and will be removed. + * This property is here to keep backward compatibility with {@code com.clickhouse.client.config.ClickHouseClientOption#CUSTOM_SETTINGS}. + * This property is expected to be a comma separated list of key-value pair that should be sent to server. + * Pairs will be converted to new properties for client config. + * Use {@link ClientConfigProperties#serverSetting(String)} instead + * + */ + @Deprecated + CUSTOM_SETTINGS("custom_settings", null), + + /** + * Deprecated as driver do not convert Date values to any timezone. Dates are sent as is. + * Problem having this setting on connection level is that affects all child statements and + * there is no way to control on column basis. + * Driver ignores this setting but will throw exception when it is completely removed. + */ + @Deprecated + USE_TZ_FOR_DATES("use_server_time_zone_for_dates", null), + + /** + * Current driver implementation uses only one http provider. So setting it has no effect. + * Deprecated will be removed soon + */ + @Deprecated + HTTP_CONNECTION_PROVIDER("http_connection_provider", null), + ; diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java index a28b15234..a814718ad 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java @@ -235,7 +235,7 @@ protected long executeUpdateImpl(String sql, QuerySettings settings) throws SQLE return updateCount; } - protected void postUpdateActions() throws SQLException { + private void postUpdateActions() throws SQLException { if (parsedStatement.getUseDatabase() != null) { this.localSettings.setDatabase(parsedStatement.getUseDatabase()); } diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java index 21d05a552..9aa5ce61a 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java @@ -299,6 +299,11 @@ private void initProperties(Map urlProperties, Properties provid propertyInfos.put(prop.getKey(), propertyInfo); if (DRIVER_PROP_KEYS.contains(prop.getKey())) { + if (prop.getKey().equalsIgnoreCase(DriverProperties.CUSTOM_HTTP_PARAMS.getKey()) || + prop.getKey().equalsIgnoreCase(DriverProperties.CUSTOM_SETTINGS.getKey())) { + ClientConfigProperties.toKeyValuePairs(prop.getValue()) + .forEach((k, v) -> clientProperties.put(ClientConfigProperties.serverSetting(k), v)); + } driverProperties.put(prop.getKey(), prop.getValue()); } else { clientProperties.put(prop.getKey(), prop.getValue()); diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java index afb5dbf39..f819fea49 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java @@ -2,6 +2,7 @@ import com.clickhouse.client.api.sql.SQLUtils; import com.clickhouse.data.ClickHouseUtils; +import com.clickhouse.jdbc.DriverProperties; import com.clickhouse.jdbc.internal.parser.antlr4.ClickHouseLexer; import com.clickhouse.jdbc.internal.parser.antlr4.ClickHouseParser; import com.clickhouse.jdbc.internal.parser.antlr4.ClickHouseParserBaseListener; @@ -25,6 +26,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; public abstract class SqlParserFacade { @@ -37,6 +39,12 @@ public abstract class SqlParserFacade { private static class JavaCCParser extends SqlParserFacade { + private final boolean processUseRolesExpr; + + public JavaCCParser(boolean saveRoles) { + this.processUseRolesExpr = saveRoles; + } + @Override public ParsedStatement parsedStatement(String sql) { ParsedStatement stmt = new ParsedStatement(); @@ -45,22 +53,11 @@ public ParsedStatement parsedStatement(String sql) { stmt.setUseDatabase(parsedStmt.getDatabase()); } - String rolesCount = parsedStmt.getSettings().get("_ROLES_COUNT"); - if (rolesCount != null) { - int rolesCountInt = Integer.parseInt(rolesCount); - ArrayList roles = new ArrayList<>(rolesCountInt); - boolean resetRoles = false; - for (int i = 0; i < rolesCountInt; i++) { - String role = parsedStmt.getSettings().get("_ROLE_" + i); - if (role.equalsIgnoreCase("NONE")) { - resetRoles = true; - } - roles.add(parsedStmt.getSettings().get("_ROLE_" + i)); - } - if (resetRoles) { - roles.clear(); + if (processUseRolesExpr) { + List roles = processRoles(parsedStmt.getSettings()); + if (roles != null) { + stmt.setRoles(roles); } - stmt.setRoles(roles); } stmt.setInsert(parsedStmt.getStatementType() == StatementType.INSERT); @@ -109,11 +106,39 @@ public ParsedPreparedStatement parsePreparedStatement(String sql) { } } + if (processUseRolesExpr) { + List roles = processRoles(parsedStmt.getSettings()); + if (roles != null) { + stmt.setRoles(roles); + } + } + stmt.setUseFunction(parsedStmt.isFuncUsed()); parseParameters(sql, stmt); return stmt; } + private List processRoles(Map settings) { + String rolesCount = settings.get("_ROLES_COUNT"); + if (rolesCount != null) { + int rolesCountInt = Integer.parseInt(rolesCount); + ArrayList roles = new ArrayList<>(rolesCountInt); + boolean resetRoles = false; + for (int i = 0; i < rolesCountInt; i++) { + String role = settings.get("_ROLE_" + i); + if (role.equalsIgnoreCase("NONE")) { + resetRoles = true; + } + roles.add(settings.get("_ROLE_" + i)); + } + if (resetRoles) { + roles.clear(); + } + return roles; + } + return null; // no roles present + } + public ClickHouseSqlStatement parse(String sql) { JdbcParseHandler handler = JdbcParseHandler.getInstance(); @@ -127,17 +152,23 @@ public ClickHouseSqlStatement parse(String sql) { private static class ANTLR4Parser extends SqlParserFacade { + protected final boolean processUseRolesExpr; + + public ANTLR4Parser(boolean saveRoles) { + this.processUseRolesExpr = saveRoles; + } + @Override public ParsedStatement parsedStatement(String sql) { ParsedStatement stmt = new ParsedStatement(); - parseSQL(sql, new ParsedStatementListener(stmt)); + parseSQL(sql, new ParsedStatementListener(stmt, processUseRolesExpr)); return stmt; } @Override public ParsedPreparedStatement parsePreparedStatement(String sql) { ParsedPreparedStatement stmt = new ParsedPreparedStatement(); - parseSQL(sql, new ParsedPreparedStatementListener(stmt)); + parseSQL(sql, new ParsedPreparedStatementListener(stmt, processUseRolesExpr)); // Combine database and table like JavaCC does String tableName = stmt.getTable(); @@ -177,12 +208,27 @@ static boolean isStmtWithResultSet(ClickHouseParser.QueryStmtContext stmtContext qCtx.existsStmt() != null || qCtx.checkStmt() != null); } + static List processRolesExpr(ClickHouseParser.SetRoleStmtContext ctx) { + if (ctx.NONE() != null) { + return Collections.emptyList(); + } else { + List roles = new ArrayList<>(); + for (ClickHouseParser.IdentifierContext id : ctx.setRolesList().identifier()) { + roles.add(SQLUtils.unquoteIdentifier(id.getText())); + } + return roles; + } + } + private static class ParsedStatementListener extends ClickHouseParserBaseListener { private final ParsedStatement parsedStatement; - public ParsedStatementListener(ParsedStatement parsedStatement) { + private final boolean processSetRolesExpr; + + public ParsedStatementListener(ParsedStatement parsedStatement, boolean processSetRolesExpr) { this.parsedStatement = parsedStatement; + this.processSetRolesExpr = processSetRolesExpr; } @Override @@ -206,14 +252,8 @@ public void enterUseStmt(ClickHouseParser.UseStmtContext ctx) { @Override public void enterSetRoleStmt(ClickHouseParser.SetRoleStmtContext ctx) { - if (ctx.NONE() != null) { - parsedStatement.setRoles(Collections.emptyList()); - } else { - List roles = new ArrayList<>(); - for (ClickHouseParser.IdentifierContext id : ctx.setRolesList().identifier()) { - roles.add(SQLUtils.unquoteIdentifier(id.getText())); - } - parsedStatement.setRoles(roles); + if (processSetRolesExpr) { + parsedStatement.setRoles(processRolesExpr(ctx)); } } } @@ -222,8 +262,11 @@ protected static class ParsedPreparedStatementListener extends ClickHouseParserB protected final ParsedPreparedStatement parsedStatement; - public ParsedPreparedStatementListener(ParsedPreparedStatement parsedStatement) { + private final boolean processSetRolesExpr; + + public ParsedPreparedStatementListener(ParsedPreparedStatement parsedStatement, boolean processSetRolesExpr) { this.parsedStatement = parsedStatement; + this.processSetRolesExpr = processSetRolesExpr; } @Override @@ -242,14 +285,8 @@ public void enterUseStmt(ClickHouseParser.UseStmtContext ctx) { @Override public void enterSetRoleStmt(ClickHouseParser.SetRoleStmtContext ctx) { - if (ctx.NONE() != null) { - parsedStatement.setRoles(Collections.emptyList()); - } else { - List roles = new ArrayList<>(); - for (ClickHouseParser.IdentifierContext id : ctx.setRolesList().identifier()) { - roles.add(SQLUtils.unquoteIdentifier(id.getText())); - } - parsedStatement.setRoles(roles); + if (processSetRolesExpr) { + parsedStatement.setRoles(processRolesExpr(ctx)); } } @@ -349,10 +386,15 @@ public void exitInsertParameterFuncExpr(ClickHouseParser.InsertParameterFuncExpr private static class ANTLR4AndParamsParser extends ANTLR4Parser { + + public ANTLR4AndParamsParser(boolean saveRoles) { + super(saveRoles); + } + @Override public ParsedPreparedStatement parsePreparedStatement(String sql) { ParsedPreparedStatement stmt = new ParsedPreparedStatement(); - parseSQL(sql, new ParseStatementAndParamsListener(stmt)); + parseSQL(sql, new ParseStatementAndParamsListener(stmt, processUseRolesExpr)); // Combine database and table like JavaCC does String tableName = stmt.getTable(); @@ -366,8 +408,8 @@ public ParsedPreparedStatement parsePreparedStatement(String sql) { private static class ParseStatementAndParamsListener extends ParsedPreparedStatementListener { - public ParseStatementAndParamsListener(ParsedPreparedStatement parsedStatement) { - super(parsedStatement); + public ParseStatementAndParamsListener(ParsedPreparedStatement parsedStatement, boolean processSetRolesExpr) { + super(parsedStatement, processSetRolesExpr); } @Override @@ -449,16 +491,18 @@ public enum SQLParser { ANTLR4 } - public static SqlParserFacade getParser(String name) throws SQLException { + public static SqlParserFacade getParser(String name, JdbcConfiguration jdbcConfiguration) throws SQLException { try { + boolean saveRoles = Boolean.parseBoolean(jdbcConfiguration.getDriverProperty(DriverProperties.REMEMBER_LAST_SET_ROLES.getKey(), + DriverProperties.REMEMBER_LAST_SET_ROLES.getDefaultValue())); SQLParser parserSelection = SQLParser.valueOf(name); switch (parserSelection) { case JAVACC: - return new JavaCCParser(); + return new JavaCCParser(saveRoles); case ANTLR4_PARAMS_PARSER: - return new ANTLR4AndParamsParser(); + return new ANTLR4AndParamsParser(saveRoles); case ANTLR4: - return new ANTLR4Parser(); + return new ANTLR4Parser(saveRoles); } throw new SQLException("Unsupported parser: " + parserSelection); } catch (IllegalArgumentException e) { diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java index 799a2016c..258d989e8 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java @@ -1113,4 +1113,34 @@ public void testCustomParameters() throws Exception { } } } + + @Test(groups = {"integration"}) + public void testOldCustomSettingsParameter() throws Exception { + if (isCloud()) { + return; // no custom settings on cloud instance + } + + String sql = "SELECT " + + " getSetting('max_threads') AS max_threads, " + + " getSetting('session_timezone') AS tz "; + Properties properties = new Properties(); + properties.put(DriverProperties.CUSTOM_SETTINGS.getKey(), "max_threads=10,session_timezone=Asia/Tokyo"); + try (Connection connection = getJdbcConnection(properties)) { + try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery(sql)){ + assertTrue(rs.next()); + assertEquals(rs.getString(1), "10"); + assertEquals(rs.getString(2), "Asia/Tokyo"); + } + } + + properties = new Properties(); + properties.put(DriverProperties.CUSTOM_HTTP_PARAMS.getKey(), "max_threads=10,session_timezone=Asia/Tokyo"); + try (Connection connection = getJdbcConnection(properties)) { + try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery(sql)){ + assertTrue(rs.next()); + assertEquals(rs.getString(1), "10"); + assertEquals(rs.getString(2), "Asia/Tokyo"); + } + } + } } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java index edb2559d3..fd0bd2f4a 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java @@ -4,6 +4,7 @@ import com.clickhouse.client.api.internal.ServerSettings; import com.clickhouse.client.api.query.GenericRecord; import com.clickhouse.data.ClickHouseVersion; +import com.clickhouse.jdbc.internal.SqlParserFacade; import org.apache.commons.lang3.RandomStringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -397,17 +398,27 @@ public void testExecuteQueryTimeout() throws Exception { } } + @DataProvider(name = "testSettingRolesDP") + public static Object[][] testSettingRolesDP() { + return new Object[][] { + {SqlParserFacade.SQLParser.JAVACC}, + {SqlParserFacade.SQLParser.ANTLR4_PARAMS_PARSER}, + {SqlParserFacade.SQLParser.ANTLR4}, + }; + } - @Test(groups = {"integration"}) - public void testSettingRole() throws SQLException { + @Test(groups = {"integration"}, dataProvider = "testSettingRolesDP", dataProviderClass = StatementTest.class) + public void testSettingRole(SqlParserFacade.SQLParser parser) throws SQLException { if (earlierThan(24, 4)) {//Min version is 24.4 return; } List roles = Arrays.asList("role1", "role2", "role3"); - String userPass = "^1A" + RandomStringUtils.random(12, true, true) + "3B$"; - try (ConnectionImpl conn = (ConnectionImpl) getJdbcConnection()) { + final String userPass = "^1A" + RandomStringUtils.random(12, true, true) + "3B$"; + Properties properties = new Properties(); + properties.setProperty(DriverProperties.SQL_PARSER.getKey(), parser.name()); + try (ConnectionImpl conn = (ConnectionImpl) getJdbcConnection(properties)) { try (Statement stmt = conn.createStatement()) { stmt.execute("DROP ROLE IF EXISTS " + String.join(", ", roles)); stmt.execute("DROP USER IF EXISTS some_user"); @@ -421,7 +432,7 @@ public void testSettingRole() throws SQLException { Properties info = new Properties(); info.setProperty("user", "some_user"); info.setProperty("password", userPass); - + info.setProperty(DriverProperties.SQL_PARSER.getKey(), parser.name()); try (ConnectionImpl conn = new ConnectionImpl(getEndpointString(), info)) { GenericRecord dataRecord = conn.getClient().queryAll("SELECT currentRoles()").get(0); assertEquals(dataRecord.getList(1).size(), 0); @@ -468,6 +479,53 @@ public void testSettingRole() throws SQLException { assertEquals(dataRecord.getList(1).get(1), "role2"); assertEquals(dataRecord.getList(1).get(2), "role3"); } + + + Properties disableSavingRoles = new Properties(); + disableSavingRoles.setProperty("user", "some_user"); + disableSavingRoles.setProperty("password", userPass); + disableSavingRoles.setProperty(DriverProperties.REMEMBER_LAST_SET_ROLES.getKey(), "false"); + disableSavingRoles.setProperty(DriverProperties.SQL_PARSER.getKey(), parser.name()); + try (ConnectionImpl conn = new ConnectionImpl(getEndpointString(), disableSavingRoles)) { + GenericRecord dataRecord = conn.getClient().queryAll("SELECT currentRoles()").get(0); + assertEquals(dataRecord.getList(1).size(), 0); + + try (Statement stmt = conn.createStatement()) { + stmt.execute("SET ROLE role1"); + } + + dataRecord = conn.getClient().queryAll("SELECT currentRoles()").get(0); + assertEquals(dataRecord.getList(1).size(), 0); + + try (Statement stmt = conn.createStatement()) { + stmt.execute("SET ROLE role2"); + } + + dataRecord = conn.getClient().queryAll("SELECT currentRoles()").get(0); + assertEquals(dataRecord.getList(1).size(), 0); + + try (Statement stmt = conn.createStatement()) { + stmt.execute("SET ROLE NONE"); + } + + dataRecord = conn.getClient().queryAll("SELECT currentRoles()").get(0); + assertEquals(dataRecord.getList(1).size(), 0); + + + try (Statement stmt = conn.createStatement()) { + stmt.execute("SET ROLE \"role1\",\"role2\""); + } + + dataRecord = conn.getClient().queryAll("SELECT currentRoles()").get(0); + assertEquals(dataRecord.getList(1).size(), 0); + + try (Statement stmt = conn.createStatement()) { + stmt.execute("SET ROLE \"role1\",\"role2\",\"role3\""); + } + + dataRecord = conn.getClient().queryAll("SELECT currentRoles()").get(0); + assertEquals(dataRecord.getList(1).size(), 0); + } } @Test diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java index 58cfe2327..cdb26a519 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java @@ -12,6 +12,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Properties; import java.util.stream.Collectors; import static org.testng.Assert.assertEquals; @@ -23,7 +24,7 @@ public abstract class BaseSqlParserFacadeTest { private SqlParserFacade parser; public BaseSqlParserFacadeTest(String name) throws Exception { - parser = SqlParserFacade.getParser(name); + parser = SqlParserFacade.getParser(name, new JdbcConfiguration("jdbc:ch:http://localhost:8123", new Properties())); } @Test