[CALCITE-4115] Improve the prompt of using SQL keywords for sql parse…

…s (part2) Reports the next token is a keyword only when SQL identifier is allowed.
picaqiu · Jul 27, 2020 · 4d345a6 · 4d345a6
1 parent 4625280
commit 4d345a6
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 6 deletions.
diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
@@ -440,7 +440,9 @@ JAVACODE SqlParseException convertException(Throwable ex)
             // Checks token.image.equals("1") to avoid recursive call.
             // The SqlAbstractParserImpl#MetadataImpl constructor uses constant "1" to
             // throw intentionally to collect the expected tokens.
-            if (!token.image.equals("1") && getMetadata().isKeyword(token.image)) {
+            if (!token.image.equals("1")
+                && getMetadata().isKeyword(token.image)
+                && SqlParserUtil.allowsIdentifier(tokenImage, expectedTokenSequences)) {
                 // If the next token is a keyword, reformat the error message as:
 
                 // Incorrect syntax near the keyword '{keyword}' at line {line_number},

diff --git a/core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java b/core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java
@@ -747,6 +747,30 @@ public static String quotingToParserState(Quoting quoting) {
     }
   }
 
+  /**
+   * Returns whether the reported ParseException tokenImage
+   * allows SQL identifier.
+   *
+   * @param tokenImage The allowed tokens from the ParseException
+   * @param expectedTokenSequences Expected token sequences
+   *
+   * @return true if SQL identifier is allowed
+   */
+  public static boolean allowsIdentifier(String[] tokenImage, int[][] expectedTokenSequences) {
+    // Compares from tailing tokens first because the <IDENTIFIER>
+    // was very probably at the tail.
+    for (int i = expectedTokenSequences.length - 1; i >= 0; i--) {
+      int[] expectedTokenSequence = expectedTokenSequences[i];
+      for (int j = expectedTokenSequence.length - 1; j >= 0; j--) {
+        if (tokenImage[expectedTokenSequence[j]].equals("<IDENTIFIER>")) {
+          return true;
+        }
+      }
+    }
+
+    return false;
+  }
+
   //~ Inner Classes ----------------------------------------------------------
 
   /** The components of a collation definition, per the SQL standard. */

diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -4309,7 +4309,7 @@ void checkPeriodPredicate(Checker checker) {
     expr("CAST(12 AS DATE)")
         .ok("CAST(12 AS DATE)");
     sql("CAST('2000-12-21' AS DATE ^NOT^ NULL)")
-        .fails("(?s).*Incorrect syntax near the keyword 'NOT' at line 1, column 27.*");
+        .fails("(?s).*Encountered \"NOT\" at line 1, column 27.*");
     sql("CAST('foo' as ^1^)")
         .fails("(?s).*Encountered \"1\" at line 1, column 15.*");
     expr("Cast(DATE '2004-12-21' AS VARCHAR(10))")
@@ -4395,9 +4395,9 @@ void checkPeriodPredicate(Checker checker) {
     expr("{fn convert(1, VARCHAR^(^5))}")
         .fails("(?s)Encountered \"\\(\" at.*");
     expr("{fn convert(1, ^INTERVAL^ YEAR TO MONTH)}")
-        .fails("(?s)Incorrect syntax near the keyword 'INTERVAL' at.*");
+        .fails("(?s)Encountered \"INTERVAL\" at.*");
     expr("{fn convert(1, ^INTERVAL^ YEAR)}")
-        .fails("(?s)Incorrect syntax near the keyword 'INTERVAL' at.*");
+        .fails("(?s)Encountered \"INTERVAL\" at.*");
   }
 
   @Test void testWindowReference() {

diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -8352,7 +8352,7 @@ private void testCurrentDateFunc(Pair<String, Hook.Closeable> pair) {
     tester.checkFails("timestampdiff(CENTURY, "
         + "timestamp '2014-02-24 12:42:25', "
         + "timestamp '2614-02-24 12:42:25')",
-        "(?s)Incorrect syntax near the keyword 'CENTURY' at .*", false);
+        "(?s)Encountered \"CENTURY\" at .*", false);
     tester.checkScalar("timestampdiff(QUARTER, "
         + "timestamp '2014-02-24 12:42:25', "
         + "cast(null as timestamp))",

diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -1566,7 +1566,7 @@ public void _testLikeAndSimilarFails() {
       expr("\"TRIM\"('b')").ok();
     } else {
       expr("^\"TRIM\"('b' FROM 'a')^")
-          .fails("(?s).*Incorrect syntax near the keyword 'FROM' at .*");
+          .fails("(?s).*Encountered \"FROM\" at .*");
 
       // Without the "FROM" noise word, TRIM is parsed as a regular
       // function without quoting and built-in function with quoting.