Yikf commented on code in PR #4098:
URL: https://github.com/apache/kyuubi/pull/4098#discussion_r1063062613
##########
kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiSqlBaseLexer.g4:
##########
@@ -128,10 +127,9 @@ IDENTIFIER
:
[A-Za-z_$0-9\u0080-\uFFFF]*?[A-Za-z_$\u0080-\uFFFF]+?[A-Za-z_$0-9\u0080-\uFFFF]*
;
+// Follow Trino STRING
STRING
: '\'' ( ~'\'' | '\'\'' )* '\''
- | 'R\'' (~'\'')* '\''
- | 'R"'(~'"')* '"'
Review Comment:
That's a good idea, but one of my concerns is that if have both `STRING` and
`TRINO_STRING` in a lexer file, have a conflict problem, and you're going to
have to think about the order of the two, but whatever the order, you're going
to have a problem;
For `STRING`, Kyuubi has no strong dependency and needs to be consistent
with Spark, in the following ways:
1. Keep the status quo, `STRING` is the `STRING` defined by Kyuubi;
2. Keep it consistent with Spark `STRING`. We use the following method to
avoid conflicts with `ESCAPE`
```
fragment SEARCH_STRING_ESCAPE: '\'' '\\' '\'';
STRING_ESCAPE
: SEARCH_STRING_ESCAPE
;
STRING
: '\'' ( ~'\'' | '\'\'' )* '\''
| 'R\'' (~'\'')* '\''
| 'R"'(~'"')* '"'
;
```
But either way, there's a problem. here's an example, Both of the following
SQL are parsed as `GetSchemas`:
```SQL
SELECT TABLE_SCHEM COMMA TABLE_CATALOG FROM SYSTEM_JDBC_SCHEMAS
TABLE_CATALOG = 'test_catalog'
SELECT TABLE_SCHEM COMMA TABLE_CATALOG FROM SYSTEM_JDBC_SCHEMAS
TABLE_CATALOG = "test_catalog"
```
But I think this is acceptable and users of `TRINO FE` will follow this. If
we are worried about having a custom table with the same schema as the built-in
table and querying the same SQL using Spark, only the stirng format is not
consistent, we can cut the lexer file in the future. But I don't think that's
the case.
@ulysses-you FYI, WDYT
--
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]
