srielau commented on code in PR #52334:
URL: https://github.com/apache/spark/pull/52334#discussion_r2426360293
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -1666,9 +1680,19 @@ alterColumnAction
| dropDefault=DROP DEFAULT
;
+stringLitWithoutMarker
+ : STRING_LITERAL
#stringLiteralValue
+ | {!double_quoted_identifiers}? DOUBLEQUOTED_STRING
#doubleQuotedStringLiteralValue
+;
+
+parameterMarker
+ : {parameter_substitution_enabled}? namedParameterMarker
#namedParameterMarkerRule
+ | {parameter_substitution_enabled}? QUESTION
#positionalParameterMarkerRule
+ ;
+
stringLit
- : STRING_LITERAL
- | {!double_quoted_identifiers}? DOUBLEQUOTED_STRING
+ : stringLitWithoutMarker
#stringLiteralInContext
+ | parameterMarker
#parameterStringValue
Review Comment:
You are wrong :-) These grammar changes provide coverage for all (hopefully)
places where literals are allowed.
For example: Wherever we allow an DOUBLE or DECIMAL we also allow INTEGER
and thus have coverage for parameter markers. Wherever we allow a
DATE/TIMESTAMP we also allow a STRING and thus allow parameter markers. And of
course expressions include literals, so any generic expression allows parameter
markers already in the grammar.
Note that I have specifically added testcases for "interesting" places. We
can add more if you feel there is a gap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
