From 9cf1e90710f3264e0037a55b1471baf89971e046 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Wed, 2 Nov 2022 18:59:28 -0700 Subject: [PATCH] Add function name as identifier Signed-off-by: Peng Huo --- docs/user/dql/functions.rst | 68 +++++++++--------- docs/user/ppl/functions/datetime.rst | 70 +++++++++---------- .../sql/ppl/DateTimeFunctionIT.java | 10 +-- .../sql/sql/DateTimeFunctionIT.java | 10 +-- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 12 ++-- .../sql/ppl/parser/AstExpressionBuilder.java | 17 ++--- .../ppl/parser/AstExpressionBuilderTest.java | 54 +++++++++++--- .../ppl/parser/AstNowLikeFunctionTest.java | 10 +-- .../antlr/OpenSearchSQLIdentifierParser.g4 | 2 + sql/src/main/antlr/OpenSearchSQLParser.g4 | 8 +-- .../sql/sql/parser/AstExpressionBuilder.java | 17 ++--- .../sql/sql/parser/AstBuilderTest.java | 10 +-- .../parser/AstQualifiedNameBuilderTest.java | 25 +++++++ 13 files changed, 175 insertions(+), 138 deletions(-) diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 15f8a768bb..a04f01446e 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -987,17 +987,17 @@ CURRENT_DATE Description >>>>>>>>>>> -`CURRENT_DATE` and `CURRENT_DATE()` are synonyms for `CURDATE() <#curdate>`_. +`CURRENT_DATE()` are synonyms for `CURDATE() <#curdate>`_. Example:: - > SELECT CURRENT_DATE(), CURRENT_DATE; + > SELECT CURRENT_DATE(); fetched rows / total rows = 1/1 - +------------------+----------------+ - | CURRENT_DATE() | CURRENT_DATE | - |------------------+----------------| - | 2022-08-02 | 2022-08-02 | - +------------------+----------------+ + +------------------+ + | CURRENT_DATE() | + |------------------+ + | 2022-08-02 | + +------------------+ CURRENT_TIME @@ -1006,17 +1006,17 @@ CURRENT_TIME Description >>>>>>>>>>> -`CURRENT_TIME` and `CURRENT_TIME()` are synonyms for `CURTIME() <#curtime>`_. +`CURRENT_TIME()` are synonyms for `CURTIME() <#curtime>`_. Example:: - > SELECT CURRENT_TIME(), CURRENT_TIME; + > SELECT CURRENT_TIME(); fetched rows / total rows = 1/1 - +-----------------+----------------+ - | CURRENT_TIME() | CURRENT_TIME | - |-----------------+----------------| - | 15:39:05 | 15:39:05 | - +-----------------+----------------+ + +-----------------+ + | CURRENT_TIME() | + |-----------------+ + | 15:39:05 | + +-----------------+ CURRENT_TIMESTAMP @@ -1025,17 +1025,17 @@ CURRENT_TIMESTAMP Description >>>>>>>>>>> -`CURRENT_TIMESTAMP` and `CURRENT_TIMESTAMP()` are synonyms for `NOW() <#now>`_. +`CURRENT_TIMESTAMP()` are synonyms for `NOW() <#now>`_. Example:: - > SELECT CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP; + > SELECT CURRENT_TIMESTAMP(); fetched rows / total rows = 1/1 - +-----------------------+---------------------+ - | CURRENT_TIMESTAMP() | CURRENT_TIMESTAMP | - |-----------------------+---------------------| - | 2022-08-02 15:54:19 | 2022-08-02 15:54:19 | - +-----------------------+---------------------+ + +-----------------------+ + | CURRENT_TIMESTAMP() | + |-----------------------+ + | 2022-08-02 15:54:19 | + +-----------------------+ CURTIME @@ -1548,17 +1548,17 @@ LOCALTIMESTAMP Description >>>>>>>>>>> -`LOCALTIMESTAMP` and `LOCALTIMESTAMP()` are synonyms for `NOW() <#now>`_. +`LOCALTIMESTAMP()` are synonyms for `NOW() <#now>`_. Example:: - > SELECT LOCALTIMESTAMP(), LOCALTIMESTAMP; + > SELECT LOCALTIMESTAMP(); fetched rows / total rows = 1/1 - +---------------------+---------------------+ - | LOCALTIMESTAMP() | LOCALTIMESTAMP | - |---------------------+---------------------| - | 2022-08-02 15:54:19 | 2022-08-02 15:54:19 | - +---------------------+---------------------+ + +---------------------+ + | LOCALTIMESTAMP() | + |---------------------+ + | 2022-08-02 15:54:19 | + +---------------------+ LOCALTIME @@ -1567,17 +1567,17 @@ LOCALTIME Description >>>>>>>>>>> -`LOCALTIME` and `LOCALTIME()` are synonyms for `NOW() <#now>`_. +`LOCALTIME()` are synonyms for `NOW() <#now>`_. Example:: > SELECT LOCALTIME(), LOCALTIME; fetched rows / total rows = 1/1 - +---------------------+---------------------+ - | LOCALTIME() | LOCALTIME | - |---------------------+---------------------| - | 2022-08-02 15:54:19 | 2022-08-02 15:54:19 | - +---------------------+---------------------+ + +---------------------+ + | LOCALTIME() | + |---------------------+ + | 2022-08-02 15:54:19 | + +---------------------+ MAKEDATE diff --git a/docs/user/ppl/functions/datetime.rst b/docs/user/ppl/functions/datetime.rst index 7d783e914b..223b3c5557 100644 --- a/docs/user/ppl/functions/datetime.rst +++ b/docs/user/ppl/functions/datetime.rst @@ -202,17 +202,17 @@ CURRENT_DATE Description >>>>>>>>>>> -`CURRENT_DATE` and `CURRENT_DATE()` are synonyms for `CURDATE() <#curdate>`_. +`CURRENT_DATE()` are synonyms for `CURDATE() <#curdate>`_. Example:: - > source=people | eval `CURRENT_DATE()` = CURRENT_DATE(), `CURRENT_DATE` = CURRENT_DATE | fields `CURRENT_DATE()`, `CURRENT_DATE` + > source=people | eval `CURRENT_DATE()` = CURRENT_DATE() | fields `CURRENT_DATE()` fetched rows / total rows = 1/1 - +------------------+----------------+ - | CURRENT_DATE() | CURRENT_DATE | - |------------------+----------------| - | 2022-08-02 | 2022-08-02 | - +------------------+----------------+ + +------------------+ + | CURRENT_DATE() | + |------------------+ + | 2022-08-02 | + +------------------+ CURRENT_TIME @@ -221,17 +221,17 @@ CURRENT_TIME Description >>>>>>>>>>> -`CURRENT_TIME` and `CURRENT_TIME()` are synonyms for `CURTIME() <#curtime>`_. +`CURRENT_TIME()` are synonyms for `CURTIME() <#curtime>`_. Example:: - > source=people | eval `CURRENT_TIME()` = CURRENT_TIME(), `CURRENT_TIME` = CURRENT_TIME | fields `CURRENT_TIME()`, `CURRENT_TIME` + > source=people | eval `CURRENT_TIME()` = CURRENT_TIME() | fields `CURRENT_TIME()` fetched rows / total rows = 1/1 - +------------------+----------------+ - | CURRENT_TIME() | CURRENT_TIME | - |------------------+----------------| - | 15:39:05 | 15:39:05 | - +------------------+----------------+ + +------------------+ + | CURRENT_TIME() | + |------------------+ + | 15:39:05 | + +------------------+ CURRENT_TIMESTAMP @@ -240,17 +240,17 @@ CURRENT_TIMESTAMP Description >>>>>>>>>>> -`CURRENT_TIMESTAMP` and `CURRENT_TIMESTAMP()` are synonyms for `NOW() <#now>`_. +`CURRENT_TIMESTAMP()` are synonyms for `NOW() <#now>`_. Example:: - > source=people | eval `CURRENT_TIMESTAMP()` = CURRENT_TIMESTAMP(), `CURRENT_TIMESTAMP` = CURRENT_TIMESTAMP | fields `CURRENT_TIMESTAMP()`, `CURRENT_TIMESTAMP` + > source=people | eval `CURRENT_TIMESTAMP()` = CURRENT_TIMESTAMP() | fields `CURRENT_TIMESTAMP()` fetched rows / total rows = 1/1 - +-----------------------+---------------------+ - | CURRENT_TIMESTAMP() | CURRENT_TIMESTAMP | - |-----------------------+---------------------| - | 2022-08-02 15:54:19 | 2022-08-02 15:54:19 | - +-----------------------+---------------------+ + +-----------------------+ + | CURRENT_TIMESTAMP() | + |-----------------------+ + | 2022-08-02 15:54:19 | + +-----------------------+ CURTIME @@ -720,17 +720,17 @@ LOCALTIMESTAMP Description >>>>>>>>>>> -`LOCALTIMESTAMP` and `LOCALTIMESTAMP()` are synonyms for `NOW() <#now>`_. +`LOCALTIMESTAMP()` are synonyms for `NOW() <#now>`_. Example:: - > source=people | eval `LOCALTIMESTAMP()` = LOCALTIMESTAMP(), `LOCALTIMESTAMP` = LOCALTIMESTAMP | fields `LOCALTIMESTAMP()`, `LOCALTIMESTAMP` + > source=people | eval `LOCALTIMESTAMP()` = LOCALTIMESTAMP() | fields `LOCALTIMESTAMP()` fetched rows / total rows = 1/1 - +---------------------+---------------------+ - | LOCALTIMESTAMP() | LOCALTIMESTAMP | - |---------------------+---------------------| - | 2022-08-02 15:54:19 | 2022-08-02 15:54:19 | - +---------------------+---------------------+ + +---------------------+ + | LOCALTIMESTAMP() | + |---------------------+ + | 2022-08-02 15:54:19 | + +---------------------+ LOCALTIME @@ -739,17 +739,17 @@ LOCALTIME Description >>>>>>>>>>> -`LOCALTIME` and `LOCALTIME()` are synonyms for `NOW() <#now>`_. +`LOCALTIME()` are synonyms for `NOW() <#now>`_. Example:: - > source=people | eval `LOCALTIME()` = LOCALTIME(), `LOCALTIME` = LOCALTIME | fields `LOCALTIME()`, `LOCALTIME` + > source=people | eval `LOCALTIME()` = LOCALTIME() | fields `LOCALTIME()` fetched rows / total rows = 1/1 - +---------------------+---------------------+ - | LOCALTIME() | LOCALTIME | - |---------------------+---------------------| - | 2022-08-02 15:54:19 | 2022-08-02 15:54:19 | - +---------------------+---------------------+ + +---------------------+ + | LOCALTIME() | + |---------------------+ + | 2022-08-02 15:54:19 | + +---------------------+ MAKEDATE diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/DateTimeFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/DateTimeFunctionIT.java index 8c0661d6ff..afabc241fe 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/DateTimeFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/DateTimeFunctionIT.java @@ -686,7 +686,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "current_timestamp") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", true) .put("referenceGetter", (Supplier) LocalDateTime::now) .put("parser", (BiFunction) LocalDateTime::parse) @@ -695,7 +695,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "localtimestamp") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", true) .put("referenceGetter", (Supplier) LocalDateTime::now) .put("parser", (BiFunction) LocalDateTime::parse) @@ -704,7 +704,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "localtime") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", true) .put("referenceGetter", (Supplier) LocalDateTime::now) .put("parser", (BiFunction) LocalDateTime::parse) @@ -731,7 +731,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "current_time") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", false) .put("referenceGetter", (Supplier) LocalTime::now) .put("parser", (BiFunction) LocalTime::parse) @@ -749,7 +749,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "current_date") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", false) .put("referenceGetter", (Supplier) LocalDate::now) .put("parser", (BiFunction) LocalDate::parse) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/DateTimeFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/DateTimeFunctionIT.java index 15dee748b2..8c47966e52 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/DateTimeFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/DateTimeFunctionIT.java @@ -505,7 +505,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "current_timestamp") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", true) .put("referenceGetter", (Supplier) LocalDateTime::now) .put("parser", (BiFunction) LocalDateTime::parse) @@ -514,7 +514,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "localtimestamp") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", true) .put("referenceGetter", (Supplier) LocalDateTime::now) .put("parser", (BiFunction) LocalDateTime::parse) @@ -523,7 +523,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "localtime") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", true) .put("referenceGetter", (Supplier) LocalDateTime::now) .put("parser", (BiFunction) LocalDateTime::parse) @@ -550,7 +550,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "current_time") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", false) .put("referenceGetter", (Supplier) LocalTime::now) .put("parser", (BiFunction) LocalTime::parse) @@ -568,7 +568,7 @@ private List> nowLikeFunctionsData() { ImmutableMap.builder() .put("name", "current_date") .put("hasFsp", false) - .put("hasShortcut", true) + .put("hasShortcut", false) .put("constValue", false) .put("referenceGetter", (Supplier) LocalDate::now) .put("parser", (BiFunction) LocalDate::parse) diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 11e1c4c71f..99ff931f39 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -430,7 +430,7 @@ dateAndTimeFunctionBase // Functions which value could be cached in scope of a single query constantFunctionName - : datetimeConstantLiteral + : CURRENT_DATE | CURRENT_TIME | CURRENT_TIMESTAMP | LOCALTIME | LOCALTIMESTAMP | UTC_TIMESTAMP | UTC_DATE | UTC_TIME | CURDATE | CURTIME | NOW ; @@ -507,7 +507,6 @@ datetimeLiteral : dateLiteral | timeLiteral | timestampLiteral - | datetimeConstantLiteral ; dateLiteral @@ -522,11 +521,6 @@ timestampLiteral : TIMESTAMP timestamp=stringLiteral ; -// Actually, these constants are shortcuts to the corresponding functions -datetimeConstantLiteral - : CURRENT_DATE | CURRENT_TIME | CURRENT_TIMESTAMP | LOCALTIME | LOCALTIMESTAMP | UTC_TIMESTAMP | UTC_DATE | UTC_TIME - ; - intervalUnit : MICROSECOND | SECOND | MINUTE | HOUR | DAY | WEEK | MONTH | QUARTER | YEAR | SECOND_MICROSECOND | MINUTE_MICROSECOND | MINUTE_SECOND | HOUR_MICROSECOND | HOUR_SECOND | HOUR_MINUTE | DAY_MICROSECOND @@ -571,4 +565,8 @@ keywordsCanBeId | TIMESTAMP | DATE | TIME | FIRST | LAST | timespanUnit | SPAN + | constantFunctionName + | dateAndTimeFunctionBase + | textFunctionBase + | mathematicalFunctionBase ; diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index d2e6131cbe..4430820081 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -18,7 +18,6 @@ import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ConvertedDataTypeContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.CountAllFunctionCallContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DataTypeFunctionCallContext; -import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DatetimeConstantLiteralContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DecimalLiteralContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DistinctCountFunctionCallContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.EvalClauseContext; @@ -258,11 +257,6 @@ public UnresolvedExpression visitConvertedDataType(ConvertedDataTypeContext ctx) return AstDSL.stringLiteral(ctx.getText()); } - @Override - public UnresolvedExpression visitDatetimeConstantLiteral(DatetimeConstantLiteralContext ctx) { - return visitConstantFunction(ctx.getText(), null); - } - public UnresolvedExpression visitConstantFunction(ConstantFunctionContext ctx) { return visitConstantFunction(ctx.constantFunctionName().getText(), ctx.functionArgs()); @@ -270,13 +264,10 @@ public UnresolvedExpression visitConstantFunction(ConstantFunctionContext ctx) { private UnresolvedExpression visitConstantFunction(String functionName, FunctionArgsContext args) { - return new ConstantFunction(functionName, - args == null - ? Collections.emptyList() - : args.functionArg() - .stream() - .map(this::visitFunctionArg) - .collect(Collectors.toList())); + return new ConstantFunction(functionName, args.functionArg() + .stream() + .map(this::visitFunctionArg) + .collect(Collectors.toList())); } private Function visitFunction(String functionName, FunctionArgsContext args) { diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java index 565157dd06..e4048c5fe1 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java @@ -7,7 +7,7 @@ package org.opensearch.sql.ppl.parser; import static java.util.Collections.emptyList; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.opensearch.sql.ast.dsl.AstDSL.agg; import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; import static org.opensearch.sql.ast.dsl.AstDSL.alias; @@ -45,16 +45,14 @@ import com.google.common.collect.ImmutableMap; import java.util.Arrays; -import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.stream.Collectors; import org.junit.Ignore; import org.junit.Test; -import org.opensearch.sql.ast.Node; import org.opensearch.sql.ast.expression.AllFields; -import org.opensearch.sql.ast.expression.Argument; import org.opensearch.sql.ast.expression.DataType; -import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.RelevanceFieldList; -import org.opensearch.sql.ppl.antlr.PPLSyntaxParser; public class AstExpressionBuilderTest extends AstBuilderTest { @@ -756,8 +754,46 @@ public void canBuildQuery_stringRelevanceFunctionWithArguments() { ); } - private Node buildExprAst(String query) { - AstBuilder astBuilder = new AstBuilder(new AstExpressionBuilder(), query); - return astBuilder.visit(new PPLSyntaxParser().parse(query)); + @Test + public void functionNameCanBeUsedAsIdentifier() { + assertFunctionNameCouldBeId( + "AVG | COUNT | SUM | MIN | MAX | VAR_SAMP | VAR_POP | STDDEV_SAMP | STDDEV_POP"); + assertFunctionNameCouldBeId( + "CURRENT_DATE | CURRENT_TIME | CURRENT_TIMESTAMP | LOCALTIME | LOCALTIMESTAMP | " + + "UTC_TIMESTAMP | UTC_DATE | UTC_TIME | CURDATE | CURTIME | NOW"); + assertFunctionNameCouldBeId( + "ADDDATE | CONVERT_TZ | DATE | DATE_ADD | DATE_FORMAT | DATE_SUB " + + "| DATETIME | DAY | DAYNAME | DAYOFMONTH " + + "| DAYOFWEEK | DAYOFYEAR | FROM_DAYS | FROM_UNIXTIME | HOUR | MAKEDATE | MAKETIME " + + "| MICROSECOND | MINUTE | MONTH | MONTHNAME " + + "| PERIOD_ADD | PERIOD_DIFF | QUARTER | SECOND | SUBDATE | SYSDATE | TIME " + + "| TIME_TO_SEC | TIMESTAMP | TO_DAYS | UNIX_TIMESTAMP | WEEK | YEAR"); + assertFunctionNameCouldBeId( + "SUBSTR | SUBSTRING | TRIM | LTRIM | RTRIM | LOWER | UPPER | CONCAT | CONCAT_WS | LENGTH " + + "| STRCMP | RIGHT | LEFT | ASCII | LOCATE | REPLACE" + ); + assertFunctionNameCouldBeId( + "ABS | CEIL | CEILING | CONV | CRC32 | E | EXP | FLOOR | LN | LOG" + + " | LOG10 | LOG2 | MOD | PI |POW | POWER | RAND | ROUND | SIGN | SQRT | TRUNCATE " + + "| ACOS | ASIN | ATAN | ATAN2 | COS | COT | DEGREES | RADIANS | SIN | TAN"); + } + + void assertFunctionNameCouldBeId(String antlrFunctionName) { + List functionList = + Arrays.stream(antlrFunctionName.split("\\|")).map(String::stripLeading) + .map(String::stripTrailing).collect( + Collectors.toList()); + + assertFalse(functionList.isEmpty()); + for (String functionName : functionList) { + assertEqual(String.format(Locale.ROOT, "source=t | fields %s", functionName), + projectWithArg( + relation("t"), + defaultFieldsArgs(), + field( + qualifiedName(functionName) + ) + )); + } } } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstNowLikeFunctionTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstNowLikeFunctionTest.java index 6c6233a17f..1350305391 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstNowLikeFunctionTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstNowLikeFunctionTest.java @@ -52,14 +52,14 @@ public AstNowLikeFunctionTest(String name, Boolean hasFsp, Boolean hasShortcut, public static Iterable functionNames() { return List.of(new Object[][]{ {"now", false, false, true}, - {"current_timestamp", false, true, true}, - {"localtimestamp", false, true, true}, - {"localtime", false, true, true}, + {"current_timestamp", false, false, true}, + {"localtimestamp", false, false, true}, + {"localtime", false, false, true}, {"sysdate", true, false, false}, {"curtime", false, false, true}, - {"current_time", false, true, true}, + {"current_time", false, false, true}, {"curdate", false, false, true}, - {"current_date", false, true, true} + {"current_date", false, false, true} }); } diff --git a/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 b/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 index 665d48c97c..cd65e5066c 100644 --- a/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLIdentifierParser.g4 @@ -63,4 +63,6 @@ keywordsCanBeId | 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 6470c9f546..4dedaaf396 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -225,7 +225,6 @@ datetimeLiteral : dateLiteral | timeLiteral | timestampLiteral - | datetimeConstantLiteral ; dateLiteral @@ -240,11 +239,6 @@ timestampLiteral : TIMESTAMP timestamp=stringLiteral ; -// Actually, these constants are shortcuts to the corresponding functions -datetimeConstantLiteral - : CURRENT_DATE | CURRENT_TIME | CURRENT_TIMESTAMP | LOCALTIME | LOCALTIMESTAMP | UTC_TIMESTAMP | UTC_DATE | UTC_TIME - ; - intervalLiteral : INTERVAL expression intervalUnit ; @@ -404,7 +398,7 @@ dateTimeFunctionName // Functions which value could be cached in scope of a single query constantFunctionName - : datetimeConstantLiteral + : CURRENT_DATE | CURRENT_TIME | CURRENT_TIMESTAMP | LOCALTIME | LOCALTIMESTAMP | UTC_TIMESTAMP | UTC_DATE | UTC_TIME | CURDATE | CURTIME | NOW ; diff --git a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java index ebfafeec23..0d551e3f38 100644 --- a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java +++ b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java @@ -23,7 +23,6 @@ import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.CountStarFunctionCallContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.DataTypeFunctionCallContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.DateLiteralContext; -import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.DatetimeConstantLiteralContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.DistinctCountFunctionCallContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.IsNullPredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.LikePredicateContext; @@ -407,11 +406,6 @@ private Function visitFunction(String functionName, FunctionArgsContext args) { ); } - @Override - public UnresolvedExpression visitDatetimeConstantLiteral(DatetimeConstantLiteralContext ctx) { - return visitConstantFunction(ctx.getText(), null); - } - @Override public UnresolvedExpression visitConstantFunction(ConstantFunctionContext ctx) { return visitConstantFunction(ctx.constantFunctionName().getText(), @@ -420,13 +414,10 @@ public UnresolvedExpression visitConstantFunction(ConstantFunctionContext ctx) { private UnresolvedExpression visitConstantFunction(String functionName, FunctionArgsContext args) { - return new ConstantFunction(functionName, - args == null - ? Collections.emptyList() - : args.functionArg() - .stream() - .map(this::visitFunctionArg) - .collect(Collectors.toList())); + return new ConstantFunction(functionName, args.functionArg() + .stream() + .map(this::visitFunctionArg) + .collect(Collectors.toList())); } private QualifiedName visitIdentifiers(List identifiers) { diff --git a/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java b/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java index 0c06754261..a955399c4d 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java @@ -682,14 +682,14 @@ public void can_build_limit_clause_with_offset() { private static Stream nowLikeFunctionsData() { return Stream.of( Arguments.of("now", false, false, true), - Arguments.of("current_timestamp", false, true, true), - Arguments.of("localtimestamp", false, true, true), - Arguments.of("localtime", false, true, true), + Arguments.of("current_timestamp", false, false, true), + Arguments.of("localtimestamp", false, false, true), + Arguments.of("localtime", false, false, true), Arguments.of("sysdate", true, false, false), Arguments.of("curtime", false, false, true), - Arguments.of("current_time", false, true, true), + Arguments.of("current_time", false, false, true), Arguments.of("curdate", false, false, true), - Arguments.of("current_date", false, true, true) + Arguments.of("current_date", false, false, true) ); } 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 fdd4f2f58c..92b535144f 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 @@ -7,8 +7,12 @@ package org.opensearch.sql.sql.parser; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import java.util.Arrays; +import java.util.List; import java.util.function.Function; +import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.tree.RuleNode; @@ -47,6 +51,27 @@ public void canBuildQualifiedIdentifier() { buildFromQualifiers("account.location.city").expectQualifiedName("account", "location", "city"); } + + @Test + public void functionNameCanBeUsedAsIdentifier() { + assertFunctionNameCouldBeId("AVG | COUNT | SUM | MIN | MAX"); + assertFunctionNameCouldBeId( + "CURRENT_DATE | CURRENT_TIME | CURRENT_TIMESTAMP | LOCALTIME | LOCALTIMESTAMP |" + + " UTC_TIMESTAMP | UTC_DATE | UTC_TIME | CURDATE | CURTIME | NOW"); + } + + void assertFunctionNameCouldBeId(String antlrFunctionName) { + List functionList = + Arrays.stream(antlrFunctionName.split("\\|")).map(String::stripLeading) + .map(String::stripTrailing).collect( + Collectors.toList()); + + assertFalse(functionList.isEmpty()); + for (String functionName : functionList) { + buildFromQualifiers(functionName).expectQualifiedName(functionName); + } + } + private AstExpressionBuilderAssertion buildFromIdentifier(String expr) { return new AstExpressionBuilderAssertion(OpenSearchSQLParser::ident, expr); }