From 2f7fce3733996738d88dc499027aaaf78781bd71 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Thu, 10 Sep 2015 18:11:22 +0900 Subject: [PATCH 1/3] TAJO-1817: Improve SQL parser error message. --- .../tajo/parser/sql/TestSQLAnalyzer.java | 58 ++++++++++++---- .../TestSQLAnalyzer/errors/identifier1.sql | 9 +++ .../TestSQLAnalyzer/errors/in_subquery_1.sql | 7 ++ .../queries/TestSQLAnalyzer/errors/join_1.sql | 13 ++++ .../TestSQLAnalyzer/errors/identifier1.result | 3 + .../errors/in_subquery_1.result | 3 + .../TestSQLAnalyzer/errors/join_1.result | 3 + .../apache/tajo/parser/sql/SQLAnalyzer.java | 37 ++++++----- .../tajo/parser/sql/SQLErrorListener.java | 5 +- .../tajo/parser/sql/SQLErrorStrategy.java | 66 ------------------- .../apache/tajo/parser/sql/SQLParseError.java | 15 +++-- .../tajo/jdbc/TestTajoJdbcNegative.java | 39 +++++++++++ .../org/apache/tajo/parser/sql/SQLParser.g4 | 3 +- 13 files changed, 156 insertions(+), 105 deletions(-) create mode 100644 tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/identifier1.sql create mode 100644 tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/in_subquery_1.sql create mode 100644 tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/join_1.sql create mode 100644 tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/identifier1.result create mode 100644 tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/in_subquery_1.result create mode 100644 tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/join_1.result delete mode 100644 tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorStrategy.java diff --git a/tajo-core-tests/src/test/java/org/apache/tajo/parser/sql/TestSQLAnalyzer.java b/tajo-core-tests/src/test/java/org/apache/tajo/parser/sql/TestSQLAnalyzer.java index e6eb71f171..e9d3784891 100644 --- a/tajo-core-tests/src/test/java/org/apache/tajo/parser/sql/TestSQLAnalyzer.java +++ b/tajo-core-tests/src/test/java/org/apache/tajo/parser/sql/TestSQLAnalyzer.java @@ -30,6 +30,7 @@ import org.apache.tajo.algebra.Expr; import org.apache.tajo.annotation.Nullable; import org.apache.tajo.conf.TajoConf; +import org.apache.tajo.exception.SQLSyntaxError; import org.apache.tajo.storage.StorageUtil; import org.apache.tajo.util.FileUtil; import org.apache.tajo.util.Pair; @@ -54,19 +55,9 @@ */ public class TestSQLAnalyzer { @Rule public TestName name = new TestName(); - - public static Expr parseQuery(String sql) { - ANTLRInputStream input = new ANTLRInputStream(sql); - SQLLexer lexer = new SQLLexer(input); - CommonTokenStream tokens = new CommonTokenStream(lexer); - - SQLParser parser = new SQLParser(tokens); - parser.setErrorHandler(new BailErrorStrategy()); - parser.setBuildParseTree(true); - - SQLAnalyzer visitor = new SQLAnalyzer(); - SQLParser.SqlContext context = parser.sql(); - return visitor.visitSql(context); + final static SQLAnalyzer analyzer = new SQLAnalyzer(); + public static Expr parseQuery(String sql) throws SQLSyntaxError { + return analyzer.parse(sql); } public Collection getResourceFiles(String subdir) throws URISyntaxException, IOException { @@ -134,6 +125,38 @@ public void testPositiveTests() throws IOException, URISyntaxException { fail("Parsing '" + pair.getFirst() + "' failed, its cause: " + t.getMessage()); } } + System.out.flush(); + } + + /** + * In order to add more unit tests, please add SQL files into resources/results/TestSQLAnalyzer/positive. + * This test just checkes if SQL statements are parsed successfully. + * + * @throws IOException + * @throws URISyntaxException + */ + @Test + public void testErrorMessages() throws IOException, URISyntaxException { + for (Pair pair : getFileContents("errors")) { + + String fileName = pair.getFirst().split("\\.")[0]; + String expectedResult = ""; + + try { + expectedResult = FileUtil.readTextFileFromResource("results/TestSQLAnalyzer/errors/" + fileName + ".result"); + } catch (FileNotFoundException fnfe) { + } + + try { + Expr expr = parseQuery(pair.getSecond()); + System.out.println(expr); + fail(pair.getFirst() + " must be failed."); + } catch (SQLSyntaxError e) { + assertEquals("Error message is different from " + fileName + ".result", expectedResult, e.getMessage()); + } + System.out.println(pair.getFirst() + " test passed..."); + } + System.out.flush(); } /** @@ -148,7 +171,12 @@ public void testPositiveTests() throws IOException, URISyntaxException { @Test public void testGeneratedAlgebras() throws IOException, URISyntaxException { for (Pair pair : getFileContents(".")) { - Expr expr = parseQuery(pair.getSecond()); + Expr expr = null; + try { + expr = parseQuery(pair.getSecond()); + } catch (SQLSyntaxError t) { + fail(t.getMessage()); + } String expectedResult = null; String fileName = null; @@ -165,6 +193,7 @@ public void testGeneratedAlgebras() throws IOException, URISyntaxException { expectedResult.trim(), expr.toJson().trim()); System.out.println(pair.getFirst() + " test passed.."); } + System.out.flush(); } private static Expr parseExpr(String sql) { @@ -194,6 +223,7 @@ public void testExprs() throws IOException, URISyntaxException { testExprs(pair.getFirst(), pair.getSecond()); System.out.println(pair.getFirst() + " test passed.."); } + System.out.flush(); } private void testExprs(String file, String fileContents) { diff --git a/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/identifier1.sql b/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/identifier1.sql new file mode 100644 index 0000000000..3cca6a06b2 --- /dev/null +++ b/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/identifier1.sql @@ -0,0 +1,9 @@ +SELECT x,y,z FROM ( + SELECT + l, + K + FROM + TTT + 1 +) XX; + diff --git a/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/in_subquery_1.sql b/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/in_subquery_1.sql new file mode 100644 index 0000000000..8102e1f1a0 --- /dev/null +++ b/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/in_subquery_1.sql @@ -0,0 +1,7 @@ +select + * +from + lineitem +where l_orderkey in ( + select xxx 1from inner_table +) \ No newline at end of file diff --git a/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/join_1.sql b/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/join_1.sql new file mode 100644 index 0000000000..11c66ce821 --- /dev/null +++ b/tajo-core-tests/src/test/resources/queries/TestSQLAnalyzer/errors/join_1.sql @@ -0,0 +1,13 @@ +SELECT + d_date_sk ss_sold_date_sk +FROM s_purchase + LEFTT OUTER JOIN customer ON (purc_customer_id = c_customer_id) + LEFT OUTER JOIN store ON (purc_store_id = s_store_id) + LEFT OUTER JOIN date_dim ON (cast(purc_purchase_date as date) = d_date) + LEFT OUTER JOIN time_dim ON (PURC_PURCHASE_TIME = t_time) + JOIN s_purchase_lineitem ON (purc_purchase_id = plin_purchase_id) + LEFT OUTER JOIN promotion ON plin_promotion_id = p_promo_id + LEFT OUTER JOIN item ON plin_item_id = i_item_id +WHERE purc_purchase_id = plin_purchase_id + AND i_rec_end_date is NULL + AND s_rec_end_date is NULL; \ No newline at end of file diff --git a/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/identifier1.result b/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/identifier1.result new file mode 100644 index 0000000000..7e17fb2d2d --- /dev/null +++ b/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/identifier1.result @@ -0,0 +1,3 @@ +ERROR: syntax error at or near "1" +LINE 7: 1 + ^ \ No newline at end of file diff --git a/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/in_subquery_1.result b/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/in_subquery_1.result new file mode 100644 index 0000000000..1c6a429149 --- /dev/null +++ b/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/in_subquery_1.result @@ -0,0 +1,3 @@ +ERROR: syntax error at or near "1" +LINE 6: select xxx 1from inner_table + ^ \ No newline at end of file diff --git a/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/join_1.result b/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/join_1.result new file mode 100644 index 0000000000..fd0cb948c5 --- /dev/null +++ b/tajo-core-tests/src/test/resources/results/TestSQLAnalyzer/errors/join_1.result @@ -0,0 +1,3 @@ +ERROR: syntax error at or near "OUTER" +LINE 4: LEFTT OUTER JOIN customer ON (purc_customer_id = c_customer_id) + ^^^^^ \ No newline at end of file diff --git a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java index 188a4f51d9..c62cb1f102 100644 --- a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java +++ b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java @@ -35,6 +35,7 @@ import org.apache.tajo.algebra.LiteralValue.LiteralType; import org.apache.tajo.algebra.Sort.SortSpec; import org.apache.tajo.exception.SQLSyntaxError; +import org.apache.tajo.exception.TajoInternalError; import org.apache.tajo.exception.TajoRuntimeException; import org.apache.tajo.parser.sql.SQLParser.*; import org.apache.tajo.storage.StorageConstants; @@ -52,20 +53,26 @@ public class SQLAnalyzer extends SQLParserBaseVisitor { public Expr parse(String sql) throws SQLSyntaxError { - ANTLRInputStream input = new ANTLRInputStream(sql); - SQLLexer lexer = new SQLLexer(input); - CommonTokenStream tokens = new CommonTokenStream(lexer); + + final ANTLRInputStream input = new ANTLRInputStream(sql); + final SQLLexer lexer = new SQLLexer(input); + lexer.removeErrorListeners(); + lexer.addErrorListener(new SQLErrorListener()); + + final CommonTokenStream tokens = new CommonTokenStream(lexer); + SqlContext context; try { - SQLParser parser = new SQLParser(tokens); - parser.setBuildParseTree(true); + final SQLParser parser = new SQLParser(tokens); parser.removeErrorListeners(); - - parser.setErrorHandler(new SQLErrorStrategy()); parser.addErrorListener(new SQLErrorListener()); + parser.setBuildParseTree(true); + context = parser.sql(); - } catch (SQLParseError e) { - throw new SQLSyntaxError(e.getMessage()); + } catch (TajoRuntimeException e) { + throw new SQLSyntaxError(e.getCause().getMessage()); + } catch (Throwable t) { + throw new TajoInternalError(t.getMessage()); } return visitSql(context); } @@ -118,7 +125,7 @@ public Expr visitSession_statement(@NotNull Session_statementContext ctx) { return new SetSession(SessionVars.TIMEZONE.name(), value); } else { - throw new TajoRuntimeException(new SQLSyntaxError("Unsupported session statement")); + throw new TajoInternalError(new SQLSyntaxError("Invalid session statement")); } } @@ -970,7 +977,7 @@ public static OpType tokenToExprType(int tokenId) { return OpType.Minus; default: - throw new RuntimeException("Unknown Token Id: " + tokenId); + throw new TajoInternalError("unknown Token Id: " + tokenId); } } @@ -1022,7 +1029,7 @@ public Expr visitPattern_matching_predicate(Pattern_matching_predicateContext ct } else if (checkIfExist(matcher.REGEXP()) || checkIfExist(matcher.RLIKE())) { return new PatternMatchPredicate(OpType.Regexp, not, predicand, pattern); } else { - throw new TajoRuntimeException(new SQLSyntaxError("Unsupported predicate: " + matcher.getText())); + throw new TajoInternalError("unknown pattern matching predicate: " + matcher.getText()); } } else if (checkIfExist(ctx.pattern_matcher().regex_matcher())) { Regex_matcherContext matcher = ctx.pattern_matcher().regex_matcher(); @@ -1035,10 +1042,10 @@ public Expr visitPattern_matching_predicate(Pattern_matching_predicateContext ct } else if (checkIfExist(matcher.Not_Similar_To_Case_Insensitive())) { return new PatternMatchPredicate(OpType.Regexp, true, predicand, pattern, true); } else { - throw new TajoRuntimeException(new SQLSyntaxError("Unsupported predicate: " + matcher.getText())); + throw new TajoInternalError("Unsupported predicate: " + matcher.getText()); } } else { - throw new TajoRuntimeException(new SQLSyntaxError("Unsupported predicate: " + ctx.pattern_matcher().getText())); + throw new TajoInternalError("invalid pattern matching predicate: " + ctx.pattern_matcher().getText()); } } @@ -1411,7 +1418,7 @@ public PartitionMethodDescExpr parseTablePartitioningClause(Table_partitioning_c } else if (checkIfExist(ctx.column_partitions())) { // For Column Partition (Hive Style) return new CreateTable.ColumnPartition(getDefinitions(ctx.column_partitions().table_elements())); } else { - throw new TajoRuntimeException(new SQLSyntaxError("Invalid Partition Type: " + ctx.toStringTree())); + throw new TajoInternalError("invalid partition type: " + ctx.toStringTree()); } } diff --git a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java index e96084431b..a04bd71de9 100644 --- a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java +++ b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java @@ -20,8 +20,10 @@ import org.antlr.v4.runtime.*; import org.apache.commons.lang.StringUtils; +import org.apache.tajo.exception.TajoRuntimeException; public class SQLErrorListener extends BaseErrorListener { + public void syntaxError(Recognizer recognizer, Object offendingSymbol, int line, int charPositionInLine, @@ -33,6 +35,7 @@ public void syntaxError(Recognizer recognizer, String[] lines = StringUtils.splitPreserveAllTokens(input, '\n'); String errorLine = lines[line - 1]; - throw new SQLParseError(token, line, charPositionInLine, msg, errorLine); + String simpleMessage = "syntax error at or near \"" + token.getText() + "\""; + throw new TajoRuntimeException(new SQLParseError(token, line, charPositionInLine, simpleMessage, errorLine)); } } diff --git a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorStrategy.java b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorStrategy.java deleted file mode 100644 index 236b85411d..0000000000 --- a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorStrategy.java +++ /dev/null @@ -1,66 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.tajo.parser.sql; - -import org.antlr.v4.runtime.*; -import org.antlr.v4.runtime.misc.NotNull; - -public class SQLErrorStrategy extends DefaultErrorStrategy { - - @Override - public void reportError(Parser recognizer, RecognitionException e) { - // if we've already reported an error and have not matched a token - // yet successfully, don't report any errors. - if (inErrorRecoveryMode(recognizer)) { - return; // don't report spurious errors - } - beginErrorCondition(recognizer); - if (e instanceof NoViableAltException) { - reportNoViableAltException(recognizer, (NoViableAltException) e); - } else if (e instanceof InputMismatchException) { - reportInputMismatchException(recognizer, (InputMismatchException) e); - } else if (e instanceof FailedPredicateException) { - reportFailedPredicate(recognizer, (FailedPredicateException) e); - } else { - recognizer.notifyErrorListeners(e.getOffendingToken(), e.getMessage(), e); - } - } - - protected void reportNoViableAltException(@NotNull Parser recognizer, @NotNull NoViableAltException e) { - TokenStream tokens = recognizer.getInputStream(); - String msg; - Token token = e.getStartToken(); - if (tokens != null) { - if (tokens.LT(-1) != null && token.getType() == Token.EOF) { - token = tokens.LT(-1); - } - msg = "syntax error at or near " + getTokenErrorDisplay(token); - } else { - msg = "no viable alternative at input " + escapeWSAndQuote(""); - } - recognizer.notifyErrorListeners(token, msg, e); - } - - protected void reportInputMismatchException(@NotNull Parser recognizer, - @NotNull InputMismatchException e) { - String msg = "mismatched input " + getTokenErrorDisplay(e.getOffendingToken()) + - " expecting " + e.getExpectedTokens().toString(recognizer.getTokenNames()); - recognizer.notifyErrorListeners(e.getOffendingToken(), msg, e); - } -} diff --git a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java index f0568a5e76..78d445d25e 100644 --- a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java +++ b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java @@ -21,8 +21,13 @@ import org.antlr.v4.runtime.Token; import org.apache.commons.lang.StringUtils; +import org.apache.tajo.error.Errors; +import org.apache.tajo.exception.TajoException; -public class SQLParseError extends RuntimeException { +/** + * Exception that represents a kind of SQL syntax error caused by the parser layer + */ +public class SQLParseError extends TajoException { private String header; private String errorLine; private int charPositionInLine; @@ -34,7 +39,7 @@ public SQLParseError(Token offendingToken, int line, int charPositionInLine, String msg, String errorLine) { - super(msg); + super(Errors.ResultCode.SYNTAX_ERROR, msg); this.offendingToken = offendingToken; this.charPositionInLine = charPositionInLine; this.line = line; @@ -58,14 +63,10 @@ public String getMessage() { return detailedMessage; } - public String getMessageHeader(){ - return this.header; - } - private String getDetailedMessageWithLocation() { StringBuilder sb = new StringBuilder(); int displayLimit = 80; - String queryPrefix = "LINE " + line + ":" + charPositionInLine + " "; + String queryPrefix = "LINE " + line + ":" + " "; String prefixPadding = StringUtils.repeat(" ", queryPrefix.length()); String locationString; diff --git a/tajo-jdbc/src/test/java/org/apache/tajo/jdbc/TestTajoJdbcNegative.java b/tajo-jdbc/src/test/java/org/apache/tajo/jdbc/TestTajoJdbcNegative.java index 232292bbbf..c1414dc025 100644 --- a/tajo-jdbc/src/test/java/org/apache/tajo/jdbc/TestTajoJdbcNegative.java +++ b/tajo-jdbc/src/test/java/org/apache/tajo/jdbc/TestTajoJdbcNegative.java @@ -115,6 +115,45 @@ public void testConnectionClosed() throws SQLException, IOException { } } + @Test + public void testSyntaxErrorOnExecuteUpdate() throws Exception { + String connUri = buildConnectionUri(tajoMasterAddress.getHostName(), tajoMasterAddress.getPort(), + DEFAULT_DATABASE_NAME); + Connection conn = DriverManager.getConnection(connUri); + assertTrue(conn.isValid(100)); + + try (Statement stmt = conn.createStatement()) { + stmt.executeUpdate("CREATE TABLE \n1table123u8sd ( name RECORD(last TEXT, first TEXT) )"); + fail("Must be failed"); + } catch (SQLException s) { + assertEquals(toSQLState(ResultCode.SYNTAX_ERROR), s.getSQLState()); + assertEquals( + "ERROR: syntax error at or near \"1\"\n" + + "LINE 2: 1table123u8sd ( name RECORD(last TEXT, first TEXT) )\n" + + " ^", s.getMessage()); + } + } + + @Test + public void testSyntaxErrorOnExecuteQuery() throws Exception { + String connUri = buildConnectionUri(tajoMasterAddress.getHostName(), tajoMasterAddress.getPort(), + DEFAULT_DATABASE_NAME); + Connection conn = DriverManager.getConnection(connUri); + assertTrue(conn.isValid(100)); + + try (Statement stmt = conn.createStatement()) { + try (ResultSet result = stmt.executeQuery("SELECT\n*\nFROM_ LINEITEM")) { + fail("Must be failed"); + } catch (SQLException s) { + assertEquals(toSQLState(ResultCode.SYNTAX_ERROR), s.getSQLState()); + assertEquals( + "ERROR: syntax error at or near \"from_\"\n" + + "LINE 3: FROM_ LINEITEM\n" + + " ^^^^^", s.getMessage()); + } + } + } + @Test public void testImmediateException() throws Exception { String connUri = buildConnectionUri(tajoMasterAddress.getHostName(), tajoMasterAddress.getPort(), diff --git a/tajo-sql-parser/src/main/antlr4/org/apache/tajo/parser/sql/SQLParser.g4 b/tajo-sql-parser/src/main/antlr4/org/apache/tajo/parser/sql/SQLParser.g4 index 41de218ee5..87b897dc9d 100644 --- a/tajo-sql-parser/src/main/antlr4/org/apache/tajo/parser/sql/SQLParser.g4 +++ b/tajo-sql-parser/src/main/antlr4/org/apache/tajo/parser/sql/SQLParser.g4 @@ -53,8 +53,7 @@ statement session_statement : SET CATALOG dbname = identifier | SET TIME ZONE (TO | EQUAL)? (Character_String_Literal | signed_numerical_literal | DEFAULT) - | SET (SESSION)? name=identifier (TO | EQUAL)? - (Character_String_Literal | signed_numerical_literal | boolean_literal | DEFAULT) + | SET (SESSION)? name=identifier (TO | EQUAL)? (Character_String_Literal | signed_numerical_literal | boolean_literal | DEFAULT) | RESET name=identifier ; From 2b57e6f48c47dab4c9ba9a0b6edfeac0fef7141a Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Thu, 10 Sep 2015 20:39:00 +0900 Subject: [PATCH 2/3] Fixed unit test failure. --- .../results/TestTajoCliNegatives/testQuerySyntax.result | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tajo-core-tests/src/test/resources/results/TestTajoCliNegatives/testQuerySyntax.result b/tajo-core-tests/src/test/resources/results/TestTajoCliNegatives/testQuerySyntax.result index 86b0b59fa2..5db9b3e6ea 100644 --- a/tajo-core-tests/src/test/resources/results/TestTajoCliNegatives/testQuerySyntax.result +++ b/tajo-core-tests/src/test/resources/results/TestTajoCliNegatives/testQuerySyntax.result @@ -1,3 +1,3 @@ -ERROR: mismatched input '-' expecting {, EXCEPT, GROUP, HAVING, INTERSECT, LIMIT, ORDER, UNION, WHERE, WINDOW, ';', ','} -LINE 1:21 select * from unknown-table - ^ +ERROR: syntax error at or near "-" +LINE 1: select * from unknown-table + ^ From 3361c3d02e7a3fb4e3995337e135112544f4c633 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Thu, 10 Sep 2015 21:12:18 +0900 Subject: [PATCH 3/3] Refined the exception. --- .../main/java/org/apache/tajo/exception/SQLSyntaxError.java | 4 ++++ .../main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java | 6 +++--- .../java/org/apache/tajo/parser/sql/SQLErrorListener.java | 3 +-- .../main/java/org/apache/tajo/parser/sql/SQLParseError.java | 6 ++---- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tajo-common/src/main/java/org/apache/tajo/exception/SQLSyntaxError.java b/tajo-common/src/main/java/org/apache/tajo/exception/SQLSyntaxError.java index 886032196d..36b6c0ddc2 100644 --- a/tajo-common/src/main/java/org/apache/tajo/exception/SQLSyntaxError.java +++ b/tajo-common/src/main/java/org/apache/tajo/exception/SQLSyntaxError.java @@ -22,9 +22,13 @@ import org.apache.tajo.error.Errors; import org.apache.tajo.rpc.protocolrecords.PrimitiveProtos.ReturnState; +/** + * Exception for SQL Syntax errors + */ public class SQLSyntaxError extends TajoException { private static final long serialVersionUID = 5388279335175632067L; + @SuppressWarnings("unused") public SQLSyntaxError(ReturnState state) { super(state); } diff --git a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java index c62cb1f102..5f787c012a 100644 --- a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java +++ b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java @@ -69,8 +69,8 @@ public Expr parse(String sql) throws SQLSyntaxError { parser.setBuildParseTree(true); context = parser.sql(); - } catch (TajoRuntimeException e) { - throw new SQLSyntaxError(e.getCause().getMessage()); + } catch (SQLParseError e) { + throw new SQLSyntaxError(e.getMessage()); } catch (Throwable t) { throw new TajoInternalError(t.getMessage()); } @@ -125,7 +125,7 @@ public Expr visitSession_statement(@NotNull Session_statementContext ctx) { return new SetSession(SessionVars.TIMEZONE.name(), value); } else { - throw new TajoInternalError(new SQLSyntaxError("Invalid session statement")); + throw new TajoInternalError("Invalid session statement"); } } diff --git a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java index a04bd71de9..4c80f4cc3a 100644 --- a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java +++ b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLErrorListener.java @@ -20,7 +20,6 @@ import org.antlr.v4.runtime.*; import org.apache.commons.lang.StringUtils; -import org.apache.tajo.exception.TajoRuntimeException; public class SQLErrorListener extends BaseErrorListener { @@ -36,6 +35,6 @@ public void syntaxError(Recognizer recognizer, String errorLine = lines[line - 1]; String simpleMessage = "syntax error at or near \"" + token.getText() + "\""; - throw new TajoRuntimeException(new SQLParseError(token, line, charPositionInLine, simpleMessage, errorLine)); + throw new SQLParseError(token, line, charPositionInLine, simpleMessage, errorLine); } } diff --git a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java index 78d445d25e..ef07cba643 100644 --- a/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java +++ b/tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLParseError.java @@ -21,13 +21,11 @@ import org.antlr.v4.runtime.Token; import org.apache.commons.lang.StringUtils; -import org.apache.tajo.error.Errors; -import org.apache.tajo.exception.TajoException; /** * Exception that represents a kind of SQL syntax error caused by the parser layer */ -public class SQLParseError extends TajoException { +public class SQLParseError extends RuntimeException { private String header; private String errorLine; private int charPositionInLine; @@ -39,7 +37,7 @@ public SQLParseError(Token offendingToken, int line, int charPositionInLine, String msg, String errorLine) { - super(Errors.ResultCode.SYNTAX_ERROR, msg); + super(msg); this.offendingToken = offendingToken; this.charPositionInLine = charPositionInLine; this.line = line;