This is an automated email from the ASF dual-hosted git repository. michaelsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 644a6890d755bda67287fa78c64363426ff3a1e4 Author: Gergely Fürnstáhl <[email protected]> AuthorDate: Wed Nov 30 11:42:30 2022 +0100 IMPALA-11758: Fixed error detection for usage of reserved words. Fixed the control flow of checking whether the parsing failed on the usage of a reserved word where an identifier was expected. Added a hint to the error message on how to fix the query. Query: create database iceberg ERROR: ParseException: Syntax error in line 1: create database iceberg ^ Encountered: ICEBERG Expected: DEFAULT, EXTENDED, FORMATTED, IF, IDENTIFIER Hint: reserved words have to be escaped when used as an identifier, e.g. `iceberg` CAUSED BY: Exception: Syntax error Testing: - Added new tests to test_reserved_words_version.py - Adjusted ParserTests - Built docs Change-Id: I9605d80a25ff878d12f6ca2ebb99ac26298d8efa Reviewed-on: http://gerrit.cloudera.org:8080/19290 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- docs/topics/impala_reserved_words.xml | 7 +++++++ fe/src/main/cup/sql-parser.cup | 24 +++++++++++++++++----- .../org/apache/impala/analysis/ParserTest.java | 20 ++++++++++++++---- .../custom_cluster/test_reserved_words_version.py | 21 +++++++++++++------ 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/docs/topics/impala_reserved_words.xml b/docs/topics/impala_reserved_words.xml index abd5c6a17..70debb14a 100644 --- a/docs/topics/impala_reserved_words.xml +++ b/docs/topics/impala_reserved_words.xml @@ -1553,6 +1553,13 @@ See the history, any recent changes, here: <entry/> <entry/> </row> + <row> + <entry><codeph>iceberg</codeph></entry> + <entry/> + <entry/> + <entry>X<fn>Impala 4.0 and higher</fn></entry> + <entry/> + </row> <row> <entry><codeph>identity</codeph></entry> <entry>X</entry> diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup index cc6760897..32ca8ccbf 100755 --- a/fe/src/main/cup/sql-parser.cup +++ b/fe/src/main/cup/sql-parser.cup @@ -241,17 +241,17 @@ parser code {: result.append("Encountered: "); String lastToken = SqlScanner.tokenIdMap.get(Integer.valueOf(errorToken_.sym)); + if (lastToken != null) { result.append(lastToken); - } else if (SqlScanner.isReserved((String)errorToken_.value)) { - result.append("A reserved word cannot be used as an identifier: ") - .append((String)errorToken_.value); } else { result.append("Unknown last token with id: " + errorToken_.sym); } + result.append("\n"); + + boolean identifierExpected = false; // append expected tokens - result.append('\n'); result.append("Expected: "); if (expectedTokenName_ == null) { String expectedToken = null; @@ -260,15 +260,29 @@ parser code {: tokenId = expectedTokenIds_.get(i); if (reportExpectedToken(tokenId, expectedTokenIds_.size())) { expectedToken = SqlScanner.tokenIdMap.get(tokenId); + if(expectedToken.equals(SqlScanner.tokenIdMap.get(SqlParserSymbols.IDENT))) { + identifierExpected = true; + } result.append(expectedToken + ", "); } } // remove trailing ", " result.delete(result.length()-2, result.length()); } else { + if(expectedTokenName_.equals(SqlScanner.tokenIdMap.get(SqlParserSymbols.IDENT))) { + identifierExpected = true; + } result.append(expectedTokenName_); } - result.append('\n'); + result.append("\n\n"); + + // Giving hints if the parsing failed on a reserved word when expected an identifier + if(identifierExpected && errorToken_.value != null && + SqlScanner.isReserved(errorToken_.value.toString())) { + result.append( + "Hint: reserved words have to be escaped when used as an identifier, e.g. `") + .append(errorToken_.value.toString()).append("`\n"); + } return result.toString(); } diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java index 7f9e70eaf..83630c144 100755 --- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java @@ -3570,7 +3570,10 @@ public class ParserTest extends FrontendTestBase { "Encountered: FROM\n" + "Expected: ALL, CASE, CAST, DATE, DEFAULT, DISTINCT, EXISTS, FALSE, GROUPING, " + "IF, INTERVAL, LEFT, NOT, NULL, REPLACE, RIGHT, STRAIGHT_JOIN, TRUNCATE, TRUE, " + - "UNNEST, IDENTIFIER"); + "UNNEST, IDENTIFIER\n" + + "\n" + + "Hint: reserved words have to be escaped when used as an identifier, e.g. `from`" + ); // missing from ParserError("select c, b, c where a = 5", @@ -3580,7 +3583,10 @@ public class ParserTest extends FrontendTestBase { "Encountered: WHERE\n" + "Expected: AND, AS, BETWEEN, DEFAULT, DIV, EXCEPT, FROM, ILIKE, IN, INTERSECT, " + "IREGEXP, IS, LIKE, LIMIT, ||, MINUS, NOT, OR, ORDER, REGEXP, RLIKE, UNION, " + - "COMMA, IDENTIFIER\n"); + "COMMA, IDENTIFIER\n" + + "\n" + + "Hint: reserved words have to be escaped when used as an identifier, e.g. `where`" + ); // missing table list ParserError("select c, b, c from where a = 5", @@ -3588,7 +3594,10 @@ public class ParserTest extends FrontendTestBase { "select c, b, c from where a = 5\n" + " ^\n" + "Encountered: WHERE\n" + - "Expected: DEFAULT, UNNEST, IDENTIFIER\n"); + "Expected: DEFAULT, UNNEST, IDENTIFIER\n" + + "\n" + + "Hint: reserved words have to be escaped when used as an identifier, e.g. `where`" + ); // missing predicate in where clause (no group by) ParserError("select c, b, c from t where", @@ -3608,7 +3617,10 @@ public class ParserTest extends FrontendTestBase { "Encountered: GROUP\n" + "Expected: CASE, CAST, DATE, DEFAULT, EXISTS, FALSE, GROUPING, IF, INTERVAL, " + "LEFT, NOT, NULL, REPLACE, RIGHT, STRAIGHT_JOIN, TRUNCATE, TRUE, UNNEST, " + - "IDENTIFIER"); + "IDENTIFIER\n" + + "\n" + + "Hint: reserved words have to be escaped when used as an identifier, e.g. `group`" + ); // unmatched string literal starting with " ParserError("select c, \"b, c from t", diff --git a/tests/custom_cluster/test_reserved_words_version.py b/tests/custom_cluster/test_reserved_words_version.py index 424d6bb5e..0c3b80075 100644 --- a/tests/custom_cluster/test_reserved_words_version.py +++ b/tests/custom_cluster/test_reserved_words_version.py @@ -21,18 +21,27 @@ from tests.common.custom_cluster_test_suite import CustomClusterTestSuite class TestReservedWordsVersion(CustomClusterTestSuite): + def __test_common(self): + self.execute_query_expect_success(self.client, "select 1 as year") + self.execute_query_expect_success(self.client, "select 1 as avg") + + assert "Hint: reserved words have to be escaped when used as an identifier, e.g. " \ + "`iceberg`" in str(self.execute_query_expect_failure(self.client, + "create database iceberg")) + self.execute_query_expect_success(self.client, "create database `iceberg`") + self.execute_query_expect_success(self.client, "drop database `iceberg`") + @pytest.mark.execute_serially @CustomClusterTestSuite.with_args("--reserved_words_version=3.0.0") def test_3_0(self): - assert "A reserved word cannot be used as an identifier: at" in \ - str(self.execute_query_expect_failure(self.client, "select 1 as at")) + assert "Hint: reserved words have to be escaped when used as an identifier, e.g. " \ + "`at`" in str(self.execute_query_expect_failure(self.client, "select 1 as at")) self.execute_query_expect_success(self.client, "select 1 as `at`") - self.execute_query_expect_success(self.client, "select 1 as year") - self.execute_query_expect_success(self.client, "select 1 as avg") + self.__test_common() @pytest.mark.execute_serially @CustomClusterTestSuite.with_args("--reserved_words_version=2.11.0") def test_2_11(self): self.execute_query_expect_success(self.client, "select 1 as at") - self.execute_query_expect_success(self.client, "select 1 as year") - self.execute_query_expect_success(self.client, "select 1 as avg") + + self.__test_common()
