szehon-ho commented on code in PR #57434:
URL: https://github.com/apache/spark/pull/57434#discussion_r3634685546
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseLexer.g4:
##########
@@ -460,6 +461,8 @@ SECOND: 'SECOND';
SECONDS: 'SECONDS';
SCHEMA: 'SCHEMA';
SCHEMAS: 'SCHEMAS';
+SCD_TYPE_1: 'SCD TYPE 1';
Review Comment:
This could be avoided (and SPARK-58270 resolved here) by composing the
clause from single-word tokens instead of the multi-word
`SCD_TYPE_1`/`SCD_TYPE_2` literals. `TYPE` and `INTEGER_VALUE` already exist,
so only a new `SCD` keyword is needed:
```antlr
SCD: 'SCD';
autoCdcStoredAsClause
: STORED AS SCD TYPE scdType=INTEGER_VALUE
;
```
then validate `scdType in {1, 2}` in `parseAutoCdcParams`. Benefits:
- The lexer tokenizes `SCD` / `TYPE` / `1` independently, so arbitrary
whitespace (and comments) between the words just works, without the
exactly-one-space limitation.
- `SCD` becomes a normal single-word keyword that flows through the usual
non-reserved machinery (docs + keyword golden files), instead of
`SCD_TYPE_1`/`SCD_TYPE_2` sitting in `ansiNonReserved`/`nonReserved`, where
they can never be identifiers and are silently skipped by `SQLKeywordSuite`
anyway. Only `SCD` needs adding to the non-reserved lists (to keep `scd` usable
as a bare identifier, same reason `HISTORY`/`TRACK` were added).
- `STORED AS SCD TYPE 3` then fails with a clear AST-level validation
message rather than a generic `PARSE_SYNTAX_ERROR`, matching the defensive
`case other` already in `buildChangeArgs`.
A couple of test additions would also help:
- Explicit `STORED AS SCD TYPE 1` combined with `TRACK HISTORY` is rejected
-- currently only the implicit default-SCD1 + `TRACK HISTORY` case is covered.
- `scd` (and `history` / `track`) still parse as identifiers, e.g. a column
or table named `scd`, to guard the non-reserved classification.
--
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]
