Repository navigation
Conversation
Signed-off-by: 付典 <fudianchn@gmail.com>
fudianchn
force-pushed
the
fix/sample-extend-as-identifier
branch
from
October 11, 2026 12:51
3289526 to
8c023a2
Compare
This branch has not been deployed
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
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.
AI disclosure: this change was prepared with AI coding agents, reviewed and revised line by line by me.
What
SAMPLEandEXTENDcan be used as unquoted table and column names, explicit or implicit aliases, and CTE names, including directly afterUPDATE. Sampling clauses and the|> EXTENDpipe operator retain their tested meanings.Why
Both are non-reserved in the common dialects and reach the parser as ordinary names when downstream tools rewrite application SQL:
FROM sample/UPDATE sample SET ...(baomidou/mybatis-plus#6504, baomidou/mybatis-plus#6871) andUPDATE t SET extend = ?(baomidou/mybatis-plus#7029, failing for users since 5.2). The identifier failures are reproduced on the tested upstream baseline8d12a5d7a.How
Move
<K_SAMPLE>and<K_EXTEND>intoNonReservedWord()so the existing name, alias, CTE, expression and statement-dispatch helpers recognize them consistently. Add a guard toisAliasAhead():SAMPLEfollowed by(,BLOCK, an integer or a decimal starts sampling rather than an implicit alias. The existingASalias handling and|> EXTENDpipe dispatch remain in place.Root cause
Both tokens were classified as reserved for their operator uses. A
RelObjectName()-only whitelist does not update the other keyword classifiers, leaving implicit aliases, CTEs,UPDATEtable names and expression contexts inconsistent. Moving the tokens toNonReservedWord()fixes that classification; the sampling guard is needed because the newly accepted implicit alias could otherwise consume the start of aSAMPLEclause.Testing
KeywordsTest: 858 cases, 0 failures. Running the final test source on upstream8d12a5d7aproduces 21 failures among 854 cases (the parameterized keyword enumeration changes with the classification).UPDATEtable names,CASE, ternaries, and pipe expressions/functions. Assertions check AST node types and their positions as well as parse/deparse output.SELECT ALLcases assert the AST because existing serialization omitsALL.testSampleClauseStaysClausechecks the sampling AST and absence of an alias forSAMPLE (10),SAMPLE BLOCK (10) SEED (1),SAMPLE 0.1 OFFSET 1000,USING SAMPLE 10 PERCENT (BERNOULLI)andTABLESAMPLE SYSTEM (10). Removing the alias guard makes the threeSAMPLEcases fail; the other two remain passing../gradlew spotlessApply check jmhJar --no-daemonpasses with 9,980 tests;mvn -B -ntp clean verifypasses with 9,962 tests. Each suite has 0 failures, 0 errors and 25 existing skips. The initialKeywordsTest.javaSpotless formatting failures are corrected.JSQLParserBenchmark.parseSQLStatements,version=latest,performance.sqlchecked to produce 54 non-null statements: same Linux host (Intel i9-13900KS), JDK 17.0.20, 512 MiB heap, 3 forks, 2 x 1 s warmup and 5 x 1 s measurement iterations per fork (15 samples per run). Final baseline (8d12a5d7a) -> change -> baseline sequence for this PR: 26.942 [20.933, 32.951] -> 27.454 [20.596, 34.313] -> 26.908 [21.204, 32.612] ms/op; brackets are 99.9% confidence intervals. These short runs did not detect a performance regression; the wide confidence intervals do not rule out smaller differences or effects on other workloads.Behavior notes
t SAMPLE n,USING SAMPLE,TABLESAMPLEand|> EXTEND ...are unchanged.UPDATE sample SET a = 1andUPDATE extend SET a = 1now parse and round-trip, including the mixed-case variants in the tests. Implicit aliases use the new non-reserved classification; theSAMPLEguard preserves the tested sampling forms.Verification of the original issue
KeywordsTest.testSampleExtendAsIdentifiercovers the reported unquoted-name cases, includingSELECT * FROM sampleand assignments toextend. This test fails on upstream8d12a5d7aand passes on the final version; additional coverage verifies table names directly afterUPDATE. Final single commit8c023a204ad105606fddd66bba45824d67412518onfix/sample-extend-as-identifier.Partially addresses #2772.