From 7412e636c59469c7980fb77c1a441fa7a78dcde0 Mon Sep 17 00:00:00 2001 From: Dai Date: Wed, 6 Jan 2021 15:59:04 -0800 Subject: [PATCH 1/5] Change sql grammar and UT --- sql/src/main/antlr/OpenDistroSQLIdentifierParser.g4 | 1 - sql/src/main/antlr/OpenDistroSQLParser.g4 | 1 + .../sql/sql/antlr/SQLSyntaxParserTest.java | 3 +-- .../sql/sql/parser/AstBuilderTest.java | 6 ------ .../sql/sql/parser/AstExpressionBuilderTest.java | 4 ++++ .../sql/sql/parser/AstQualifiedNameBuilderTest.java | 1 - 6 files changed, 6 insertions(+), 10 deletions(-) diff --git a/sql/src/main/antlr/OpenDistroSQLIdentifierParser.g4 b/sql/src/main/antlr/OpenDistroSQLIdentifierParser.g4 index b1a1deac15..730d33cbe7 100644 --- a/sql/src/main/antlr/OpenDistroSQLIdentifierParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLIdentifierParser.g4 @@ -48,7 +48,6 @@ qualifiedName ident : DOT? ID - | DOUBLE_QUOTE_ID | BACKTICK_QUOTE_ID | keywordsCanBeId ; diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index cf4df2f4fe..6cdb1b1153 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -194,6 +194,7 @@ decimalLiteral stringLiteral : STRING_LITERAL + | DOUBLE_QUOTE_ID ; booleanLiteral 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 0b9a391d5e..0f2605d7d6 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 @@ -53,7 +53,7 @@ public void canParseSelectFieldWithAlias() { @Test public void canParseSelectFieldWithQuotedAlias() { - assertNotNull(parser.parse("SELECT name AS \"n\", age AS `a` FROM accounts")); + assertNotNull(parser.parse("SELECT name AS `n` FROM accounts")); } @Test @@ -76,7 +76,6 @@ public void canNotParseIndexNameWithSpecialChar() { @Test public void canParseIndexNameWithSpecialCharQuoted() { assertNotNull(parser.parse("SELECT * FROM `hello+world`")); - assertNotNull(parser.parse("SELECT * FROM \"hello$world\"")); } @Test diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java index 00057cc48c..b4c2eca9bd 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java @@ -137,11 +137,6 @@ public void can_build_select_fields_with_alias_quoted() { assertEquals( project( relation("test"), - alias( - "name", - qualifiedName("name"), - "first name" - ), alias( "(age + 10)", function("+", qualifiedName("age"), intLiteral(10)), @@ -149,7 +144,6 @@ public void can_build_select_fields_with_alias_quoted() { ) ), buildAST("SELECT" - + " name AS \"first name\", " + " (age + 10) AS `Age_Expr` " + "FROM test" ) diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java index c306625dbc..29e7db7d17 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java @@ -61,6 +61,10 @@ public void canBuildStringLiteral() { stringLiteral("hello"), buildExprAst("'hello'") ); + assertEquals( + stringLiteral("hello"), + buildExprAst("\"hello\"") + ); } @Test diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstQualifiedNameBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstQualifiedNameBuilderTest.java index 7f7d5cf48a..ebbfd0ee5f 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstQualifiedNameBuilderTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstQualifiedNameBuilderTest.java @@ -49,7 +49,6 @@ public void canBuildRegularIdentifierForElasticsearch() { @Test public void canBuildDelimitedIdentifier() { - buildFromIdentifier("\"hello$world\"").expectQualifiedName("hello$world"); buildFromIdentifier("`logs.2020.01`").expectQualifiedName("logs.2020.01"); } From 4207f44da1bbddd138731feabe308bb79f4777b2 Mon Sep 17 00:00:00 2001 From: Dai Date: Wed, 6 Jan 2021 16:24:39 -0800 Subject: [PATCH 2/5] Fix broken ITs --- .../amazon/opendistroforelasticsearch/sql/sql/IdentifierIT.java | 1 - integ-test/src/test/resources/correctness/queries/select.txt | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/IdentifierIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/IdentifierIT.java index 4a0f42e35c..5e8c30ec8d 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/IdentifierIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/IdentifierIT.java @@ -52,7 +52,6 @@ public void testSpecialIndexNames() throws IOException { public void testQuotedIndexNames() throws IOException { createIndexWithOneDoc("logs+2020+01", "logs.2020.01"); queryAndAssertTheDoc("SELECT * FROM `logs+2020+01`"); - queryAndAssertTheDoc("SELECT * FROM \"logs.2020.01\""); } @Test diff --git a/integ-test/src/test/resources/correctness/queries/select.txt b/integ-test/src/test/resources/correctness/queries/select.txt index b05d6f9175..b02330d0a7 100644 --- a/integ-test/src/test/resources/correctness/queries/select.txt +++ b/integ-test/src/test/resources/correctness/queries/select.txt @@ -4,7 +4,7 @@ SELECT `Cancelled`, `AvgTicketPrice` FROM kibana_sample_data_flights SELECT ABS(DistanceMiles), (FlightDelayMin * 2) - 3 FROM kibana_sample_data_flights SELECT abs(DistanceMiles), Abs(FlightDelayMin) FROM kibana_sample_data_flights SELECT Cancelled AS Cancel, AvgTicketPrice AS ATP FROM kibana_sample_data_flights -SELECT Cancelled AS `Cancel`, AvgTicketPrice AS "ATP" FROM kibana_sample_data_flights +SELECT Cancelled AS `Cancel` FROM kibana_sample_data_flights SELECT kibana_sample_data_flights.AvgTicketPrice FROM kibana_sample_data_flights SELECT flights.AvgTicketPrice, Carrier FROM kibana_sample_data_flights flights SELECT flights.AvgTicketPrice, Carrier FROM kibana_sample_data_flights AS flights From eaf7e7a36d41e85878539974a6b5aa32cca40d22 Mon Sep 17 00:00:00 2001 From: Dai Date: Wed, 6 Jan 2021 17:02:01 -0800 Subject: [PATCH 3/5] Change doctest --- docs/category.json | 1 + docs/user/general/datatypes.rst | 11 ++++++++++- docs/user/general/identifiers.rst | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/category.json b/docs/category.json index 2e7ec0419a..d570fdd449 100644 --- a/docs/category.json +++ b/docs/category.json @@ -22,6 +22,7 @@ ], "sql_cli": [ "user/dql/expressions.rst", + "user/general/datatypes.rst", "user/general/identifiers.rst", "user/general/values.rst", "user/dql/basics.rst", diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index 312db30464..d7b83ae6f1 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -222,7 +222,16 @@ Conversion from TIMESTAMP String Data Types ================= -TODO +A string is a sequence of characters enclosed in either single or double quotes. For example, both 'text' and "text" will be treated as string literal. To use quote characters in a string literal, you can include double quotes within single quoted string or single quotes within double quoted string:: + + od> SELECT 'hello', "world", '"hello"', "'world'" + fetched rows / total rows = 1/1 + +-----------+---------+-------------+-----------+ + | 'hello' | world | '"hello"' | 'world' | + |-----------+---------+-------------+-----------| + | hello | world | "hello" | 'world' | + +-----------+---------+-------------+-----------+ + diff --git a/docs/user/general/identifiers.rst b/docs/user/general/identifiers.rst index c3d2b70c66..a2c499f32c 100644 --- a/docs/user/general/identifiers.rst +++ b/docs/user/general/identifiers.rst @@ -56,7 +56,7 @@ Delimited Identifiers Description ----------- -A delimited identifier is an identifier enclosed in back ticks ````` or double quotation marks ``"``. In this case, the identifier enclosed is not necessarily a regular identifier. In other words, it can contain any special character not allowed by regular identifier. +A delimited identifier is an identifier enclosed in back ticks `````. In this case, the identifier enclosed is not necessarily a regular identifier. In other words, it can contain any special character not allowed by regular identifier. Please note the difference between single quote and double quotes in SQL syntax. Single quote is used to enclose a string literal while double quotes have same purpose as back ticks to escape special characters in an identifier. From 45edc2b790354cc652c84c283d459b9b53b49b2e Mon Sep 17 00:00:00 2001 From: Dai Date: Wed, 6 Jan 2021 17:12:39 -0800 Subject: [PATCH 4/5] Change PPL doc --- docs/experiment/ppl/general/datatypes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/experiment/ppl/general/datatypes.rst b/docs/experiment/ppl/general/datatypes.rst index 4682d61452..f5e0841bf4 100644 --- a/docs/experiment/ppl/general/datatypes.rst +++ b/docs/experiment/ppl/general/datatypes.rst @@ -222,5 +222,5 @@ Conversion from TIMESTAMP String Data Types ================= -TODO +A string is a sequence of characters enclosed in either single or double quotes. For example, both 'text' and "text" will be treated as string literal. From 354324ad09ae62a61cb2419a378a45d57160fabf Mon Sep 17 00:00:00 2001 From: Dai Date: Thu, 7 Jan 2021 10:52:10 -0800 Subject: [PATCH 5/5] Unquote identifier enclosed by back quotes only --- .../sql/common/utils/StringUtils.java | 4 ++-- docs/user/general/datatypes.rst | 10 +++++----- .../sql/sql/parser/AstBuilderTest.java | 3 ++- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/common/src/main/java/com/amazon/opendistroforelasticsearch/sql/common/utils/StringUtils.java b/common/src/main/java/com/amazon/opendistroforelasticsearch/sql/common/utils/StringUtils.java index 8b16e819ed..c6afb25b1c 100644 --- a/common/src/main/java/com/amazon/opendistroforelasticsearch/sql/common/utils/StringUtils.java +++ b/common/src/main/java/com/amazon/opendistroforelasticsearch/sql/common/utils/StringUtils.java @@ -49,13 +49,13 @@ public static String unquoteText(String text) { } /** - * Unquote Identifier which has " or ` as mark. + * Unquote Identifier which has ` as mark. * @param identifier identifier that possibly enclosed by double quotes or back ticks * @return An unquoted string whose outer pair of (double/back-tick) quotes have been * removed */ public static String unquoteIdentifier(String identifier) { - if (isQuoted(identifier, "\"") || isQuoted(identifier, "`")) { + if (isQuoted(identifier, "`")) { return identifier.substring(1, identifier.length() - 1); } else { return identifier; diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index d7b83ae6f1..8ad7be0a96 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -226,11 +226,11 @@ A string is a sequence of characters enclosed in either single or double quotes. od> SELECT 'hello', "world", '"hello"', "'world'" fetched rows / total rows = 1/1 - +-----------+---------+-------------+-----------+ - | 'hello' | world | '"hello"' | 'world' | - |-----------+---------+-------------+-----------| - | hello | world | "hello" | 'world' | - +-----------+---------+-------------+-----------+ + +-----------+-----------+-------------+-------------+ + | 'hello' | "world" | '"hello"' | "'world'" | + |-----------+-----------+-------------+-------------| + | hello | world | "hello" | 'world' | + +-----------+-----------+-------------+-------------+ diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java index b4c2eca9bd..cc272981a6 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java @@ -62,10 +62,11 @@ public void can_build_select_literals() { values(emptyList()), alias("123", intLiteral(123)), alias("'hello'", stringLiteral("hello")), + alias("\"world\"", stringLiteral("world")), alias("false", booleanLiteral(false)), alias("-4.567", doubleLiteral(-4.567)) ), - buildAST("SELECT 123, 'hello', false, -4.567") + buildAST("SELECT 123, 'hello', \"world\", false, -4.567") ); }