[jdbc] Adds CREATE USER statement to parser grammar
#2421
Conversation
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
| | CREATE USER ((IF NOT EXISTS) | (OR REPLACE))? userIdentifier (COMMA userIdentifier)* clusterClause? | ||
| userIdentifiedClause? | ||
| userCreateHostClause? | ||
| validUntilClause? | ||
| (DEFAULT ROLE identifier (COMMA identifier)*)? | ||
| (DEFAULT DATABASE identifier | NONE)? | ||
|
|
||
| settingsClause? #CreateUserStmt |
There was a problem hiding this comment.
Tests are required for the new CREATE USER statement grammar. Please add unit and/or integration tests to ensure the parser correctly handles valid and invalid CREATE USER statements.
|
@chernser do we need to contribute the changes to the clickhouse upstream? |
| } | ||
|
|
||
| @Test(groups = {"integration"}) | ||
| public void testDDLStatements() throws Exception { |
There was a problem hiding this comment.
I think we can expand testing with a few more examples (to see that the parser works correctly)
Here we can find the https://clickhouse.com/docs/sql-reference/statements/create/user
like adding
- "CREATE USER IF NOT EXISTS 'user011' IDENTIFIED WITH no_password"
- "IDENTIFIED WITH plaintext_password BY 'qwerty'"
and a few more options
|
/windsurf-review |
| try { | ||
| lastStatementSql = parseJdbcEscapeSyntax(sql); | ||
| LOG.debug("SQL Query: {}", lastStatementSql); | ||
| LOG.trace("SQL Query: {}", lastStatementSql); // this is not secure for create statements because of passwords |
There was a problem hiding this comment.
Logging full SQL queries, even at TRACE level, may expose sensitive information such as passwords in CREATE USER statements. Consider redacting sensitive data from the SQL before logging, or ensure users are aware of this risk in documentation or configuration.
| @Test(groups = { "integration" }) | ||
| void testRecursiveWithClause() throws Exception { | ||
| if (ClickHouseVersion.of(getServerVersion()).check("(,24.3]")) { | ||
| return; // recursive CTEs were introduces in 24.4 |
There was a problem hiding this comment.
Typo in comment: 'introduces' should be 'introduced'.
|
|
||
| public class SqlParser { | ||
|
|
||
| private static Logger LOG = LoggerFactory.getLogger(SqlParser.class); |
There was a problem hiding this comment.
Consider declaring the logger as 'private static final Logger LOG = LoggerFactory.getLogger(SqlParser.class);' to follow best practices for static loggers.
|
Summary
Adds
CREATE USERstatement grammar.Closes #2398
Closes #2428
Checklist
Delete items not relevant to your PR: