From 67f7f983d7708d9e0bc61a33db97a0af0e41a2cc Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 3 Jan 2023 11:37:17 -0800 Subject: [PATCH] Allow common keywords and scalar function name used as identifier (#1191) (#1212) * Allow score, type and scalar function name as identifier Signed-off-by: Chen Dai * Revert score and ignore failed IT Signed-off-by: Chen Dai * Add comparison test to address PR comment Signed-off-by: Chen Dai Signed-off-by: Chen Dai (cherry picked from commit 2f4924a9b0a8a120f092c23c303cd908504b1f46) Co-authored-by: Chen Dai --- .../sql/legacy/AggregationExpressionIT.java | 1 + .../org/opensearch/sql/legacy/JdbcTestIT.java | 1 + .../correctness/queries/aggregation.txt | 3 +- .../antlr/OpenSearchSQLIdentifierParser.g4 | 68 ------------------- sql/src/main/antlr/OpenSearchSQLParser.g4 | 36 +++++++++- .../parser/AstQualifiedNameBuilderTest.java | 4 ++ 6 files changed, 42 insertions(+), 71 deletions(-) delete mode 100644 sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java index d4374dcbaf..854a33fc91 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationExpressionIT.java @@ -121,6 +121,7 @@ public void hasGroupKeyMaxAddLiteralShouldPass() { rows("f", 1)); } + @Ignore("Handled by v2 engine which returns 'name': 'Log(MAX(age) + MIN(age))' instead") @Test public void noGroupKeyLogMaxAddMinShouldPass() { JSONObject response = executeJdbcRequest(String.format( diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java index 74f999f368..e3a0cbd89d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java @@ -165,6 +165,7 @@ public void functionWithoutAliasShouldHaveEntireFunctionAsNameInSchema() { ); } + @Ignore("Handled by v2 engine which returns 'name': 'substring(lastname, 1, 2)' instead") @Test public void functionWithAliasShouldHaveAliasAsNameInSchema() { assertThat( diff --git a/integ-test/src/test/resources/correctness/queries/aggregation.txt b/integ-test/src/test/resources/correctness/queries/aggregation.txt index 0c0648a937..9b610cb885 100644 --- a/integ-test/src/test/resources/correctness/queries/aggregation.txt +++ b/integ-test/src/test/resources/correctness/queries/aggregation.txt @@ -11,4 +11,5 @@ SELECT VAR_SAMP(AvgTicketPrice) FROM opensearch_dashboards_sample_data_flights SELECT STDDEV_POP(AvgTicketPrice) FROM opensearch_dashboards_sample_data_flights SELECT STDDEV_SAMP(AvgTicketPrice) FROM opensearch_dashboards_sample_data_flights SELECT COUNT(DISTINCT Origin), COUNT(DISTINCT Dest) FROM opensearch_dashboards_sample_data_flights -SELECT COUNT(DISTINCT Origin) FROM (SELECT * FROM opensearch_dashboards_sample_data_flights) AS flights \ No newline at end of file +SELECT COUNT(DISTINCT Origin) FROM (SELECT * FROM opensearch_dashboards_sample_data_flights) AS flights +SELECT LOG(MAX(AvgTicketPrice) + MIN(AvgTicketPrice)) FROM opensearch_dashboards_sample_data_flights \ No newline at end of file diff --git a/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 b/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 deleted file mode 100644 index cd65e5066c..0000000000 --- a/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -/* -MySQL (Positive Technologies) grammar -The MIT License (MIT). -Copyright (c) 2015-2017, Ivan Kochurkin (kvanttt@gmail.com), Positive Technologies. -Copyright (c) 2017, Ivan Khudyashev (IHudyashov@ptsecurity.com) - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in -all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -THE SOFTWARE. -*/ - -parser grammar OpenSearchSQLIdentifierParser; - -options { tokenVocab=OpenSearchSQLLexer; } - - -// Identifiers - -tableName - : qualifiedName - ; - -columnName - : qualifiedName - ; - -alias - : ident - ; - -qualifiedName - : ident (DOT ident)* - ; - -ident - : DOT? ID - | BACKTICK_QUOTE_ID - | keywordsCanBeId - ; - -keywordsCanBeId - : FULL - | FIELD | D | T | TS // OD SQL and ODBC special - | COUNT | SUM | AVG | MAX | MIN - | TIMESTAMP | DATE | TIME | DAYOFWEEK - | FIRST | LAST - | CURRENT_DATE | CURRENT_TIME | CURRENT_TIMESTAMP | LOCALTIME | LOCALTIMESTAMP | UTC_TIMESTAMP | UTC_DATE | UTC_TIME - | CURDATE | CURTIME | NOW - ; diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index f2bf58ca60..859d96c505 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -30,8 +30,6 @@ THE SOFTWARE. parser grammar OpenSearchSQLParser; -import OpenSearchSQLIdentifierParser; - options { tokenVocab=OpenSearchSQLLexer; } @@ -562,3 +560,37 @@ alternateMultiMatchField | argName=alternateMultiMatchArgName EQUAL_SYMBOL LT_SQR_PRTHS argVal=relevanceArgValue RT_SQR_PRTHS ; + + +// Identifiers + +tableName + : qualifiedName + ; + +columnName + : qualifiedName + ; + +alias + : ident + ; + +qualifiedName + : ident (DOT ident)* + ; + +ident + : DOT? ID + | BACKTICK_QUOTE_ID + | keywordsCanBeId + | scalarFunctionName + ; + +keywordsCanBeId + : FULL + | FIELD | D | T | TS // OD SQL and ODBC special + | COUNT | SUM | AVG | MAX | MIN + | FIRST | LAST + | TYPE // TODO: Type is keyword required by relevancy function. Remove this when relevancy functions moved out + ; diff --git a/sql/src/test/java/org/opensearch/sql/sql/parser/AstQualifiedNameBuilderTest.java b/sql/src/test/java/org/opensearch/sql/sql/parser/AstQualifiedNameBuilderTest.java index 92b535144f..28665dd7ef 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/parser/AstQualifiedNameBuilderTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/parser/AstQualifiedNameBuilderTest.java @@ -51,6 +51,10 @@ public void canBuildQualifiedIdentifier() { buildFromQualifiers("account.location.city").expectQualifiedName("account", "location", "city"); } + @Test + public void commonKeywordCanBeUsedAsIdentifier() { + buildFromIdentifier("type").expectQualifiedName("type"); + } @Test public void functionNameCanBeUsedAsIdentifier() {