From 0211d7e9aee421495420083da6030735039a51ce Mon Sep 17 00:00:00 2001 From: Chen Dai <46505291+dai-chen@users.noreply.github.com> Date: Wed, 29 Jul 2020 09:06:42 -0700 Subject: [PATCH] Support queries end with semi colon (#609) * Change grammar and add UT * Add doc * Move IT --- docs/user/dql/basics.rst | 2 ++ integ-test/build.gradle | 1 + .../opendistroforelasticsearch/sql/legacy/QueryIT.java | 5 +++++ .../src/test/resources/doctest/templates/dql/basics.rst | 2 ++ legacy/src/main/antlr/OpenDistroSqlParser.g4 | 2 +- .../sql/legacy/query/ESActionFactory.java | 5 ++++- .../sql/legacy/antlr/SyntaxAnalysisTest.java | 7 ++++++- sql/src/main/antlr/OpenDistroSQLParser.g4 | 2 +- .../sql/sql/antlr/SQLSyntaxParserTest.java | 5 +++++ 9 files changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/user/dql/basics.rst b/docs/user/dql/basics.rst index 3b7d0c4746..082abe9aee 100644 --- a/docs/user/dql/basics.rst +++ b/docs/user/dql/basics.rst @@ -28,6 +28,8 @@ The syntax of ``SELECT`` statement is as follows:: [ORDER BY expression [IS [NOT] NULL] [ASC | DESC] [, ...]] [LIMIT [offset, ] size] +Although multiple query statements to execute in batch is not supported, ending with semicolon ``;`` is still allowed. For example, you can run ``SELECT * FROM accounts;`` without issue. This is useful to support queries generated by other tool, such as Microsoft Excel or BI tool. + Fundamentals ------------ diff --git a/integ-test/build.gradle b/integ-test/build.gradle index be53aebc0b..472eda4b82 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -137,6 +137,7 @@ task docTest(type: RestIntegTestTask) { include 'com/amazon/opendistroforelasticsearch/sql/doctest/**/*IT.class' exclude 'com/amazon/opendistroforelasticsearch/sql/correctness/**/*IT.class' exclude 'com/amazon/opendistroforelasticsearch/sql/ppl/**/*IT.class' + exclude 'com/amazon/opendistroforelasticsearch/sql/sql/**/*IT.class' exclude 'com/amazon/opendistroforelasticsearch/sql/legacy/**/*IT.class' } } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/QueryIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/QueryIT.java index f4a4c36a16..bdf5e09435 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/QueryIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/QueryIT.java @@ -90,6 +90,11 @@ protected void init() throws Exception { loadIndex(Index.BANK_WITH_NULL_VALUES); } + @Test + public void queryEndWithSemiColonTest() { + executeQuery(StringUtils.format("SELECT * FROM %s;", TEST_INDEX_BANK), "jdbc"); + } + @Test public void searchTypeTest() throws IOException { JSONObject response = executeQuery(String.format(Locale.ROOT, "SELECT * FROM %s LIMIT 1000", diff --git a/integ-test/src/test/resources/doctest/templates/dql/basics.rst b/integ-test/src/test/resources/doctest/templates/dql/basics.rst index 34cd5d3d09..4c8877d20a 100644 --- a/integ-test/src/test/resources/doctest/templates/dql/basics.rst +++ b/integ-test/src/test/resources/doctest/templates/dql/basics.rst @@ -28,6 +28,8 @@ The syntax of ``SELECT`` statement is as follows:: [ORDER BY expression [IS [NOT] NULL] [ASC | DESC] [, ...]] [LIMIT [offset, ] size] +Although multiple query statements to execute in batch is not supported, ending with semicolon ``;`` is still allowed. For example, you can run ``SELECT * FROM accounts;`` without issue. This is useful to support queries generated by other tool, such as Microsoft Excel or BI tool. + Fundamentals ------------ diff --git a/legacy/src/main/antlr/OpenDistroSqlParser.g4 b/legacy/src/main/antlr/OpenDistroSqlParser.g4 index ce30892e16..86c5c89d20 100644 --- a/legacy/src/main/antlr/OpenDistroSqlParser.g4 +++ b/legacy/src/main/antlr/OpenDistroSqlParser.g4 @@ -32,7 +32,7 @@ options { tokenVocab=OpenDistroSqlLexer; } // Root rule root - : sqlStatement? EOF + : sqlStatement? SEMI? EOF ; // Only SELECT, DELETE, SHOW and DSCRIBE are supported for now diff --git a/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/query/ESActionFactory.java b/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/query/ESActionFactory.java index ac05da3236..eeb3efe49a 100644 --- a/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/query/ESActionFactory.java +++ b/legacy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/query/ESActionFactory.java @@ -87,8 +87,11 @@ public static QueryAction create(Client client, String sql) public static QueryAction create(Client client, QueryActionRequest request) throws SqlParseException, SQLFeatureNotSupportedException { String sql = request.getSql(); - // Linebreak matcher + // Remove line breaker anywhere and semicolon at the end sql = sql.replaceAll("\\R", " ").trim(); + if (sql.endsWith(";")) { + sql = sql.substring(0, sql.length() - 1); + } switch (getFirstWord(sql)) { case "SELECT": diff --git a/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/antlr/SyntaxAnalysisTest.java b/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/antlr/SyntaxAnalysisTest.java index af7fceecd9..b15c5090cf 100644 --- a/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/antlr/SyntaxAnalysisTest.java +++ b/legacy/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/antlr/SyntaxAnalysisTest.java @@ -79,7 +79,7 @@ public void missingWhereKeywordShouldThrowException() { expectValidationFailWithErrorMessage( "SELECT * FROM accounts age = 1", "offending symbol [=]", // parser thought 'age' is alias of 'accounts' and failed at '=' - "Expecting", "'WHERE'" // "Expecting tokens in {, 'INNER', 'JOIN', ... 'WHERE', ','}" + "Expecting", ";" // "Expecting tokens in {, ';'}" ); } @@ -130,6 +130,11 @@ public void arithmeticExpressionInWhereClauseShouldPass() { validate("SELECT * FROM accounts WHERE age + 1 = 10"); } + @Test + public void queryEndWithSemiColonShouldPass() { + validate("SELECT * FROM accounts;"); + } + private void expectValidationFailWithErrorMessage(String query, String... messages) { exception.expect(SyntaxAnalysisException.class); exception.expectMessage(allOf(Arrays.stream(messages). diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index c9b3babc4e..49422c061c 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -34,7 +34,7 @@ options { tokenVocab=OpenDistroSQLLexer; } // Root rule root - : sqlStatement? EOF + : sqlStatement? SEMI? EOF ; // Only SELECT diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java index 6d3dee9585..94d668c4ba 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -26,6 +26,11 @@ class SQLSyntaxParserTest { private final SQLSyntaxParser parser = new SQLSyntaxParser(); + @Test + public void canParseQueryEndWithSemiColon() { + assertNotNull(parser.parse("SELECT 123;")); + } + @Test public void canParseSelectLiterals() { assertNotNull(parser.parse("SELECT 123, 'hello'"));