[jdbc-v2] Antlr4 parser #2351
[jdbc-v2] Antlr4 parser #2351
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the custom SQL parsing logic with an Antlr4-based parser and refactors how SQL statements (including prepared statements and batch operations) are built and executed.
- Removes the legacy StatementParser in favor of a new SqlParser based on Antlr4.
- Refactors both regular statements and prepared statements (including batch insert logic) to use the new parser and updated string substitution methods.
- Updates various supporting utilities and adapts test classes to the new parsing and statement-building mechanisms.
Reviewed Changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParser.java | Introduces the new Antlr4-based parser implementation. |
| jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/ParsedPreparedStatement.java | Implements a listener that collects parsed details for prepared statements. |
| jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java | Refactors SQL building and batch insert logic to use the new parser information and string replacement logic. |
| jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java | Updates statement execution to use the new "lastStatementSql" variable and revised query settings. |
| jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/StatementParser.java | Removed legacy SQL parsing logic. |
| jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java | Integrates the new SqlParser and adjusts schema/table handling for beta features. |
| Other test and utility files | Minor cleanup and adaptation to the new parsing architecture. |
Files not reviewed (2)
- jdbc-v2/pom.xml: Language not supported
- jdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/ClickHouseLexer.g4: Language not supported
jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java
Outdated
Show resolved
Hide resolved
mzitnik
left a comment
mzitnik
left a comment
There was a problem hiding this comment.
Can we also add some performance tests to measure the time it takes for the parser
|
|
||
| if (parsedStatement.isInsert() && config.isBetaFeatureEnabled(DriverProperties.BETA_ROW_BINARY_WRITER)) { | ||
| /* | ||
| * RowBinary can be used when |
There was a problem hiding this comment.
It is more accurate to say. Only if all the fields of the table are included (and in the same order)
| @@ -0,0 +1,300 @@ | |||
|
|
|||
There was a problem hiding this comment.
Can you provide a source(link) for this information
There was a problem hiding this comment.
Sorry did not get the question - what information?
There was a problem hiding this comment.
Where did you take all the definitions
|
This PR is a game-changer! Is there a chance to get this merged in for 0.8.6? |
|
Summary
Closes #2348
Closes #2325
Closes #2283
Closes #2354
Checklist
Delete items not relevant to your PR:
?with input parameters #2348, Using roles with '-' results an error #2325, JDBC Insert query failing if the query does not contain VALUES #2283