feat: support OPTION hints after MERGE and INSERT ... VALUES, and the OPTIMIZE FOR (@var UNKNOWN) parameter form - #2486
Open
fudianchn wants to merge 1 commit into
Conversation
… OPTIMIZE FOR (@var UNKNOWN) parameter form Completes the follow-ups declared in the limitations of JSQLParser#2472: - attach the OPTION clause to MERGE statements, which rejected it before, mirroring the existing UPDATE and DELETE attach points - render the OPTION clause when deparsing INSERT ... VALUES: it was parsed into the Values select but silently dropped from the output - model the OPTIMIZE FOR (@variable_name UNKNOWN) parameter form as a new UnknownVariable expression (the @var = literal form already worked via VariableAssignment) Signed-off-by: 付典 <fudianchn@gmail.com>
manticore-projects
requested changes
Aug 18, 2026
| * <a href="https://learn.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-query">Hints | ||
| * (Transact-SQL) - Query Hints</a>. | ||
| */ | ||
| public class UnknownVariable extends ASTNodeAccessImpl implements Expression { |
Contributor
There was a problem hiding this comment.
Do we really need an extra class for this? Why not just carry a normal "Identifier" or "String" along? I would love to avoid especially the extra methods in the Visitors.
| (PlainSelect) assertSqlCanBeParsedAndDeparsed(sql, true); | ||
| OptionHint optimizeFor = plainSelect.getOption().getOptionHints().get(0); | ||
| Assertions.assertEquals(1, optimizeFor.getParameters().size()); | ||
| Assertions.assertTrue(optimizeFor.getParameters().get(0) instanceof UnknownVariable); |
Contributor
There was a problem hiding this comment.
Assertions should provide assertInstanceOf directly.
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
2 participants
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
Completes the follow-ups declared in the limitations of #2472 (SQL Server
OPTIONquery hints):MERGE ... OPTION (...)previously failed to parse. The clause is now attached toMERGEstatements, mirroring the existingUPDATE/DELETEattach points (Merge.setOption(...), deparser and validator included).INSERT INTO t (a) VALUES (1) OPTION (...)was parsed but theOPTIONclause was silently dropped from the deparser output:ValuesStatementDeParsernever rendered it. The modeling is unchanged (the clause lives on the innerValuesselect, as introduced in feat: support SQL Server OPTION query hints (#161) #2472); only the missing rendering is added.OPTIMIZE FOR (@variable_name UNKNOWN)previously failed to parse (the@var = literalform already worked viaVariableAssignment). The parameter form is modeled as a newUnknownVariableexpression, produced by a dedicatedOptionHintParameter()production with a semantic lookahead (S_AT_IDENTIFIERfollowed byK_UNKNOWN), so the bareUNKNOWNkeyword stays illegal everywhere else.The expression type is the first variant named in the #2472 limitation ("a dedicated expression type or a nullable-value
VariableAssignment"); the second variant would avoid the newExpressionVisitormethod but overloadVariableAssignmentwith a non-assignment meaning.Testing
OptionClauseTest: 5 new tests (18/18 pass), each failing on master (2 parse errors, 1 dropped clause, 2 more of the same); AST assertions pin theUnknownVariableparameter and the statement-level attach point. Full./gradlew spotlessCheck check: 4867 tests, 0 failures, javacc warning count unchanged.Performance
gradle jmh,JSQLParserBenchmark.parseSQLStatementsonperformance.sql, version=latest, 10 forks x 10 iterations (100 samples) per run, two interleaved runs per build on a 32-core host:70139091157f74Mean delta +0.4% with all four confidence intervals overlapping -> no regression. (Replaces an earlier non-interleaved measurement taken under transient background load, whose runs showed ~10x wider intervals.)