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 <[email protected]> * Revert score and ignore failed IT Signed-off-by: Chen Dai <[email protected]> * Add comparison test to address PR comment Signed-off-by: Chen Dai <[email protected]> Signed-off-by: Chen Dai <[email protected]> (cherry picked from commit 2f4924a) Co-authored-by: Chen Dai <[email protected]>
opensearch-project · Jan 3, 2023 · 67f7f98 · 67f7f98
1 parent 9220e93
commit 67f7f98
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 71 deletions.
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
@@ -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
@@ -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
@@ -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
+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
diff --git a/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 b/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4
diff --git 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
@@ -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() {
-Original file line number
+Diff line change
@@ Expand Up @@
         );
       }
+      @Ignore("Handled by v2 engine which returns 'name': 'substring(lastname, 1, 2)' instead")
       @Test
       public void functionWithAliasShouldHaveAliasAsNameInSchema() {
         assertThat(
@@ Expand Down @@