szehon-ho commented on code in PR #57434:
URL: https://github.com/apache/spark/pull/57434#discussion_r3634742268


##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -2294,6 +2307,8 @@ ansiNonReserved
     | ROWS
     | SCHEMA
     | SCHEMAS
+    | SCD_TYPE_1

Review Comment:
   These two (and the matching `SCD_TYPE_1`/`SCD_TYPE_2` pair added to 
`nonReserved` below) can just be dropped from the non-reserved lists -- they 
don't serve the usual purpose, and removing them is safe:
   
   - Non-reserved only governs whether a token can be used as a bare 
identifier, but `SCD_TYPE_1`/`SCD_TYPE_2` are multi-word literals (`'SCD TYPE 
1'`), so they can never stand in as an identifier anyway.
   - `autoCdcStoredAsClause` references the tokens directly, so parsing `STORED 
AS SCD TYPE 1|2` doesn't depend on their non-reserved membership.
   - `SQLKeywordSuite`'s extractor regex is `[A-Z_]+`, which the digit in the 
token name fails, so these are already excluded from `allCandidateKeywords` and 
the computed non-reserved sets -- listing them here has no effect on the suite, 
and they were (correctly) never added to the docs table or the 
`keywords*.sql.out` golden files.
   - They're new in this PR, so nothing external depends on them.
   
   Net: it's a pure deletion with no docs/golden/test changes, and `scd` stays 
a valid identifier either way (it isn't a keyword in this approach).
   



-- 
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]

Reply via email to