Fix jdbc-v2: assume a function in the values list of a recovered ANTLR4 parse tree - #3028
Open
polyglotAI-bot wants to merge 1 commit into
Open
Fix jdbc-v2: assume a function in the values list of a recovered ANTLR4 parse tree#3028polyglotAI-bot wants to merge 1 commit into
polyglotAI-bot wants to merge 1 commit into
Conversation
…R4 parse tree A function call in an insert values list is reported by a listener callback on the ANTLR4 parse tree. A statement the grammar cannot match is still given a parse tree, completed by error recovery, which skips the tokens the parser recovered on - a function call among them is never reported, so useFunction stayed false while a function was present. With the beta RowBinary writer enabled such an insert was then routed to it, where a literal function-call column cannot be written. Fixes: #3027
|
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
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.
Description
Fixes #3027.
A function call in an insert values list is reported to
ParsedPreparedStatement.useFunctiononly by a listener callbackon the ANTLR4 parse tree (
insertParameterFuncExpr). A statement the bundled grammar cannot match is still given a parsetree, completed by error recovery, which skips the tokens the parser recovered on: a function call among them is never
reported, so
useFunctionstayedfalsewhile a function was present - e.g.INSERT INTO t (v1, v2) VALUES (?, hex(x'AB')), valid ClickHouse the grammar has no hex string literal for.ConnectionImpl#prepareStatementuses thatflag to decide whether an insert can be written with the beta
RowBinarywriter (which requires a values list ofparameter placeholders only), so with
beta.row_binary_for_simple_insert=truesuch a statement was routed to the writer,where a literal function-call column cannot be written. Since a recovered tree cannot tell whether a function call is
present, both ANTLR4 backends now assume one for an insert that could not be parsed without errors, so the statement
takes the generic parameter substitution path - as it already did for a function call the grammar matches. The default
JAVACCbackend derives function usage from the SQL itself and is not affected.Changes
jdbc-v2/.../internal/SqlParserFacade.java: addedassumeFunctionInValuesListOfRecoveredParseTree(...), called fromthe existing
isHasErrors()branch ofANTLR4Parser#parsePreparedStatementand of the override inANTLR4AndParamsParser(the two entry points that build aParsedPreparedStatementfrom an ANTLR4 parse tree). Itsets
useFunctionfor an insert only - in these backends the flag describes the insert values list.CHANGELOG.md: bug-fix entry, including that the assumption applies to any insert parsed with errors.Test
BaseSqlParserFacadeTest#testInsertUseFunction(runs for all three parser backends): a@DataProviderpinninguseFunctionfor a values list of placeholders only (false), a function call the grammar matches (now(),toString(?)-true), and a function call it cannot match (hex(x'AB')in the first value, in the second value, andin the second of two value groups -
true). Each row also pins whether the statement parses without errors (ANTLR4backends), so the rows keep covering the error-recovery branch. 6 rows fail on
mainforANTLR4andANTLR4_PARAMS_PARSER, none with the fix; theJAVACCrows pass before and after.BaseSqlParserFacadeTest#testUseFunctionOfUnparseableSelect: contrast case - aSELECTthat cannot be parsed keepsreporting no function usage in the ANTLR4 backends (skipped for
JAVACC, which reports any function use).WriterStatementImplTest#testInsertWithUnparseableFunctionNotWrittenWithRowBinary(integration, both ANTLR4backends): through
prepareStatementwith the beta writer enabled,INSERT INTO t (v1, v2) VALUES (?, hex(x'AB'))isnot a
WriterStatementImpland storesv2 = 'AB'; a values list of placeholders only still is aWriterStatementImpland stores its bound value. Both rows fail on
main(routed to the writer).mvn -pl jdbc-v2 test: 1322 tests, 0 failures. Focused integration run (WriterStatementImplTest,PreparedStatementTest): 77 tests, 0 failures.Pre-PR validation gate
prepareStatement)prepareStatement→ConnectionImplrouting)AGENTS.md/docs/changes_checklist.md(new package-private method with thesmallest visibility and focused tests;
debug-level log on a non-hot error path that logs no SQL values;parametrized
@DataProviderinstead of near-identical methods; no public API or configuration change, sodocs/features.mdneeds no update)Related
Same family as #3019 / #3025: parse-tree-derived fields of a recovered ANTLR4 tree. This PR fixes a different field
(
useFunction) with a different consumer (theRowBinaryrouting decision) and is independent of the PRs for those.