FrankChen021 commented on code in PR #19719:
URL: https://github.com/apache/druid/pull/19719#discussion_r3662302887


##########
sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParser.java:
##########
@@ -232,6 +259,29 @@ private static DruidException 
translateParseException(SqlParseException e)
     return InvalidSqlInput.exception(e.getMessage());
   }
 
+  private static boolean isIdentifierExpected(String[] tokenDictionary, 
int[][] expectedTokenSequences)
+  {
+    for (int[] expectedTokenSequence : expectedTokenSequences) {
+      if (expectedTokenSequence.length > 0) {
+        final String token = tokenDictionary[expectedTokenSequence[0]];
+        if ("<IDENTIFIER>".equals(token)
+            || "<QUOTED_IDENTIFIER>".equals(token)
+            || "<BACK_QUOTED_IDENTIFIER>".equals(token)
+            || "<BRACKET_QUOTED_IDENTIFIER>".equals(token)
+            || "<UNICODE_QUOTED_IDENTIFIER>".equals(token)) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+  private static boolean isFunctionCall(ParseException parseException)
+  {
+    final Token nextToken = parseException.currentToken.next;
+    return nextToken.next != null && "(".equals(nextToken.next.image);

Review Comment:
   Fixed in 584c521e9a. Function-call context is now determined from the 
original SQL at Calcite's failure position, rather than accessing an unfetched 
JavaCC lookahead token.



##########
sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserTest.java:
##########
@@ -140,4 +140,44 @@ public void test_sqlLiteralToContextValue_unsupportedType()
     );
     Assert.assertTrue(exception.getMessage().contains("Unsupported type for 
SET"));
   }
+
+  @Test
+  public void testParse_reservedKeywordIdentifier()
+  {
+    final DruidException exception = Assert.assertThrows(
+        DruidException.class,
+        () -> DruidSqlParser.parse("SELECT start FROM sys.\"segments\" LIMIT 
1", false)
+    );
+
+    Assert.assertEquals(
+        "Token [start] (line [1], column [8]) is a reserved keyword. "
+        + "To use it as an identifier, quote it as [\"start\"]",
+        exception.getMessage()
+    );
+  }
+
+  @Test
+  public void testParse_reservedKeywordOutsideIdentifierContext()
+  {
+    final DruidException exception = Assert.assertThrows(
+        DruidException.class,
+        () -> DruidSqlParser.parse("SELECT * FROM foo GROUP ORDER BY x", false)
+    );
+
+    Assert.assertTrue(exception.getMessage().contains("Received an unexpected 
token"));

Review Comment:
   Fixed in 584c521e9a. The test now asserts only that the specialized 
reserved-keyword hint is absent, without depending on Calcite diagnostic 
wording.



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