Open
Conversation
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
...v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4_light/ClickHouseLightParser.g4
Show resolved
Hide resolved
| AVG : A V G; | ||
|
|
||
| JSON_FALSE : 'false'; | ||
| JSON_TRUE : 'true'; |
There was a problem hiding this comment.
Boolean literals parsed only in lowercase
Medium Severity
JSON_TRUE and JSON_FALSE are defined as literal 'true'/'false' and are case-sensitive, unlike the keyword tokens which are case-insensitive. Queries using TRUE/FALSE (or mixed case) won’t tokenize as these literals, so literal parsing in ANTLR4_LIGHT can fail for otherwise valid inputs.
| stmt.execute("TRUNCATE " + table); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Integration test ignores parser selection parameter
Low Severity
testBatchInsertWithRowBinary now takes a SqlParserFacade.SQLParser parser argument and the data provider builds cases for multiple parsers, but the test never uses parser to configure the connection or statement creation. The added coverage is effectively unused.
Contributor
Author
There was a problem hiding this comment.
Will look into it.
|
| stmt.setAssignValuesListStopPosition(stop); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Light parser never sets useFunction flag for INSERT VALUES
Medium Severity
The ParsedPreparedStatementListener in ANTLR4LightParser never overrides enterInsertParameterFuncExpr to call stmt.setUseFunction(true). The grammar defines # InsertParameterFuncExpr as a label for identifier functionArgs in the assignmentValue rule, but the listener ignores this event. The existing ANTLR4 parser correctly sets useFunction in both enterInsertParameterFuncExpr and exitInsertParameterFuncExpr. When BETA_ROW_BINARY_WRITER is enabled, ConnectionImpl.prepareStatement checks !parsedStatement.isUseFunction() to decide routing to WriterStatementImpl—with the light parser, INSERT statements containing function calls in VALUES (e.g. now(), abs()) are incorrectly routed to the row binary writer, which cannot handle them.
Additional Locations (1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes
Checklist
Delete items not relevant to your PR:
Note
Medium Risk
Introduces a new selectable SQL parsing path that influences statement classification (
hasResultSet/insert/table extraction), which can affect execution behavior for edge-case queries; changes are covered by extensive new parser tests.Overview
Adds a new lightweight ANTLR4 grammar (
ClickHouseLexer.g4+ClickHouseLightParser.g4) and a correspondingSqlParserFacadeoption (SQLParser.ANTLR4_LIGHT) that parses only minimal structure (verb/USEdb, andINSERTtable + optional column list) while otherwise accepting arbitrary tokens.ParsedStatementandParsedPreparedStatementnow recordstatementVerb, and the light parser uses this plus a verb allowlist to inferhasResultSetwithout deep parsing; placeholder scanning remains the existing string-basedparseParameters.Tests are updated/added to cover the new parser mode (including quoted identifiers with dots and verb/result-set classification), and the row-binary batch insert integration test is moved from
PreparedStatementTesttoWriterStatementImplTestand parameterized to run under multiple parser selections.Written by Cursor Bugbot for commit 8207129. This will update automatically on new commits. Configure here.