diff --git a/build.gradle b/build.gradle index 28ecdc9dbe2..8a2f9046bfc 100644 --- a/build.gradle +++ b/build.gradle @@ -7,6 +7,8 @@ buildscript { ext { opensearch_version = System.getProperty("opensearch.version", "2.2.0-SNAPSHOT") + spring_version = "5.3.22" + jackson_version = "2.13.3" isSnapshot = "true" == System.getProperty("build.snapshot", "true") buildVersionQualifier = System.getProperty("build.version_qualifier", "") version_tokens = opensearch_version.tokenize('-') diff --git a/common/src/main/java/org/opensearch/sql/common/utils/LogUtils.java b/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java similarity index 53% rename from common/src/main/java/org/opensearch/sql/common/utils/LogUtils.java rename to common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java index 2f8c22c059b..b3991230545 100644 --- a/common/src/main/java/org/opensearch/sql/common/utils/LogUtils.java +++ b/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java @@ -6,20 +6,29 @@ package org.opensearch.sql.common.utils; +import java.time.LocalDateTime; import java.util.Map; +import java.util.Optional; import java.util.UUID; import org.apache.logging.log4j.ThreadContext; /** - * Utility class for generating/accessing the request id from logging context. + * Utility class for recording and accessing context for the query being executed. + * Implementation Details: context variables is being persisted statically in the thread context + * @see: @ThreadContext */ -public class LogUtils { +public class QueryContext { /** * The key of the request id in the context map. */ private static final String REQUEST_ID_KEY = "request_id"; + /** + * Timestamp when SQL plugin started to process current request. + */ + private static final String REQUEST_PROCESSING_STARTED = "request_processing_started"; + /** * Generates a random UUID and adds to the {@link ThreadContext} as the request id. *

@@ -29,8 +38,10 @@ public class LogUtils { * call this method twice on the same thread within the lifetime of the request. *

*/ - public static void addRequestId() { - ThreadContext.put(REQUEST_ID_KEY, UUID.randomUUID().toString()); + public static String addRequestId() { + var id = UUID.randomUUID().toString(); + ThreadContext.put(REQUEST_ID_KEY, id); + return id; } /** @@ -38,8 +49,27 @@ public static void addRequestId() { * @return the current request id from {@link ThreadContext}. */ public static String getRequestId() { - final String requestId = ThreadContext.get(REQUEST_ID_KEY); - return requestId; + var id = ThreadContext.get(REQUEST_ID_KEY); + if (null == id) { + id = addRequestId(); + } + return id; + } + + public static void recordProcessingStarted() { + ThreadContext.put(REQUEST_PROCESSING_STARTED, LocalDateTime.now().toString()); + } + + /** + * Get recorded previously time indicating when processing started for the current query. + * @return A LocalDateTime object + */ + public static LocalDateTime getProcessingStartedTime() { + if (ThreadContext.containsKey(REQUEST_PROCESSING_STARTED)) { + return LocalDateTime.parse(ThreadContext.get(REQUEST_PROCESSING_STARTED)); + } + // This shouldn't happen outside of unit tests + return LocalDateTime.now(); } /** @@ -57,7 +87,7 @@ public static Runnable withCurrentContext(final Runnable task) { }; } - private LogUtils() { + private QueryContext() { throw new AssertionError( getClass().getCanonicalName() + " is a utility class and must not be initialized"); } diff --git a/core/build.gradle b/core/build.gradle index 342d5673cd7..1fa3e19e269 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -40,8 +40,8 @@ repositories { dependencies { api group: 'com.google.guava', name: 'guava', version: '31.0.1-jre' - api group: 'org.springframework', name: 'spring-context', version: '5.3.22' - api group: 'org.springframework', name: 'spring-beans', version: '5.3.22' + api group: 'org.springframework', name: 'spring-context', version: "${spring_version}" + api group: 'org.springframework', name: 'spring-beans', version: "${spring_version}" api group: 'org.apache.commons', name: 'commons-lang3', version: '3.10' api group: 'com.facebook.presto', name: 'presto-matching', version: '0.240' api group: 'org.apache.commons', name: 'commons-math3', version: '3.6.1' @@ -49,7 +49,7 @@ dependencies { testImplementation('org.junit.jupiter:junit-jupiter:5.6.2') testImplementation group: 'org.hamcrest', name: 'hamcrest-library', version: '2.1' - testImplementation group: 'org.springframework', name: 'spring-test', version: '5.3.22' + testImplementation group: 'org.springframework', name: 'spring-test', version: "${spring_version}" testImplementation group: 'org.mockito', name: 'mockito-core', version: '3.12.4' testImplementation group: 'org.mockito', name: 'mockito-junit-jupiter', version: '3.12.4' } diff --git a/core/src/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java index bd2d0756139..a094d2e4873 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -682,6 +682,42 @@ public FunctionExpression match_bool_prefix(Expression... args) { return compile(BuiltinFunctionName.MATCH_BOOL_PREFIX, args); } + public FunctionExpression now(Expression... args) { + return compile(BuiltinFunctionName.NOW, args); + } + + public FunctionExpression current_timestamp(Expression... args) { + return compile(BuiltinFunctionName.CURRENT_TIMESTAMP, args); + } + + public FunctionExpression localtimestamp(Expression... args) { + return compile(BuiltinFunctionName.LOCALTIMESTAMP, args); + } + + public FunctionExpression localtime(Expression... args) { + return compile(BuiltinFunctionName.LOCALTIME, args); + } + + public FunctionExpression sysdate(Expression... args) { + return compile(BuiltinFunctionName.SYSDATE, args); + } + + public FunctionExpression curtime(Expression... args) { + return compile(BuiltinFunctionName.CURTIME, args); + } + + public FunctionExpression current_time(Expression... args) { + return compile(BuiltinFunctionName.CURRENT_TIME, args); + } + + public FunctionExpression curdate(Expression... args) { + return compile(BuiltinFunctionName.CURDATE, args); + } + + public FunctionExpression current_date(Expression... args) { + return compile(BuiltinFunctionName.CURRENT_DATE, args); + } + private FunctionExpression compile(BuiltinFunctionName bfn, Expression... args) { return (FunctionExpression) repository.compile(bfn.getName(), Arrays.asList(args.clone())); } diff --git a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java index c4de0e13ad9..46061e7ec52 100644 --- a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java @@ -18,11 +18,17 @@ import static org.opensearch.sql.expression.function.FunctionDSL.impl; import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling; +import java.math.BigDecimal; +import java.math.RoundingMode; import java.time.LocalDate; +import java.time.LocalDateTime; import java.time.format.TextStyle; import java.util.Locale; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; +import javax.annotation.Nullable; import lombok.experimental.UtilityClass; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.data.model.ExprDateValue; import org.opensearch.sql.data.model.ExprDatetimeValue; import org.opensearch.sql.data.model.ExprIntegerValue; @@ -78,6 +84,89 @@ public void register(BuiltinFunctionRepository repository) { repository.register(to_days()); repository.register(week()); repository.register(year()); + + repository.register(now()); + repository.register(current_timestamp()); + repository.register(localtimestamp()); + repository.register(localtime()); + repository.register(sysdate()); + repository.register(curtime()); + repository.register(current_time()); + repository.register(curdate()); + repository.register(current_date()); + } + + /** + * NOW() returns a constant time that indicates the time at which the statement began to execute. + */ + private LocalDateTime now(@Nullable Integer fsp) { + return formatLocalDateTime(QueryContext::getProcessingStartedTime, fsp); + } + + private FunctionResolver now(FunctionName functionName) { + return define(functionName, + impl(() -> new ExprDatetimeValue(now((Integer)null)), DATETIME), + impl((v) -> new ExprDatetimeValue(now(v.integerValue())), DATETIME, INTEGER) + ); + } + + private FunctionResolver now() { + return now(BuiltinFunctionName.NOW.getName()); + } + + private FunctionResolver current_timestamp() { + return now(BuiltinFunctionName.CURRENT_TIMESTAMP.getName()); + } + + private FunctionResolver localtimestamp() { + return now(BuiltinFunctionName.LOCALTIMESTAMP.getName()); + } + + private FunctionResolver localtime() { + return now(BuiltinFunctionName.LOCALTIME.getName()); + } + + /** + * SYSDATE() returns the time at which it executes. + */ + private LocalDateTime sysDate(@Nullable Integer fsp) { + return formatLocalDateTime(LocalDateTime::now, fsp); + } + + private FunctionResolver sysdate() { + return define(BuiltinFunctionName.SYSDATE.getName(), + impl(() -> new ExprDatetimeValue(sysDate(null)), DATETIME), + impl((v) -> new ExprDatetimeValue(sysDate(v.integerValue())), DATETIME, INTEGER) + ); + } + + private FunctionResolver curtime(FunctionName functionName) { + return define(functionName, + impl(() -> new ExprTimeValue(sysDate(null).toLocalTime()), TIME), + impl((v) -> new ExprTimeValue(sysDate(v.integerValue()).toLocalTime()), TIME, INTEGER) + ); + } + + private FunctionResolver curtime() { + return curtime(BuiltinFunctionName.CURTIME.getName()); + } + + private FunctionResolver current_time() { + return curtime(BuiltinFunctionName.CURRENT_TIME.getName()); + } + + private FunctionResolver curdate(FunctionName functionName) { + return define(functionName, + impl(() -> new ExprDateValue(sysDate(null).toLocalDate()), DATE) + ); + } + + private FunctionResolver curdate() { + return curdate(BuiltinFunctionName.CURDATE.getName()); + } + + private FunctionResolver current_date() { + return curdate(BuiltinFunctionName.CURRENT_DATE.getName()); } /** @@ -108,6 +197,10 @@ private FunctionResolver adddate() { return add_date(BuiltinFunctionName.ADDDATE.getName()); } + private FunctionResolver date_add() { + return add_date(BuiltinFunctionName.DATE_ADD.getName()); + } + /** * Extracts the date part of a date and time value. * Also to construct a date type. The supported signatures: @@ -121,10 +214,6 @@ private FunctionResolver date() { impl(nullMissingHandling(DateTimeFunction::exprDate), DATE, TIMESTAMP)); } - private FunctionResolver date_add() { - return add_date(BuiltinFunctionName.DATE_ADD.getName()); - } - /** * Specify a start date and subtract a temporal amount to the date. * The return type depends on the date type and the interval unit. Detailed supported signatures: @@ -679,4 +768,26 @@ private ExprValue exprYear(ExprValue date) { return new ExprIntegerValue(date.dateValue().getYear()); } + /** + * Prepare LocalDateTime value. + * @param supplier A function which returns LocalDateTime to format. + * @param fsp argument is given to specify a fractional seconds precision from 0 to 6, + * the return value includes a fractional seconds part of that many digits. + * @return LocalDateTime object. + */ + private LocalDateTime formatLocalDateTime(Supplier supplier, + @Nullable Integer fsp) { + var res = supplier.get(); + if (fsp == null) { + return res; + } + var defaultPrecision = 9; // There are 10^9 nanoseconds in one second + if (fsp < 0 || fsp > 6) { // Check that the argument is in the allowed range [0, 6] + throw new IllegalArgumentException( + String.format("Invalid `fsp` value: %d, allowed 0 to 6", fsp)); + } + var nano = new BigDecimal(res.getNano()) + .setScale(fsp - defaultPrecision, RoundingMode.DOWN).intValue(); + return res.withNano(nano); + } } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java index cd343284530..e4c3a8a9a0f 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java @@ -82,7 +82,16 @@ public enum BuiltinFunctionName { TO_DAYS(FunctionName.of("to_days")), WEEK(FunctionName.of("week")), YEAR(FunctionName.of("year")), - + // `now`-like functions + NOW(FunctionName.of("now")), + CURDATE(FunctionName.of("curdate")), + CURRENT_DATE(FunctionName.of("current_date")), + CURTIME(FunctionName.of("curtime")), + CURRENT_TIME(FunctionName.of("current_time")), + LOCALTIME(FunctionName.of("localtime")), + CURRENT_TIMESTAMP(FunctionName.of("current_timestamp")), + LOCALTIMESTAMP(FunctionName.of("localtimestamp")), + SYSDATE(FunctionName.of("sysdate")), /** * Text Functions. */ diff --git a/core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanDSL.java b/core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanDSL.java index 4185d55c559..cdd3d3a103b 100644 --- a/core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanDSL.java +++ b/core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanDSL.java @@ -18,7 +18,6 @@ import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.LiteralExpression; import org.opensearch.sql.expression.NamedExpression; -import org.opensearch.sql.expression.ParseExpression; import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.aggregation.NamedAggregator; import org.opensearch.sql.expression.window.WindowDefinition; diff --git a/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java index 72db4025522..e11b1484dd5 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java @@ -27,8 +27,13 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.Function; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.opensearch.sql.analysis.symbol.Namespace; import org.opensearch.sql.analysis.symbol.Symbol; import org.opensearch.sql.ast.dsl.AstDSL; @@ -45,6 +50,7 @@ import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.HighlightExpression; import org.opensearch.sql.expression.config.ExpressionConfig; import org.opensearch.sql.expression.window.aggregation.AggregateWindowFunction; @@ -537,6 +543,49 @@ public void match_phrase_prefix_all_params() { ); } + private static Stream functionNames() { + var dsl = new DSL(new ExpressionConfig().functionRepository()); + return Stream.of( + Arguments.of((Function)dsl::now, + "now", true), + Arguments.of((Function)dsl::current_timestamp, + "current_timestamp", true), + Arguments.of((Function)dsl::localtimestamp, + "localtimestamp", true), + Arguments.of((Function)dsl::localtime, + "localtime", true), + Arguments.of((Function)dsl::sysdate, + "sysdate", true), + Arguments.of((Function)dsl::curtime, + "curtime", true), + Arguments.of((Function)dsl::current_time, + "current_time", true), + Arguments.of((Function)dsl::curdate, + "curdate", false), + Arguments.of((Function)dsl::current_date, + "current_date", false)); + } + + @ParameterizedTest(name = "{1}") + @MethodSource("functionNames") + public void now_like_functions(Function function, + String name, + Boolean hasFsp) { + assertAnalyzeEqual( + function.apply(new Expression[]{}), + AstDSL.function(name)); + + if (hasFsp) { + assertAnalyzeEqual( + function.apply(new Expression[]{DSL.ref("integer_value", INTEGER)}), + AstDSL.function(name, field("integer_value"))); + + assertAnalyzeEqual( + function.apply(new Expression[]{DSL.literal(3)}), + AstDSL.function(name, intLiteral(3))); + } + } + @Test void highlight() { assertAnalyzeEqual(new HighlightExpression(DSL.literal("fieldA")), diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/NowLikeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/NowLikeFunctionTest.java new file mode 100644 index 00000000000..76a7e4be464 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/NowLikeFunctionTest.java @@ -0,0 +1,128 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.expression.datetime; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opensearch.sql.data.type.ExprCoreType.DATE; +import static org.opensearch.sql.data.type.ExprCoreType.DATETIME; +import static org.opensearch.sql.data.type.ExprCoreType.TIME; + +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Period; +import java.time.temporal.Temporal; +import java.util.List; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.opensearch.sql.data.type.ExprCoreType; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.ExpressionTestBase; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.config.ExpressionConfig; + + +public class NowLikeFunctionTest extends ExpressionTestBase { + private static Stream functionNames() { + var dsl = new DSL(new ExpressionConfig().functionRepository()); + return Stream.of( + Arguments.of((Function)dsl::now, + "now", DATETIME, true, (Supplier)LocalDateTime::now), + Arguments.of((Function)dsl::current_timestamp, + "current_timestamp", DATETIME, true, (Supplier)LocalDateTime::now), + Arguments.of((Function)dsl::localtimestamp, + "localtimestamp", DATETIME, true, (Supplier)LocalDateTime::now), + Arguments.of((Function)dsl::localtime, + "localtime", DATETIME, true, (Supplier)LocalDateTime::now), + Arguments.of((Function)dsl::sysdate, + "sysdate", DATETIME, true, (Supplier)LocalDateTime::now), + Arguments.of((Function)dsl::curtime, + "curtime", TIME, true, (Supplier)LocalTime::now), + Arguments.of((Function)dsl::current_time, + "current_time", TIME, true, (Supplier)LocalTime::now), + Arguments.of((Function)dsl::curdate, + "curdate", DATE, false, (Supplier)LocalDate::now), + Arguments.of((Function)dsl::current_date, + "current_date", DATE, false, (Supplier)LocalDate::now)); + } + + private Temporal extractValue(FunctionExpression func) { + switch ((ExprCoreType)func.type()) { + case DATE: return func.valueOf(null).dateValue(); + case DATETIME: return func.valueOf(null).datetimeValue(); + case TIME: return func.valueOf(null).timeValue(); + // unreachable code + default: throw new IllegalArgumentException(String.format("%s", func.type())); + } + } + + private long getDiff(Temporal sample, Temporal reference) { + if (sample instanceof LocalDate) { + return Period.between((LocalDate) sample, (LocalDate) reference).getDays(); + } + return Duration.between(sample, reference).toSeconds(); + } + + /** + * Check how NOW-like functions are processed. + * @param function Function + * @param name Function name + * @param resType Return type + * @param hasFsp Whether function has fsp argument + * @param referenceGetter A callback to get reference value + */ + @ParameterizedTest(name = "{1}") + @MethodSource("functionNames") + public void test_now_like_functions(Function function, + @SuppressWarnings("unused") // Used in the test name above + String name, + ExprCoreType resType, + Boolean hasFsp, + Supplier referenceGetter) { + // Check return types: + // `func()` + FunctionExpression expr = function.apply(new Expression[]{}); + assertEquals(resType, expr.type()); + if (hasFsp) { + // `func(fsp = 0)` + expr = function.apply(new Expression[]{DSL.literal(0)}); + assertEquals(resType, expr.type()); + // `func(fsp = 6)` + expr = function.apply(new Expression[]{DSL.literal(6)}); + assertEquals(resType, expr.type()); + + for (var wrongFspValue: List.of(-1, 10)) { + var exception = assertThrows(IllegalArgumentException.class, + () -> function.apply(new Expression[]{DSL.literal(wrongFspValue)}).valueOf(null)); + assertEquals(String.format("Invalid `fsp` value: %d, allowed 0 to 6", wrongFspValue), + exception.getMessage()); + } + } + + // Check how calculations are precise: + // `func()` + assertTrue(Math.abs(getDiff( + extractValue(function.apply(new Expression[]{})), + referenceGetter.get() + )) <= 1); + if (hasFsp) { + // `func(fsp)` + assertTrue(Math.abs(getDiff( + extractValue(function.apply(new Expression[]{DSL.literal(0)})), + referenceGetter.get() + )) <= 1); + } + } +} diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index f1d4d987a37..e4a44213cca 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -1328,9 +1328,210 @@ NOW Description >>>>>>>>>>> +Returns the current date and time as a value in 'YYYY-MM-DD hh:mm:ss.nnnnnn' format. The value is expressed in the cluster time zone. +`NOW()` returns a constant time that indicates the time at which the statement began to execute. This differs from the behavior for `SYSDATE() <#sysdate>`_, which returns the exact time at which it executes. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Argument type: (optional) INTEGER + +Return type: DATETIME + +Specifications: + +1. NOW() -> DATETIME +2. NOW(INTEGER) -> DATETIME + +Example:: + + > SELECT NOW(), NOW(0); + fetched rows / total rows = 1/1 + +----------------------------+---------------------+ + | NOW() | NOW(0) | + |----------------------------+---------------------| + | 2022-08-02 15:39:05.173069 | 2022-08-02 15:39:05 | + +----------------------------+---------------------+ + + +CURRENT_TIMESTAMP +----------------- + +Description +>>>>>>>>>>> + +`CURRENT_TIMESTAMP` and `CURRENT_TIMESTAMP()` are synonyms for `NOW() <#now>`_. + +Example:: + + > SELECT CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP(0), CURRENT_TIMESTAMP; + fetched rows / total rows = 1/1 + +----------------------------+------------------------+----------------------------+ + | CURRENT_TIMESTAMP() | CURRENT_TIMESTAMP(0) | CURRENT_TIMESTAMP | + |----------------------------+------------------------+----------------------------| + | 2022-08-02 15:54:19.209361 | 2022-08-02 15:54:19 | 2022-08-02 15:54:19.209361 | + +----------------------------+------------------------+----------------------------+ + + +LOCALTIMESTAMP +-------------- + +Description +>>>>>>>>>>> + +`LOCALTIMESTAMP` and `LOCALTIMESTAMP()` are synonyms for `NOW() <#now>`_. + +Example:: + + > SELECT LOCALTIMESTAMP(), LOCALTIMESTAMP(0), LOCALTIMESTAMP; + fetched rows / total rows = 1/1 + +----------------------------+---------------------+----------------------------+ + | LOCALTIMESTAMP() | LOCALTIMESTAMP(0) | LOCALTIMESTAMP | + |----------------------------+---------------------+----------------------------| + | 2022-08-02 15:54:19.209361 | 2022-08-02 15:54:19 | 2022-08-02 15:54:19.209361 | + +----------------------------+---------------------+----------------------------+ + + +LOCALTIME +--------- + +Description +>>>>>>>>>>> + +`LOCALTIME` and `LOCALTIME()` are synonyms for `NOW() <#now>`_. + +Example:: + + > SELECT LOCALTIME(), LOCALTIME(0), LOCALTIME; + fetched rows / total rows = 1/1 + +----------------------------+---------------------+----------------------------+ + | LOCALTIME() | LOCALTIME(0) | LOCALTIME | + |----------------------------+---------------------+----------------------------| + | 2022-08-02 15:54:19.209361 | 2022-08-02 15:54:19 | 2022-08-02 15:54:19.209361 | + +----------------------------+---------------------+----------------------------+ + + +SYSDATE +------- + +Description +>>>>>>>>>>> + +Returns the current date and time as a value in 'YYYY-MM-DD hh:mm:ss.nnnnnn'. +SYSDATE() returns the time at which it executes. This differs from the behavior for `NOW() <#now>`_, which returns a constant time that indicates the time at which the statement began to execute. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Argument type: (optional) INTEGER + +Return type: DATETIME + +Specifications: + +1. SYSDATE() -> DATETIME +2. SYSDATE(INTEGER) -> DATETIME + +Example:: + + > SELECT SYSDATE(), SYSDATE(0); + fetched rows / total rows = 1/1 + +----------------------------+---------------------+ + | SYSDATE() | SYSDATE(0) | + |----------------------------+---------------------| + | 2022-08-02 15:39:05.173069 | 2022-08-02 15:39:05 | + +----------------------------+---------------------+ + + +CURTIME +------- + +Description +>>>>>>>>>>> + +Returns the current time as a value in 'hh:mm:ss.nnnnnn'. +CURTIME() returns the time at which it executes as `SYSDATE() <#sysdate>`_ does. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Argument type: (optional) INTEGER + +Return type: TIME + Specifications: -1. NOW() -> DATE +1. CURTIME() -> TIME +2. CURTIME(INTEGER) -> TIME + +Example:: + + > SELECT CURTIME(), CURTIME(0); + fetched rows / total rows = 1/1 + +-----------------+--------------+ + | CURTIME() | CURTIME(0) | + |-----------------+--------------| + | 15:39:05.173069 | 15:39:05 | + +-----------------+--------------+ + + +CURRENT_TIME +------------ + +Description +>>>>>>>>>>> + +`CURRENT_TIME` and `CURRENT_TIME()` are synonyms for `CURTIME() <#curtime>`_. + +Example:: + + > SELECT CURRENT_TIME(), CURRENT_TIME(0), CURRENT_TIME; + fetched rows / total rows = 1/1 + +------------------+-------------------+-----------------+ + | CURRENT_TIME() | CURRENT_TIME(0) | CURRENT_TIME | + |------------------+-------------------+-----------------| + | 15:39:05.173069 | 15:39:05 | 15:39:05.173069 | + +------------------+-------------------+-----------------+ + + +CURDATE +------- + +Description +>>>>>>>>>>> + +Returns the current time as a value in 'YYYY-MM-DD'. +CURDATE() returns the time at which it executes as `SYSDATE() <#sysdate>`_ does. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Return type: DATE + +Specifications: + +CURDATE() -> DATE + +Example:: + + > SELECT CURDATE(); + fetched rows / total rows = 1/1 + +-------------+ + | CURDATE() | + |-------------| + | 2022-08-02 | + +-------------+ + + +CURRENT_DATE +------------ + +Description +>>>>>>>>>>> + +`CURRENT_DATE` and `CURRENT_DATE()` are synonyms for `CURDATE() <#curdate>`_. + +Example:: + + > SELECT CURRENT_DATE(), CURRENT_DATE; + fetched rows / total rows = 1/1 + +------------------+----------------+ + | CURRENT_DATE() | CURRENT_DATE | + |------------------+----------------| + | 2022-08-02 | 2022-08-02 | + +------------------+----------------+ QUARTER diff --git a/docs/user/ppl/functions/datetime.rst b/docs/user/ppl/functions/datetime.rst index 5be5686c34d..5a653c3e64b 100644 --- a/docs/user/ppl/functions/datetime.rst +++ b/docs/user/ppl/functions/datetime.rst @@ -497,9 +497,210 @@ NOW Description >>>>>>>>>>> +Returns the current date and time as a value in 'YYYY-MM-DD hh:mm:ss.nnnnnn' format. The value is expressed in the cluster time zone. +`NOW()` returns a constant time that indicates the time at which the statement began to execute. This differs from the behavior for `SYSDATE() <#sysdate>`_, which returns the exact time at which it executes. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Argument type: (optional) INTEGER + +Return type: DATETIME + +Specifications: + +1. NOW() -> DATETIME +2. NOW(INTEGER) -> DATETIME + +Example:: + + > source=people | eval `NOW()` = NOW(), `NOW(0)` = NOW(0) | fields `NOW()`, `NOW(0)` + fetched rows / total rows = 1/1 + +----------------------------+---------------------+ + | NOW() | NOW(0) | + |----------------------------+---------------------| + | 2022-08-02 15:39:05.173069 | 2022-08-02 15:39:05 | + +----------------------------+---------------------+ + + +CURRENT_TIMESTAMP +----------------- + +Description +>>>>>>>>>>> + +`CURRENT_TIMESTAMP` and `CURRENT_TIMESTAMP()` are synonyms for `NOW() <#now>`_. + +Example:: + + > source=people | eval `CURRENT_TIMESTAMP()` = CURRENT_TIMESTAMP(), `CURRENT_TIMESTAMP(0)` = CURRENT_TIMESTAMP(0), `CURRENT_TIMESTAMP` = CURRENT_TIMESTAMP | fields `CURRENT_TIMESTAMP()`, `CURRENT_TIMESTAMP(0)`, `CURRENT_TIMESTAMP` + fetched rows / total rows = 1/1 + +----------------------------+------------------------+----------------------------+ + | CURRENT_TIMESTAMP() | CURRENT_TIMESTAMP(0) | CURRENT_TIMESTAMP | + |----------------------------+------------------------+----------------------------| + | 2022-08-02 15:54:19.209361 | 2022-08-02 15:54:19 | 2022-08-02 15:54:19.209361 | + +----------------------------+------------------------+----------------------------+ + + +LOCALTIMESTAMP +-------------- + +Description +>>>>>>>>>>> + +`LOCALTIMESTAMP` and `LOCALTIMESTAMP()` are synonyms for `NOW() <#now>`_. + +Example:: + + > source=people | eval `LOCALTIMESTAMP()` = LOCALTIMESTAMP(), `LOCALTIMESTAMP(0)` = LOCALTIMESTAMP(0), `LOCALTIMESTAMP` = LOCALTIMESTAMP | fields `LOCALTIMESTAMP()`, `LOCALTIMESTAMP(0)`, `LOCALTIMESTAMP` + fetched rows / total rows = 1/1 + +----------------------------+---------------------+----------------------------+ + | LOCALTIMESTAMP() | LOCALTIMESTAMP(0) | LOCALTIMESTAMP | + |----------------------------+---------------------+----------------------------| + | 2022-08-02 15:54:19.209361 | 2022-08-02 15:54:19 | 2022-08-02 15:54:19.209361 | + +----------------------------+---------------------+----------------------------+ + + +LOCALTIME +--------- + +Description +>>>>>>>>>>> + +`LOCALTIME` and `LOCALTIME()` are synonyms for `NOW() <#now>`_. + +Example:: + + > source=people | eval `LOCALTIME()` = LOCALTIME(), `LOCALTIME(0)` = LOCALTIME(0), `LOCALTIME` = LOCALTIME | fields `LOCALTIME()`, `LOCALTIME(0)`, `LOCALTIME` + fetched rows / total rows = 1/1 + +----------------------------+---------------------+----------------------------+ + | LOCALTIME() | LOCALTIME(0) | LOCALTIME | + |----------------------------+---------------------+----------------------------| + | 2022-08-02 15:54:19.209361 | 2022-08-02 15:54:19 | 2022-08-02 15:54:19.209361 | + +----------------------------+---------------------+----------------------------+ + + +SYSDATE +------- + +Description +>>>>>>>>>>> + +Returns the current date and time as a value in 'YYYY-MM-DD hh:mm:ss.nnnnnn'. +SYSDATE() returns the time at which it executes. This differs from the behavior for `NOW() <#now>`_, which returns a constant time that indicates the time at which the statement began to execute. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Argument type: (optional) INTEGER + +Return type: DATETIME + Specifications: -1. NOW() -> DATE +1. SYSDATE() -> DATETIME +2. SYSDATE(INTEGER) -> DATETIME + +Example:: + + > source=people | eval `SYSDATE()` = SYSDATE(), `SYSDATE(0)` = SYSDATE(0) | fields `SYSDATE()`, `SYSDATE(0)` + fetched rows / total rows = 1/1 + +----------------------------+---------------------+ + | SYSDATE() | SYSDATE(0) | + |----------------------------+---------------------| + | 2022-08-02 15:39:05.173069 | 2022-08-02 15:39:05 | + +----------------------------+---------------------+ + + +CURTIME +------- + +Description +>>>>>>>>>>> + +Returns the current time as a value in 'hh:mm:ss.nnnnnn'. +CURTIME() returns the time at which it executes as `SYSDATE() <#sysdate>`_ does. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Argument type: (optional) INTEGER + +Return type: TIME + +Specifications: + +1. CURTIME() -> TIME +2. CURTIME(INTEGER) -> TIME + +Example:: + + > source=people | eval `CURTIME()` = CURTIME(), `CURTIME(0)` = CURTIME(0) | fields `CURTIME()`, `CURTIME(0)` + fetched rows / total rows = 1/1 + +-----------------+--------------+ + | CURTIME() | CURTIME(0) | + |-----------------+--------------| + | 15:39:05.173069 | 15:39:05 | + +-----------------+--------------+ + + +CURRENT_TIME +------------ + +Description +>>>>>>>>>>> + +`CURRENT_TIME` and `CURRENT_TIME()` are synonyms for `CURTIME() <#curtime>`_. + +Example:: + + > source=people | eval `CURRENT_TIME()` = CURRENT_TIME(), `CURRENT_TIME(0)` = CURRENT_TIME(0), `CURRENT_TIME` = CURRENT_TIME | fields `CURRENT_TIME()`, `CURRENT_TIME(0)`, `CURRENT_TIME` + fetched rows / total rows = 1/1 + +------------------+-------------------+-----------------+ + | CURRENT_TIME() | CURRENT_TIME(0) | CURRENT_TIME | + |------------------+-------------------+-----------------| + | 15:39:05.173069 | 15:39:05 | 15:39:05.173069 | + +------------------+-------------------+-----------------+ + + +CURDATE +------- + +Description +>>>>>>>>>>> + +Returns the current time as a value in 'YYYY-MM-DD'. +CURDATE() returns the time at which it executes as `SYSDATE() <#sysdate>`_ does. +If the argument is given, it specifies a fractional seconds precision from 0 to 6, the return value includes a fractional seconds part of that many digits. + +Return type: DATE + +Specifications: + +CURDATE() -> DATE + +Example:: + + > source=people | eval `CURDATE()` = CURDATE() | fields `CURDATE()` + fetched rows / total rows = 1/1 + +-------------+ + | CURDATE() | + |-------------| + | 2022-08-02 | + +-------------+ + + +CURRENT_DATE +------------ + +Description +>>>>>>>>>>> + +`CURRENT_DATE` and `CURRENT_DATE()` are synonyms for `CURDATE() <#curdate>`_. + +Example:: + + > source=people | eval `CURRENT_DATE()` = CURRENT_DATE(), `CURRENT_DATE` = CURRENT_DATE | fields `CURRENT_DATE()`, `CURRENT_DATE` + fetched rows / total rows = 1/1 + +------------------+----------------+ + | CURRENT_DATE() | CURRENT_DATE | + |------------------+----------------| + | 2022-08-02 | 2022-08-02 | + +------------------+----------------+ QUARTER diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 864b4df0971..429c360a1ba 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -53,9 +53,9 @@ configurations.all { // enforce 1.1.3, https://www.whitesourcesoftware.com/vulnerability-database/WS-2019-0379 resolutionStrategy.force 'commons-codec:commons-codec:1.13' resolutionStrategy.force 'com.google.guava:guava:31.0.1-jre' - resolutionStrategy.force 'com.fasterxml.jackson.core:jackson-core:2.13.3' - resolutionStrategy.force 'com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.13.3' - resolutionStrategy.force 'com.fasterxml.jackson.core:jackson-databind:2.13.3' + resolutionStrategy.force "com.fasterxml.jackson.core:jackson-core:${jackson_version}" + resolutionStrategy.force "com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${jackson_version}" + resolutionStrategy.force "com.fasterxml.jackson.core:jackson-databind:${jackson_version}" } dependencies { diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 15f0261f0d0..5c339cc7bb4 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -31,6 +31,7 @@ import java.nio.file.Paths; import java.util.Locale; +import static com.google.common.base.Strings.isNullOrEmpty; import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient; import static org.opensearch.sql.legacy.TestUtils.getAccountIndexMapping; import static org.opensearch.sql.legacy.TestUtils.getBankIndexMapping; @@ -71,6 +72,8 @@ public abstract class SQLIntegTestCase extends OpenSearchSQLRestTestCase { public static final String TRANSIENT = "transient"; public static final Integer DEFAULT_QUERY_SIZE_LIMIT = Integer.parseInt(System.getProperty("defaultQuerySizeLimit", "200")); + public static final Integer DEFAULT_MAX_RESULT_WINDOW = + Integer.parseInt(System.getProperty("defaultMaxResultWindow", "10000")); public boolean shouldResetQuerySizeLimit() { return true; @@ -161,6 +164,15 @@ protected static void wipeAllClusterSettings() throws IOException { updateClusterSettings(new ClusterSetting("transient", "*", null)); } + protected void setMaxResultWindow(String indexName, Integer window) throws IOException { + updateIndexSettings(indexName, "{ \"index\": { \"max_result_window\":" + window + " } }"); + } + + protected void resetMaxResultWindow(String indexName) throws IOException { + updateIndexSettings(indexName, + "{ \"index\": { \"max_result_window\": " + DEFAULT_MAX_RESULT_WINDOW + " } }"); + } + /** * Provide for each test to load test index, data and other setup work */ @@ -378,6 +390,18 @@ public String toString() { } } + protected static JSONObject updateIndexSettings(String indexName, String setting) + throws IOException { + Request request = new Request("PUT", "/" + indexName + "/_settings"); + if (!isNullOrEmpty(setting)) { + request.setJsonEntity(setting); + } + RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder(); + restOptionsBuilder.addHeader("Content-Type", "application/json"); + request.setOptions(restOptionsBuilder); + return new JSONObject(executeRequest(request)); + } + protected String makeRequest(String query) { return makeRequest(query, 0); } 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 fcbfc27710c..2db47d2d20c 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 @@ -7,12 +7,31 @@ package org.opensearch.sql.ppl; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_PEOPLE2; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; import static org.opensearch.sql.util.MatcherUtils.verifySchema; import static org.opensearch.sql.util.MatcherUtils.verifySome; import java.io.IOException; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Period; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; +import java.time.temporal.ChronoField; +import java.time.temporal.Temporal; +import java.util.ArrayList; +import java.util.List; +import java.util.TimeZone; +import java.util.function.BiFunction; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import com.google.common.collect.ImmutableMap; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.opensearch.sql.common.utils.StringUtils; @@ -22,6 +41,7 @@ public class DateTimeFunctionIT extends PPLIntegTestCase { @Override public void init() throws IOException { loadIndex(Index.DATE); + loadIndex(Index.PEOPLE2); } @Test @@ -463,4 +483,167 @@ public void testDateFormatISO8601() throws IOException { verifyDateFormat(date, "date", dateFormat, dateFormatted); } + private List> nowLikeFunctionsData() { + return List.of( + ImmutableMap.builder() + .put("name", "now") + .put("hasFsp", true) + .put("hasShortcut", false) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "current_timestamp") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "localtimestamp") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "localtime") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "sysdate") + .put("hasFsp", true) + .put("hasShortcut", false) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "curtime") + .put("hasFsp", true) + .put("hasShortcut", false) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalTime::now) + .put("parser", (BiFunction) LocalTime::parse) + .put("serializationPattern", "HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "current_time") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalTime::now) + .put("parser", (BiFunction) LocalTime::parse) + .put("serializationPattern", "HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "curdate") + .put("hasFsp", false) + .put("hasShortcut", false) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalDate::now) + .put("parser", (BiFunction) LocalDate::parse) + .put("serializationPattern", "uuuu-MM-dd") + .build(), + ImmutableMap.builder() + .put("name", "current_date") + .put("hasFsp", false) + .put("hasShortcut", true) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalDate::now) + .put("parser", (BiFunction) LocalDate::parse) + .put("serializationPattern", "uuuu-MM-dd") + .build() + ); + } + + private long getDiff(Temporal sample, Temporal reference) { + if (sample instanceof LocalDate) { + return Period.between((LocalDate) sample, (LocalDate) reference).getDays(); + } + return Duration.between(sample, reference).toSeconds(); + } + + @Test + public void testNowLikeFunctions() throws IOException { + // Integration test framework sets for OpenSearch instance a random timezone. + // If server's TZ doesn't match localhost's TZ, time measurements for `now` would differ. + // We should set localhost's TZ now and recover the value back in the end of the test. + var testTz = TimeZone.getDefault(); + TimeZone.setDefault(TimeZone.getTimeZone(System.getProperty("user.timezone"))); + + for (var funcData : nowLikeFunctionsData()) { + String name = (String) funcData.get("name"); + Boolean hasFsp = (Boolean) funcData.get("hasFsp"); + Boolean hasShortcut = (Boolean) funcData.get("hasShortcut"); + Boolean constValue = (Boolean) funcData.get("constValue"); + Supplier referenceGetter = (Supplier) funcData.get("referenceGetter"); + BiFunction parser = + (BiFunction) funcData.get("parser"); + String serializationPatternStr = (String) funcData.get("serializationPattern"); + + var serializationPattern = new DateTimeFormatterBuilder() + .appendPattern(serializationPatternStr) + .optionalStart() + .appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true) + .toFormatter(); + + Temporal reference = referenceGetter.get(); + double delta = 2d; // acceptable time diff, secs + if (reference instanceof LocalDate) + delta = 1d; // Max date delta could be 1 if test runs on the very edge of two days + // We ignore probability of a test run on edge of month or year to simplify the checks + + var calls = new ArrayList() {{ + add(name + "()"); + }}; + if (hasShortcut) + calls.add(name); + if (hasFsp) + calls.add(name + "(0)"); + + // Column order is: func(), func, func(0) + // shortcut ^ fsp ^ + // Query looks like: + // source=people2 | eval `now()`=now() | fields `now()`; + JSONObject result = executeQuery("source=" + TEST_INDEX_PEOPLE2 + + " | eval " + calls.stream().map(c -> String.format("`%s`=%s", c, c)).collect(Collectors.joining(",")) + + " | fields " + calls.stream().map(c -> String.format("`%s`", c)).collect(Collectors.joining(","))); + + var rows = result.getJSONArray("datarows"); + JSONArray firstRow = rows.getJSONArray(0); + for (int i = 0; i < rows.length(); i++) { + var row = rows.getJSONArray(i); + if (constValue) + assertTrue(firstRow.similar(row)); + + int column = 0; + assertEquals(0, + getDiff(reference, parser.apply(row.getString(column++), serializationPattern)), delta); + + if (hasShortcut) { + assertEquals(0, + getDiff(reference, parser.apply(row.getString(column++), serializationPattern)), delta); + } + if (hasFsp) { + assertEquals(0, + getDiff(reference, parser.apply(row.getString(column), serializationPattern)), delta); + } + } + } + + TimeZone.setDefault(testTz); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/HeadCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/HeadCommandIT.java index 1ae45ab4694..48c489ce109 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/HeadCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/HeadCommandIT.java @@ -14,6 +14,7 @@ import org.json.JSONObject; import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.jupiter.api.Test; public class HeadCommandIT extends PPLIntegTestCase { @@ -26,6 +27,7 @@ public void beforeTest() throws IOException { @After public void afterTest() throws IOException { resetQuerySizeLimit(); + resetMaxResultWindow(TEST_INDEX_ACCOUNT); } @Override @@ -60,6 +62,76 @@ public void testHeadWithNumber() throws IOException { rows("Nanette", 28)); } + @Ignore("Fix https://github.com/opensearch-project/sql/issues/703#issuecomment-1211422130") + @Test + public void testHeadWithNumberLargerThanQuerySizeLimit() throws IOException { + setQuerySizeLimit(5); + JSONObject result = + executeQuery(String.format( + "source=%s | fields firstname, age | head 10", TEST_INDEX_ACCOUNT)); + verifyDataRows(result, + rows("Amber", 32), + rows("Hattie", 36), + rows("Nanette", 28), + rows("Dale", 33), + rows("Elinor", 36), + rows("Virginia", 39), + rows("Dillard", 34), + rows("Mcgee", 39), + rows("Aurelia", 37), + rows("Fulton", 23)); + } + + @Test + public void testHeadWithNumberLargerThanMaxResultWindow() throws IOException { + setMaxResultWindow(TEST_INDEX_ACCOUNT, 10); + JSONObject result = + executeQuery(String.format( + "source=%s | fields firstname, age | head 15", TEST_INDEX_ACCOUNT)); + verifyDataRows(result, + rows("Amber", 32), + rows("Hattie", 36), + rows("Nanette", 28), + rows("Dale", 33), + rows("Elinor", 36), + rows("Virginia", 39), + rows("Dillard", 34), + rows("Mcgee", 39), + rows("Aurelia", 37), + rows("Fulton", 23), + rows("Burton", 31), + rows("Josie", 32), + rows("Hughes", 30), + rows("Hall", 25), + rows("Deidre", 33)); + } + + @Ignore("Fix https://github.com/opensearch-project/sql/issues/703#issuecomment-1211422130") + @Test + public void testHeadWithLargeNumber() throws IOException { + setQuerySizeLimit(5); + setMaxResultWindow(TEST_INDEX_ACCOUNT, 10); + JSONObject result = + executeQuery(String.format( + "source=%s | fields firstname, age | head 15", TEST_INDEX_ACCOUNT)); + verifyDataRows(result, + rows("Amber", 32), + rows("Hattie", 36), + rows("Nanette", 28), + rows("Dale", 33), + rows("Elinor", 36), + rows("Virginia", 39), + rows("Dillard", 34), + rows("Mcgee", 39), + rows("Aurelia", 37), + rows("Fulton", 23), + rows("Burton", 31), + rows("Josie", 32), + rows("Hughes", 30), + rows("Hall", 25), + rows("Deidre", 33)); + } + @Test public void testHeadWithNumberAndFrom() throws IOException { JSONObject result = 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 d19c3719b6d..91d02bca513 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 @@ -7,6 +7,7 @@ package org.opensearch.sql.sql; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_PEOPLE2; import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; @@ -15,7 +16,23 @@ import static org.opensearch.sql.util.TestUtils.getResponseBody; import java.io.IOException; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Period; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; +import java.time.temporal.ChronoField; +import java.time.temporal.Temporal; +import java.util.ArrayList; +import java.util.List; import java.util.Locale; +import java.util.TimeZone; +import java.util.function.BiFunction; +import java.util.function.Supplier; +import com.google.common.collect.ImmutableMap; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.opensearch.client.Request; @@ -30,6 +47,7 @@ public class DateTimeFunctionIT extends SQLIntegTestCase { public void init() throws Exception { super.init(); loadIndex(Index.BANK); + loadIndex(Index.PEOPLE2); } @Test @@ -452,6 +470,166 @@ public void testDateFormat() throws IOException { verifyDateFormat(date, "date", dateFormat, dateFormatted); } + private List> nowLikeFunctionsData() { + return List.of( + ImmutableMap.builder() + .put("name", "now") + .put("hasFsp", true) + .put("hasShortcut", false) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "current_timestamp") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "localtimestamp") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "localtime") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", true) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "sysdate") + .put("hasFsp", true) + .put("hasShortcut", false) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalDateTime::now) + .put("parser", (BiFunction) LocalDateTime::parse) + .put("serializationPattern", "uuuu-MM-dd HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "curtime") + .put("hasFsp", true) + .put("hasShortcut", false) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalTime::now) + .put("parser", (BiFunction) LocalTime::parse) + .put("serializationPattern", "HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "current_time") + .put("hasFsp", true) + .put("hasShortcut", true) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalTime::now) + .put("parser", (BiFunction) LocalTime::parse) + .put("serializationPattern", "HH:mm:ss") + .build(), + ImmutableMap.builder() + .put("name", "curdate") + .put("hasFsp", false) + .put("hasShortcut", false) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalDate::now) + .put("parser", (BiFunction) LocalDate::parse) + .put("serializationPattern", "uuuu-MM-dd") + .build(), + ImmutableMap.builder() + .put("name", "current_date") + .put("hasFsp", false) + .put("hasShortcut", true) + .put("constValue", false) + .put("referenceGetter", (Supplier) LocalDate::now) + .put("parser", (BiFunction) LocalDate::parse) + .put("serializationPattern", "uuuu-MM-dd") + .build() + ); + } + + private long getDiff(Temporal sample, Temporal reference) { + if (sample instanceof LocalDate) { + return Period.between((LocalDate) sample, (LocalDate) reference).getDays(); + } + return Duration.between(sample, reference).toSeconds(); + } + + @Test + public void testNowLikeFunctions() throws IOException { + // Integration test framework sets for OpenSearch instance a random timezone. + // If server's TZ doesn't match localhost's TZ, time measurements for `now` would differ. + // We should set localhost's TZ now and recover the value back in the end of the test. + var testTz = TimeZone.getDefault(); + TimeZone.setDefault(TimeZone.getTimeZone(System.getProperty("user.timezone"))); + + for (var funcData : nowLikeFunctionsData()) { + String name = (String) funcData.get("name"); + Boolean hasFsp = (Boolean) funcData.get("hasFsp"); + Boolean hasShortcut = (Boolean) funcData.get("hasShortcut"); + Boolean constValue = (Boolean) funcData.get("constValue"); + Supplier referenceGetter = (Supplier) funcData.get("referenceGetter"); + BiFunction parser = + (BiFunction) funcData.get("parser"); + String serializationPatternStr = (String) funcData.get("serializationPattern"); + + var serializationPattern = new DateTimeFormatterBuilder() + .appendPattern(serializationPatternStr) + .optionalStart() + .appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true) + .toFormatter(); + + Temporal reference = referenceGetter.get(); + double delta = 2d; // acceptable time diff, secs + if (reference instanceof LocalDate) + delta = 1d; // Max date delta could be 1 if test runs on the very edge of two days + // We ignore probability of a test run on edge of month or year to simplify the checks + + var calls = new ArrayList() {{ + add(name + "()"); + }}; + if (hasShortcut) + calls.add(name); + if (hasFsp) + calls.add(name + "(0)"); + + // Column order is: func(), func, func(0) + // shortcut ^ fsp ^ + JSONObject result = executeQuery("select " + String.join(", ", calls) + " from " + TEST_INDEX_PEOPLE2); + + var rows = result.getJSONArray("datarows"); + JSONArray firstRow = rows.getJSONArray(0); + for (int i = 0; i < rows.length(); i++) { + var row = rows.getJSONArray(i); + if (constValue) + assertTrue(firstRow.similar(row)); + + int column = 0; + assertEquals(0, + getDiff(reference, parser.apply(row.getString(column++), serializationPattern)), delta); + + if (hasShortcut) { + assertEquals(0, + getDiff(reference, parser.apply(row.getString(column++), serializationPattern)), delta); + } + if (hasFsp) { + assertEquals(0, + getDiff(reference, parser.apply(row.getString(column), serializationPattern)), delta); + } + } + } + + TimeZone.setDefault(testTz); + } + protected JSONObject executeQuery(String query) throws IOException { Request request = new Request("POST", QUERY_API_ENDPOINT); request.setJsonEntity(String.format(Locale.ROOT, "{\n" + " \"query\": \"%s\"\n" + "}", query)); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/AsyncRestExecutor.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/AsyncRestExecutor.java index e6406e8b3ee..4ad6e557771 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/AsyncRestExecutor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/AsyncRestExecutor.java @@ -19,13 +19,13 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestStatus; import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.legacy.esdomain.LocalClusterState; import org.opensearch.sql.legacy.exception.SqlParseException; import org.opensearch.sql.legacy.metrics.MetricName; import org.opensearch.sql.legacy.metrics.Metrics; import org.opensearch.sql.legacy.query.QueryAction; import org.opensearch.sql.legacy.query.join.BackOffRetryStrategy; -import org.opensearch.sql.legacy.utils.LogUtils; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.Transports; @@ -73,13 +73,13 @@ public void execute(Client client, Map params, QueryAction query if (isBlockingAction(queryAction) && isRunningInTransportThread()) { if (LOG.isDebugEnabled()) { LOG.debug("[{}] Async blocking query action [{}] for executor [{}] in current thread [{}]", - LogUtils.getRequestId(), name(executor), name(queryAction), Thread.currentThread().getName()); + QueryContext.getRequestId(), name(executor), name(queryAction), Thread.currentThread().getName()); } async(client, params, queryAction, channel); } else { if (LOG.isDebugEnabled()) { LOG.debug("[{}] Continue running query action [{}] for executor [{}] in current thread [{}]", - LogUtils.getRequestId(), name(executor), name(queryAction), Thread.currentThread().getName()); + QueryContext.getRequestId(), name(executor), name(queryAction), Thread.currentThread().getName()); } doExecuteWithTimeMeasured(client, params, queryAction, channel); } @@ -110,18 +110,18 @@ private void async(Client client, Map params, QueryAction queryA doExecuteWithTimeMeasured(client, params, queryAction, channel); } catch (IOException | SqlParseException | OpenSearchException e) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); - LOG.warn("[{}] [MCB] async task got an IO/SQL exception: {}", LogUtils.getRequestId(), + LOG.warn("[{}] [MCB] async task got an IO/SQL exception: {}", QueryContext.getRequestId(), e.getMessage()); channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); } catch (IllegalStateException e) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); - LOG.warn("[{}] [MCB] async task got a runtime exception: {}", LogUtils.getRequestId(), + LOG.warn("[{}] [MCB] async task got a runtime exception: {}", QueryContext.getRequestId(), e.getMessage()); channel.sendResponse(new BytesRestResponse(RestStatus.INSUFFICIENT_STORAGE, "Memory circuit is broken.")); } catch (Throwable t) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); - LOG.warn("[{}] [MCB] async task got an unknown throwable: {}", LogUtils.getRequestId(), + LOG.warn("[{}] [MCB] async task got an unknown throwable: {}", QueryContext.getRequestId(), t.getMessage()); channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, String.valueOf(t.getMessage()))); @@ -132,7 +132,7 @@ private void async(Client client, Map params, QueryAction queryA // Preserve context of calling thread to ensure headers of requests are forwarded when running blocking actions threadPool.schedule( - LogUtils.withCurrentContext(runnable), + QueryContext.withCurrentContext(runnable), new TimeValue(0L), SQL_WORKER_THREAD_POOL_NAME ); @@ -152,7 +152,7 @@ private void doExecuteWithTimeMeasured(Client client, Duration elapsed = Duration.ofNanos(System.nanoTime() - startTime); int slowLogThreshold = LocalClusterState.state().getSettingValue(Settings.Key.SQL_SLOWLOG); if (elapsed.getSeconds() >= slowLogThreshold) { - LOG.warn("[{}] Slow query: elapsed={} (ms)", LogUtils.getRequestId(), elapsed.toMillis()); + LOG.warn("[{}] Slow query: elapsed={} (ms)", QueryContext.getRequestId(), elapsed.toMillis()); } } } diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorAsyncRestExecutor.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorAsyncRestExecutor.java index 0dc1fe301f5..7bb64215022 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorAsyncRestExecutor.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/cursor/CursorAsyncRestExecutor.java @@ -17,11 +17,11 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestStatus; import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.legacy.esdomain.LocalClusterState; import org.opensearch.sql.legacy.metrics.MetricName; import org.opensearch.sql.legacy.metrics.Metrics; import org.opensearch.sql.legacy.query.join.BackOffRetryStrategy; -import org.opensearch.sql.legacy.utils.LogUtils; import org.opensearch.threadpool.ThreadPool; public class CursorAsyncRestExecutor { @@ -57,20 +57,20 @@ private void async(Client client, Map params, RestChannel channe doExecuteWithTimeMeasured(client, params, channel); } catch (IOException e) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); - LOG.warn("[{}] [MCB] async task got an IO/SQL exception: {}", LogUtils.getRequestId(), + LOG.warn("[{}] [MCB] async task got an IO/SQL exception: {}", QueryContext.getRequestId(), e.getMessage()); e.printStackTrace(); channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); } catch (IllegalStateException e) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); - LOG.warn("[{}] [MCB] async task got a runtime exception: {}", LogUtils.getRequestId(), + LOG.warn("[{}] [MCB] async task got a runtime exception: {}", QueryContext.getRequestId(), e.getMessage()); e.printStackTrace(); channel.sendResponse(new BytesRestResponse(RestStatus.INSUFFICIENT_STORAGE, "Memory circuit is broken.")); } catch (Throwable t) { Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); - LOG.warn("[{}] [MCB] async task got an unknown throwable: {}", LogUtils.getRequestId(), + LOG.warn("[{}] [MCB] async task got an unknown throwable: {}", QueryContext.getRequestId(), t.getMessage()); t.printStackTrace(); channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, @@ -82,7 +82,7 @@ private void async(Client client, Map params, RestChannel channe // Preserve context of calling thread to ensure headers of requests are forwarded when running blocking actions threadPool.schedule( - LogUtils.withCurrentContext(runnable), + QueryContext.withCurrentContext(runnable), new TimeValue(0L), SQL_WORKER_THREAD_POOL_NAME ); @@ -101,7 +101,7 @@ private void doExecuteWithTimeMeasured(Client client, Duration elapsed = Duration.ofNanos(System.nanoTime() - startTime); int slowLogThreshold = LocalClusterState.state().getSettingValue(Settings.Key.SQL_SLOWLOG); if (elapsed.getSeconds() >= slowLogThreshold) { - LOG.warn("[{}] Slow query: elapsed={} (ms)", LogUtils.getRequestId(), elapsed.toMillis()); + LOG.warn("[{}] Slow query: elapsed={} (ms)", QueryContext.getRequestId(), elapsed.toMillis()); } } } diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java index 10d9dab0fa0..e7c40b536a4 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java @@ -35,6 +35,7 @@ import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; import org.opensearch.sql.common.antlr.SyntaxCheckException; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.legacy.antlr.OpenSearchLegacySqlAnalyzer; @@ -60,7 +61,6 @@ import org.opensearch.sql.legacy.request.SqlRequestParam; import org.opensearch.sql.legacy.rewriter.matchtoterm.VerificationException; import org.opensearch.sql.legacy.utils.JsonPrettyFormatter; -import org.opensearch.sql.legacy.utils.LogUtils; import org.opensearch.sql.legacy.utils.QueryDataAnonymizer; import org.opensearch.sql.sql.domain.SQLQueryRequest; @@ -123,7 +123,8 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli Metrics.getInstance().getNumericalMetric(MetricName.REQ_TOTAL).increment(); Metrics.getInstance().getNumericalMetric(MetricName.REQ_COUNT_TOTAL).increment(); - LogUtils.addRequestId(); + QueryContext.addRequestId(); + QueryContext.recordProcessingStarted(); try { if (!isSQLFeatureEnabled()) { @@ -137,12 +138,12 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli if (isExplainRequest(request)) { throw new IllegalArgumentException("Invalid request. Cannot explain cursor"); } else { - LOG.info("[{}] Cursor request {}: {}", LogUtils.getRequestId(), request.uri(), sqlRequest.cursor()); + LOG.info("[{}] Cursor request {}: {}", QueryContext.getRequestId(), request.uri(), sqlRequest.cursor()); return channel -> handleCursorRequest(request, sqlRequest.cursor(), client, channel); } } - LOG.info("[{}] Incoming request {}: {}", LogUtils.getRequestId(), request.uri(), + LOG.info("[{}] Incoming request {}: {}", QueryContext.getRequestId(), request.uri(), QueryDataAnonymizer.anonymizeData(sqlRequest.getSql())); Format format = SqlRequestParam.getFormat(request.params()); @@ -152,11 +153,11 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli sqlRequest.getSql(), request.path(), request.params()); RestChannelConsumer result = newSqlQueryHandler.prepareRequest(newSqlRequest, client); if (result != RestSQLQueryAction.NOT_SUPPORTED_YET) { - LOG.info("[{}] Request is handled by new SQL query engine", LogUtils.getRequestId()); + LOG.info("[{}] Request is handled by new SQL query engine", QueryContext.getRequestId()); return result; } LOG.debug("[{}] Request {} is not supported and falling back to old SQL engine", - LogUtils.getRequestId(), newSqlRequest); + QueryContext.getRequestId(), newSqlRequest); final QueryAction queryAction = explainRequest(client, sqlRequest, format); return channel -> executeSqlRequest(request, queryAction, client, channel); @@ -182,10 +183,10 @@ private void handleCursorRequest(final RestRequest request, final String cursor, private static void logAndPublishMetrics(final Exception e) { if (isClientError(e)) { - LOG.error(LogUtils.getRequestId() + " Client side error during query execution", e); + LOG.error(QueryContext.getRequestId() + " Client side error during query execution", e); Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_CUS).increment(); } else { - LOG.error(LogUtils.getRequestId() + " Server side error during query execution", e); + LOG.error(QueryContext.getRequestId() + " Server side error during query execution", e); Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); } } diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java index 70ec21c3fab..5b48ef67104 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java @@ -22,9 +22,9 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.legacy.executor.format.ErrorMessageFactory; import org.opensearch.sql.legacy.metrics.Metrics; -import org.opensearch.sql.legacy.utils.LogUtils; /** * Currently this interface is for node level. @@ -67,7 +67,7 @@ public List replacedRoutes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { - LogUtils.addRequestId(); + QueryContext.addRequestId(); try { return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.OK, diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/utils/LogUtils.java b/legacy/src/main/java/org/opensearch/sql/legacy/utils/LogUtils.java deleted file mode 100644 index 4830dd44135..00000000000 --- a/legacy/src/main/java/org/opensearch/sql/legacy/utils/LogUtils.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - - -package org.opensearch.sql.legacy.utils; - -import java.util.Map; -import java.util.Optional; -import java.util.UUID; -import org.apache.logging.log4j.ThreadContext; - -/** - * Utility class for generating/accessing the request id from logging context. - */ -public class LogUtils { - - /** - * The key of the request id in the context map - */ - private static final String REQUEST_ID_KEY = "request_id"; - - private static final String EMPTY_ID = "ID"; - - /** - * Generates a random UUID and adds to the {@link ThreadContext} as the request id. - *

- * Note: If a request id already present, this method will overwrite it with a new - * one. This is to pre-vent re-using the same request id for different requests in - * case the same thread handles both of them. But this also means one should not - * call this method twice on the same thread within the lifetime of the request. - *

- */ - public static void addRequestId() { - - ThreadContext.put(REQUEST_ID_KEY, UUID.randomUUID().toString()); - } - - /** - * @return the current request id from {@link ThreadContext} - */ - public static String getRequestId() { - return Optional.ofNullable(ThreadContext.get(REQUEST_ID_KEY)).orElseGet(() -> EMPTY_ID); - } - - /** - * Wraps a given instance of {@link Runnable} into a new one which gets all the - * entries from current ThreadContext map. - * - * @param task the instance of Runnable to wrap - * @return the new task - */ - public static Runnable withCurrentContext(final Runnable task) { - - final Map currentContext = ThreadContext.getImmutableContext(); - return () -> { - ThreadContext.putAll(currentContext); - task.run(); - }; - } - - private LogUtils() { - - throw new AssertionError(getClass().getCanonicalName() + " is a utility class and must not be initialized"); - } -} diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/LogUtilsTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/QueryContextTest.java similarity index 79% rename from legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/LogUtilsTest.java rename to legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/QueryContextTest.java index 564ce8c9ea7..55b78af0d7e 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/LogUtilsTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/QueryContextTest.java @@ -8,15 +8,15 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import org.apache.logging.log4j.ThreadContext; import org.junit.After; import org.junit.Assert; import org.junit.Test; -import org.opensearch.sql.legacy.utils.LogUtils; +import org.opensearch.sql.common.utils.QueryContext; -public class LogUtilsTest { +public class QueryContextTest { private static final String REQUEST_ID_KEY = "request_id"; @@ -30,7 +30,7 @@ public void cleanUpContext() { public void addRequestId() { Assert.assertNull(ThreadContext.get(REQUEST_ID_KEY)); - LogUtils.addRequestId(); + QueryContext.addRequestId(); final String requestId = ThreadContext.get(REQUEST_ID_KEY); Assert.assertNotNull(requestId); } @@ -38,16 +38,16 @@ public void addRequestId() { @Test public void addRequestId_alreadyExists() { - LogUtils.addRequestId(); + QueryContext.addRequestId(); final String requestId = ThreadContext.get(REQUEST_ID_KEY); - LogUtils.addRequestId(); + QueryContext.addRequestId(); final String requestId2 = ThreadContext.get(REQUEST_ID_KEY); Assert.assertThat(requestId2, not(equalTo(requestId))); } @Test public void getRequestId_doesNotExist() { - assertEquals("ID", LogUtils.getRequestId()); + assertNotNull(QueryContext.getRequestId()); } @Test @@ -55,7 +55,7 @@ public void getRequestId() { final String test_request_id = "test_id_111"; ThreadContext.put(REQUEST_ID_KEY, test_request_id); - final String requestId = LogUtils.getRequestId(); + final String requestId = QueryContext.getRequestId(); Assert.assertThat(requestId, equalTo(test_request_id)); } @@ -68,6 +68,6 @@ public void withCurrentContext() throws InterruptedException { }; ThreadContext.put("test11", "value11"); ThreadContext.put("test22", "value11"); - new Thread(LogUtils.withCurrentContext(task)).join(); + new Thread(QueryContext.withCurrentContext(task)).join(); } } diff --git a/opensearch/build.gradle b/opensearch/build.gradle index 2b26943d5bf..8b5f917dffd 100644 --- a/opensearch/build.gradle +++ b/opensearch/build.gradle @@ -32,9 +32,9 @@ dependencies { api project(':core') api group: 'org.opensearch', name: 'opensearch', version: "${opensearch_version}" implementation "io.github.resilience4j:resilience4j-retry:1.5.0" - implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: '2.13.3' - implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.13.3' - implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: '2.13.3' + implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: "${jackson_version}" + implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: "${jackson_version}" + implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: "${jackson_version}" implementation group: 'org.json', name: 'json', version:'20180813' compileOnly group: 'org.opensearch.client', name: 'opensearch-rest-high-level-client', version: "${opensearch_version}" implementation group: 'org.opensearch', name:'opensearch-ml-client', version: "${opensearch_build}" diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchClient.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchClient.java index c1b7d782d27..09a83f65a57 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchClient.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchClient.java @@ -30,6 +30,14 @@ public interface OpenSearchClient { */ Map getIndexMappings(String... indexExpression); + /** + * Fetch index.max_result_window settings according to index expression given. + * + * @param indexExpression index expression + * @return map from index name to its max result window + */ + Map getIndexMaxResultWindows(String... indexExpression); + /** * Perform search query in the search request. * diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java index fe262808121..db35f3580c3 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java @@ -24,11 +24,14 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.AliasMetadata; +import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.collect.ImmutableOpenMap; +import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.index.IndexSettings; import org.opensearch.sql.opensearch.mapping.IndexMapping; import org.opensearch.sql.opensearch.request.OpenSearchRequest; import org.opensearch.sql.opensearch.response.OpenSearchResponse; @@ -86,6 +89,29 @@ public Map getIndexMappings(String... indexExpression) { } } + /** + * Fetch index.max_result_window settings according to index expression given. + * + * @param indexExpression index expression + * @return map from index name to its max result window + */ + @Override + public Map getIndexMaxResultWindows(String... indexExpression) { + ClusterState state = clusterService.state(); + ImmutableOpenMap indicesMetadata = state.metadata().getIndices(); + String[] concreteIndices = resolveIndexExpression(state, indexExpression); + + ImmutableMap.Builder result = ImmutableMap.builder(); + for (String index : concreteIndices) { + Settings settings = indicesMetadata.get(index).getSettings(); + Integer maxResultWindow = settings.getAsInt("index.max_result_window", + IndexSettings.MAX_RESULT_WINDOW_SETTING.getDefault(settings)); + result.put(index, maxResultWindow); + } + + return result.build(); + } + /** * TODO: Scroll doesn't work for aggregation. Support aggregation later. */ diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java index 9da8c442e09..f354215e058 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java @@ -11,12 +11,15 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; import java.util.stream.Stream; import lombok.RequiredArgsConstructor; import org.opensearch.action.admin.cluster.settings.ClusterGetSettingsRequest; +import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; import org.opensearch.action.search.ClearScrollRequest; import org.opensearch.client.RequestOptions; import org.opensearch.client.RestHighLevelClient; @@ -26,6 +29,7 @@ import org.opensearch.client.indices.GetMappingsResponse; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.metadata.AliasMetadata; +import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.Settings; import org.opensearch.sql.opensearch.mapping.IndexMapping; import org.opensearch.sql.opensearch.request.OpenSearchRequest; @@ -54,6 +58,36 @@ public Map getIndexMappings(String... indexExpression) { } } + @Override + public Map getIndexMaxResultWindows(String... indexExpression) { + GetSettingsRequest request = new GetSettingsRequest() + .indices(indexExpression).includeDefaults(true); + try { + GetSettingsResponse response = client.indices().getSettings(request, RequestOptions.DEFAULT); + ImmutableOpenMap settings = response.getIndexToSettings(); + ImmutableOpenMap defaultSettings = response.getIndexToDefaultSettings(); + Map result = new HashMap<>(); + + defaultSettings.forEach(entry -> { + Integer maxResultWindow = entry.value.getAsInt("index.max_result_window", null); + if (maxResultWindow != null) { + result.put(entry.key, maxResultWindow); + } + }); + + settings.forEach(entry -> { + Integer maxResultWindow = entry.value.getAsInt("index.max_result_window", null); + if (maxResultWindow != null) { + result.put(entry.key, maxResultWindow); + } + }); + + return result; + } catch (IOException e) { + throw new IllegalStateException("Failed to get max result window for " + indexExpression, e); + } + } + @Override public OpenSearchResponse search(OpenSearchRequest request) { return request.search( diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java index 5ca3670ca16..6f6fea841b6 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java @@ -49,7 +49,7 @@ public class OpenSearchQueryRequest implements OpenSearchRequest { /** - * ElasticsearchExprValueFactory. + * OpenSearchExprValueFactory. */ @EqualsAndHashCode.Exclude @ToString.Exclude @@ -61,7 +61,7 @@ public class OpenSearchQueryRequest implements OpenSearchRequest { private boolean searchDone = false; /** - * Constructor of ElasticsearchQueryRequest. + * Constructor of OpenSearchQueryRequest. */ public OpenSearchQueryRequest(String indexName, int size, OpenSearchExprValueFactory factory) { @@ -69,7 +69,7 @@ public OpenSearchQueryRequest(String indexName, int size, } /** - * Constructor of ElasticsearchQueryRequest. + * Constructor of OpenSearchQueryRequest. */ public OpenSearchQueryRequest(IndexName indexName, int size, OpenSearchExprValueFactory factory) { @@ -81,6 +81,16 @@ public OpenSearchQueryRequest(IndexName indexName, int size, this.exprValueFactory = factory; } + /** + * Constructor of OpenSearchQueryRequest. + */ + public OpenSearchQueryRequest(IndexName indexName, SearchSourceBuilder sourceBuilder, + OpenSearchExprValueFactory factory) { + this.indexName = indexName; + this.sourceBuilder = sourceBuilder; + this.exprValueFactory = factory; + } + @Override public OpenSearchResponse search(Function searchAction, Function scrollAction) { diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java new file mode 100644 index 00000000000..646395d790c --- /dev/null +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java @@ -0,0 +1,202 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.opensearch.request; + +import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME; +import static org.opensearch.search.sort.SortOrder.ASC; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import org.apache.commons.lang3.tuple.Pair; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.search.aggregations.AggregationBuilder; +import org.opensearch.search.builder.SearchSourceBuilder; +import org.opensearch.search.fetch.subphase.highlight.HighlightBuilder; +import org.opensearch.search.sort.SortBuilder; +import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.common.utils.StringUtils; +import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.expression.ReferenceExpression; +import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory; +import org.opensearch.sql.opensearch.response.agg.OpenSearchAggregationResponseParser; + +/** + * OpenSearch search request builder. + */ +@EqualsAndHashCode +@Getter +@ToString +public class OpenSearchRequestBuilder { + + /** + * Default query timeout in minutes. + */ + public static final TimeValue DEFAULT_QUERY_TIMEOUT = TimeValue.timeValueMinutes(1L); + + /** + * {@link OpenSearchRequest.IndexName}. + */ + private final OpenSearchRequest.IndexName indexName; + + /** + * Index max result window. + */ + private final Integer maxResultWindow; + + /** + * Search request source builder. + */ + private final SearchSourceBuilder sourceBuilder; + + /** + * OpenSearchExprValueFactory. + */ + @EqualsAndHashCode.Exclude + @ToString.Exclude + private final OpenSearchExprValueFactory exprValueFactory; + + /** + * Query size of the request. + */ + private Integer querySize; + + public OpenSearchRequestBuilder(String indexName, + Integer maxResultWindow, + Settings settings, + OpenSearchExprValueFactory exprValueFactory) { + this(new OpenSearchRequest.IndexName(indexName), maxResultWindow, settings, exprValueFactory); + } + + /** + * Constructor. + */ + public OpenSearchRequestBuilder(OpenSearchRequest.IndexName indexName, + Integer maxResultWindow, + Settings settings, + OpenSearchExprValueFactory exprValueFactory) { + this.indexName = indexName; + this.maxResultWindow = maxResultWindow; + this.sourceBuilder = new SearchSourceBuilder(); + this.exprValueFactory = exprValueFactory; + this.querySize = settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT); + sourceBuilder.from(0); + sourceBuilder.size(querySize); + sourceBuilder.timeout(DEFAULT_QUERY_TIMEOUT); + } + + /** + * Build DSL request. + * + * @return query request or scroll request + */ + public OpenSearchRequest build() { + Integer from = sourceBuilder.from(); + Integer size = sourceBuilder.size(); + + if (from + size <= maxResultWindow) { + return new OpenSearchQueryRequest(indexName, sourceBuilder, exprValueFactory); + } else { + sourceBuilder.size(maxResultWindow - from); + return new OpenSearchScrollRequest(indexName, sourceBuilder, exprValueFactory); + } + } + + /** + * Push down query to DSL request. + * + * @param query query request + */ + public void pushDown(QueryBuilder query) { + QueryBuilder current = sourceBuilder.query(); + + if (current == null) { + sourceBuilder.query(query); + } else { + if (isBoolFilterQuery(current)) { + ((BoolQueryBuilder) current).filter(query); + } else { + sourceBuilder.query(QueryBuilders.boolQuery() + .filter(current) + .filter(query)); + } + } + + if (sourceBuilder.sorts() == null) { + sourceBuilder.sort(DOC_FIELD_NAME, ASC); // Make sure consistent order + } + } + + /** + * Push down aggregation to DSL request. + * + * @param aggregationBuilder pair of aggregation query and aggregation parser. + */ + public void pushDownAggregation( + Pair, OpenSearchAggregationResponseParser> aggregationBuilder) { + aggregationBuilder.getLeft().forEach(builder -> sourceBuilder.aggregation(builder)); + sourceBuilder.size(0); + exprValueFactory.setParser(aggregationBuilder.getRight()); + } + + /** + * Push down sort to DSL request. + * + * @param sortBuilders sortBuilders. + */ + public void pushDownSort(List> sortBuilders) { + for (SortBuilder sortBuilder : sortBuilders) { + sourceBuilder.sort(sortBuilder); + } + } + + /** + * Push down size (limit) and from (offset) to DSL request. + */ + public void pushDownLimit(Integer limit, Integer offset) { + querySize = limit; + sourceBuilder.from(offset).size(limit); + } + + /** + * Add highlight to DSL requests. + * @param field name of the field to highlight + */ + public void pushDownHighlight(String field) { + if (sourceBuilder.highlighter() != null) { + sourceBuilder.highlighter().field(StringUtils.unquoteText(field)); + } else { + HighlightBuilder highlightBuilder = + new HighlightBuilder().field(StringUtils.unquoteText(field)); + sourceBuilder.highlighter(highlightBuilder); + } + } + + /** + * Push down project list to DSL requets. + */ + public void pushDownProjects(Set projects) { + final Set projectsSet = + projects.stream().map(ReferenceExpression::getAttr).collect(Collectors.toSet()); + sourceBuilder.fetchSource(projectsSet.toArray(new String[0]), new String[0]); + } + + public void pushTypeMapping(Map typeMapping) { + exprValueFactory.setTypeMapping(typeMapping); + } + + private boolean isBoolFilterQuery(QueryBuilder current) { + return (current instanceof BoolQueryBuilder); + } +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java index ebbebcd8ebe..4509e443c0a 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java @@ -54,10 +54,12 @@ public class OpenSearchScrollRequest implements OpenSearchRequest { private String scrollId; /** Search request source builder. */ - private final SearchSourceBuilder sourceBuilder = new SearchSourceBuilder(); + private final SearchSourceBuilder sourceBuilder; + /** Constructor. */ public OpenSearchScrollRequest(IndexName indexName, OpenSearchExprValueFactory exprValueFactory) { this.indexName = indexName; + this.sourceBuilder = new SearchSourceBuilder(); this.exprValueFactory = exprValueFactory; } @@ -65,6 +67,16 @@ public OpenSearchScrollRequest(String indexName, OpenSearchExprValueFactory expr this(new IndexName(indexName), exprValueFactory); } + /** Constructor. */ + public OpenSearchScrollRequest(IndexName indexName, + SearchSourceBuilder sourceBuilder, + OpenSearchExprValueFactory exprValueFactory) { + this.indexName = indexName; + this.sourceBuilder = sourceBuilder; + this.exprValueFactory = exprValueFactory; + } + + /** Constructor. */ @Override public OpenSearchResponse search(Function searchAction, Function scrollAction) { diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java index 5c6d3687c6a..f3214970997 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java @@ -121,6 +121,16 @@ public Map getFieldTypes() { return fieldTypes; } + /** + * Get the minimum of the max result windows of the indices. + * + * @return max result window + */ + public Integer getMaxResultWindow() { + return client.getIndexMaxResultWindows(indexName.getIndexNames()) + .values().stream().min(Integer::compare).get(); + } + private ExprType transformESTypeToExprType(String openSearchType) { return OPENSEARCH_TYPE_TO_EXPR_TYPE_MAPPING.getOrDefault(openSearchType, ExprCoreType.UNKNOWN); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java index c028f283a27..ef6159020f9 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java @@ -58,6 +58,11 @@ public class OpenSearchIndex implements Table { */ private Map cachedFieldTypes = null; + /** + * The cached max result window setting of index. + */ + private Integer cachedMaxResultWindow = null; + /** * Constructor. */ @@ -80,13 +85,24 @@ public Map getFieldTypes() { return cachedFieldTypes; } + /** + * Get the max result window setting of the table. + */ + public Integer getMaxResultWindow() { + if (cachedMaxResultWindow == null) { + cachedMaxResultWindow = + new OpenSearchDescribeIndexRequest(client, indexName).getMaxResultWindow(); + } + return cachedMaxResultWindow; + } + /** * TODO: Push down operations to index scan operator as much as possible in future. */ @Override public PhysicalPlan implement(LogicalPlan plan) { OpenSearchIndexScan indexScan = new OpenSearchIndexScan(client, settings, indexName, - new OpenSearchExprValueFactory(getFieldTypes())); + getMaxResultWindow(), new OpenSearchExprValueFactory(getFieldTypes())); /* * Visit logical plan with index scan as context so logical operators visited, such as @@ -128,7 +144,7 @@ public PhysicalPlan visitIndexScan(OpenSearchLogicalIndexScan node, OpenSearchIndexScan context) { if (null != node.getSortList()) { final SortQueryBuilder builder = new SortQueryBuilder(); - context.pushDownSort(node.getSortList().stream() + context.getRequestBuilder().pushDownSort(node.getSortList().stream() .map(sort -> builder.build(sort.getValue(), sort.getKey())) .collect(Collectors.toList())); } @@ -136,15 +152,15 @@ public PhysicalPlan visitIndexScan(OpenSearchLogicalIndexScan node, if (null != node.getFilter()) { FilterQueryBuilder queryBuilder = new FilterQueryBuilder(new DefaultExpressionSerializer()); QueryBuilder query = queryBuilder.build(node.getFilter()); - context.pushDown(query); + context.getRequestBuilder().pushDown(query); } if (node.getLimit() != null) { - context.pushDownLimit(node.getLimit(), node.getOffset()); + context.getRequestBuilder().pushDownLimit(node.getLimit(), node.getOffset()); } if (node.hasProjects()) { - context.pushDownProjects(node.getProjectList()); + context.getRequestBuilder().pushDownProjects(node.getProjectList()); } return indexScan; } @@ -158,15 +174,15 @@ public PhysicalPlan visitIndexAggregation(OpenSearchLogicalIndexAgg node, FilterQueryBuilder queryBuilder = new FilterQueryBuilder( new DefaultExpressionSerializer()); QueryBuilder query = queryBuilder.build(node.getFilter()); - context.pushDown(query); + context.getRequestBuilder().pushDown(query); } AggregationQueryBuilder builder = new AggregationQueryBuilder(new DefaultExpressionSerializer()); Pair, OpenSearchAggregationResponseParser> aggregationBuilder = builder.buildAggregationBuilder(node.getAggregatorList(), node.getGroupByList(), node.getSortList()); - context.pushDownAggregation(aggregationBuilder); - context.pushTypeMapping( + context.getRequestBuilder().pushDownAggregation(aggregationBuilder); + context.getRequestBuilder().pushTypeMapping( builder.buildTypeMapping(node.getAggregatorList(), node.getGroupByList())); return indexScan; @@ -191,7 +207,7 @@ public PhysicalPlan visitAD(LogicalAD node, OpenSearchIndexScan context) { @Override public PhysicalPlan visitHighlight(LogicalHighlight node, OpenSearchIndexScan context) { - context.pushDownHighlight(node.getHighlightField().toString()); + context.getRequestBuilder().pushDownHighlight(node.getHighlightField().toString()); return visitChild(node, context); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScan.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScan.java index 6e88f3de891..e9746e1fae1 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScan.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScan.java @@ -6,38 +6,18 @@ package org.opensearch.sql.opensearch.storage; -import static org.opensearch.search.sort.FieldSortBuilder.DOC_FIELD_NAME; -import static org.opensearch.search.sort.SortOrder.ASC; - -import com.google.common.collect.Iterables; -import java.util.ArrayList; +import java.util.Collections; import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.ToString; -import org.apache.commons.lang3.tuple.Pair; -import org.opensearch.index.query.BoolQueryBuilder; -import org.opensearch.index.query.QueryBuilder; -import org.opensearch.index.query.QueryBuilders; -import org.opensearch.search.aggregations.AggregationBuilder; -import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.search.fetch.subphase.highlight.HighlightBuilder; -import org.opensearch.search.sort.SortBuilder; import org.opensearch.sql.common.setting.Settings; -import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.type.ExprType; -import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.opensearch.client.OpenSearchClient; import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory; -import org.opensearch.sql.opensearch.request.OpenSearchQueryRequest; import org.opensearch.sql.opensearch.request.OpenSearchRequest; +import org.opensearch.sql.opensearch.request.OpenSearchRequestBuilder; import org.opensearch.sql.opensearch.response.OpenSearchResponse; -import org.opensearch.sql.opensearch.response.agg.OpenSearchAggregationResponseParser; import org.opensearch.sql.storage.TableScanOperator; /** @@ -50,11 +30,24 @@ public class OpenSearchIndexScan extends TableScanOperator { /** OpenSearch client. */ private final OpenSearchClient client; - /** Search request. */ + /** Search request builder. */ @EqualsAndHashCode.Include @Getter @ToString.Include - private final OpenSearchRequest request; + private final OpenSearchRequestBuilder requestBuilder; + + /** Search request. */ + @EqualsAndHashCode.Include + @ToString.Include + private OpenSearchRequest request; + + /** Total query size. */ + @EqualsAndHashCode.Include + @ToString.Include + private Integer querySize; + + /** Number of rows returned. */ + private Integer queryCount; /** Search response for current batch. */ private Iterator iterator; @@ -62,133 +55,57 @@ public class OpenSearchIndexScan extends TableScanOperator { /** * Constructor. */ - public OpenSearchIndexScan(OpenSearchClient client, - Settings settings, String indexName, + public OpenSearchIndexScan(OpenSearchClient client, Settings settings, + String indexName, Integer maxResultWindow, OpenSearchExprValueFactory exprValueFactory) { - this(client, settings, new OpenSearchRequest.IndexName(indexName), exprValueFactory); + this(client, settings, + new OpenSearchRequest.IndexName(indexName),maxResultWindow, exprValueFactory); } /** * Constructor. */ - public OpenSearchIndexScan(OpenSearchClient client, - Settings settings, OpenSearchRequest.IndexName indexName, - OpenSearchExprValueFactory exprValueFactory) { + public OpenSearchIndexScan(OpenSearchClient client, Settings settings, + OpenSearchRequest.IndexName indexName, Integer maxResultWindow, + OpenSearchExprValueFactory exprValueFactory) { this.client = client; - this.request = new OpenSearchQueryRequest(indexName, - settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT), exprValueFactory); + this.requestBuilder = new OpenSearchRequestBuilder( + indexName, maxResultWindow, settings,exprValueFactory); } @Override public void open() { super.open(); - - // For now pull all results immediately once open - List responses = new ArrayList<>(); - OpenSearchResponse response = client.search(request); - while (!response.isEmpty()) { - responses.add(response); - response = client.search(request); - } - iterator = Iterables.concat(responses.toArray(new OpenSearchResponse[0])).iterator(); + querySize = requestBuilder.getQuerySize(); + request = requestBuilder.build(); + iterator = Collections.emptyIterator(); + queryCount = 0; + fetchNextBatch(); } @Override public boolean hasNext() { + if (queryCount >= querySize) { + iterator = Collections.emptyIterator(); + } else if (!iterator.hasNext()) { + fetchNextBatch(); + } return iterator.hasNext(); } @Override public ExprValue next() { + queryCount++; return iterator.next(); } - /** - * Push down query to DSL request. - * @param query query request - */ - public void pushDown(QueryBuilder query) { - SearchSourceBuilder source = request.getSourceBuilder(); - QueryBuilder current = source.query(); - - if (current == null) { - source.query(query); - } else { - if (isBoolFilterQuery(current)) { - ((BoolQueryBuilder) current).filter(query); - } else { - source.query(QueryBuilders.boolQuery() - .filter(current) - .filter(query)); - } - } - - if (source.sorts() == null) { - source.sort(DOC_FIELD_NAME, ASC); // Make sure consistent order - } - } - - /** - * Push down aggregation to DSL request. - * @param aggregationBuilder pair of aggregation query and aggregation parser. - */ - public void pushDownAggregation( - Pair, OpenSearchAggregationResponseParser> aggregationBuilder) { - SearchSourceBuilder source = request.getSourceBuilder(); - aggregationBuilder.getLeft().forEach(builder -> source.aggregation(builder)); - source.size(0); - request.getExprValueFactory().setParser(aggregationBuilder.getRight()); - } - - /** - * Push down sort to DSL request. - * - * @param sortBuilders sortBuilders. - */ - public void pushDownSort(List> sortBuilders) { - SearchSourceBuilder source = request.getSourceBuilder(); - for (SortBuilder sortBuilder : sortBuilders) { - source.sort(sortBuilder); - } - } - - /** - * Push down size (limit) and from (offset) to DSL request. - */ - public void pushDownLimit(Integer limit, Integer offset) { - SearchSourceBuilder sourceBuilder = request.getSourceBuilder(); - sourceBuilder.from(offset).size(limit); - } - - /** - * Add highlight to DSL requests. - * @param field name of the field to highlight - */ - public void pushDownHighlight(String field) { - SearchSourceBuilder sourceBuilder = request.getSourceBuilder(); - if (sourceBuilder.highlighter() != null) { - sourceBuilder.highlighter().field(StringUtils.unquoteText(field)); - } else { - HighlightBuilder highlightBuilder = - new HighlightBuilder().field(StringUtils.unquoteText(field)); - sourceBuilder.highlighter(highlightBuilder); + private void fetchNextBatch() { + OpenSearchResponse response = client.search(request); + if (!response.isEmpty()) { + iterator = response.iterator(); } } - /** - * Push down project list to DSL requets. - */ - public void pushDownProjects(Set projects) { - SearchSourceBuilder sourceBuilder = request.getSourceBuilder(); - final Set projectsSet = - projects.stream().map(ReferenceExpression::getAttr).collect(Collectors.toSet()); - sourceBuilder.fetchSource(projectsSet.toArray(new String[0]), new String[0]); - } - - public void pushTypeMapping(Map typeMapping) { - request.getExprValueFactory().setTypeMapping(typeMapping); - } - @Override public void close() { super.close(); @@ -196,12 +113,8 @@ public void close() { client.cleanup(request); } - private boolean isBoolFilterQuery(QueryBuilder current) { - return (current instanceof BoolQueryBuilder); - } - @Override public String explain() { - return getRequest().toString(); + return getRequestBuilder().build().toString(); } } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java index 50410e07ccd..8fdb93427b7 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.net.URL; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -72,6 +73,7 @@ class OpenSearchNodeClientTest { private static final String TEST_MAPPING_FILE = "mappings/accounts.json"; + private static final String TEST_MAPPING_SETTINGS_FILE = "mappings/accounts2.json"; @Mock(answer = RETURNS_DEEP_STUBS) private NodeClient nodeClient; @@ -151,6 +153,36 @@ public void getIndexMappingsWithNonExistIndex() { assertThrows(IndexNotFoundException.class, () -> client.getIndexMappings("non_exist_index")); } + @Test + public void getIndexMaxResultWindows() throws IOException { + URL url = Resources.getResource(TEST_MAPPING_SETTINGS_FILE); + String mappings = Resources.toString(url, Charsets.UTF_8); + String indexName = "accounts"; + ClusterService clusterService = mockClusterServiceForSettings(indexName, mappings); + OpenSearchNodeClient client = new OpenSearchNodeClient(clusterService, nodeClient); + + Map indexMaxResultWindows = client.getIndexMaxResultWindows(indexName); + assertEquals(1, indexMaxResultWindows.size()); + + Integer indexMaxResultWindow = indexMaxResultWindows.values().iterator().next(); + assertEquals(100, indexMaxResultWindow); + } + + @Test + public void getIndexMaxResultWindowsWithDefaultSettings() throws IOException { + URL url = Resources.getResource(TEST_MAPPING_FILE); + String mappings = Resources.toString(url, Charsets.UTF_8); + String indexName = "accounts"; + ClusterService clusterService = mockClusterServiceForSettings(indexName, mappings); + OpenSearchNodeClient client = new OpenSearchNodeClient(clusterService, nodeClient); + + Map indexMaxResultWindows = client.getIndexMaxResultWindows(indexName); + assertEquals(1, indexMaxResultWindows.size()); + + Integer indexMaxResultWindow = indexMaxResultWindows.values().iterator().next(); + assertEquals(10000, indexMaxResultWindow); + } + /** Jacoco enforce this constant lambda be tested. */ @Test public void testAllFieldsPredicate() { @@ -353,6 +385,33 @@ public ClusterService mockClusterService(String indexName, Throwable t) { return mockService; } + public ClusterService mockClusterServiceForSettings(String indexName, String mappings) { + ClusterService mockService = mock(ClusterService.class); + ClusterState mockState = mock(ClusterState.class); + Metadata mockMetaData = mock(Metadata.class); + + when(mockService.state()).thenReturn(mockState); + when(mockState.metadata()).thenReturn(mockMetaData); + try { + ImmutableOpenMap.Builder indexBuilder = + ImmutableOpenMap.builder(); + IndexMetadata indexMetadata = IndexMetadata.fromXContent(createParser(mappings)); + + indexBuilder.put(indexName, indexMetadata); + when(mockMetaData.getIndices()).thenReturn(indexBuilder.build()); + + // IndexNameExpressionResolver use this method to check if index exists. If not, + // IndexNotFoundException is thrown. + IndexAbstraction indexAbstraction = mock(IndexAbstraction.class); + when(indexAbstraction.getIndices()).thenReturn(Collections.singletonList(indexMetadata)); + when(mockMetaData.getIndicesLookup()) + .thenReturn(ImmutableSortedMap.of(indexName, indexAbstraction)); + } catch (IOException e) { + throw new IllegalStateException("Failed to mock cluster service", e); + } + return mockService; + } + private XContentParser createParser(String mappings) throws IOException { return XContentType.JSON .xContent() diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java index 0c2503ea57a..bc334aaf399 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java @@ -34,6 +34,8 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.action.admin.cluster.settings.ClusterGetSettingsResponse; +import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; import org.opensearch.action.search.SearchResponse; import org.opensearch.client.RequestOptions; import org.opensearch.client.RestHighLevelClient; @@ -43,6 +45,7 @@ import org.opensearch.client.indices.GetMappingsResponse; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.MappingMetadata; +import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.DeprecationHandler; import org.opensearch.common.xcontent.NamedXContentRegistry; @@ -128,6 +131,61 @@ void getIndexMappingsWithIOException() throws IOException { assertThrows(IllegalStateException.class, () -> client.getIndexMappings("test")); } + @Test + void getIndexMaxResultWindowsSettings() throws IOException { + String indexName = "test"; + Integer maxResultWindow = 1000; + + GetSettingsResponse response = mock(GetSettingsResponse.class); + Settings maxResultWindowSettings = Settings.builder() + .put("index.max_result_window", maxResultWindow) + .build(); + Settings emptySettings = Settings.builder().build(); + ImmutableOpenMap indexToSettings = + mockSettings(indexName, maxResultWindowSettings); + ImmutableOpenMap indexToDefaultSettings = + mockSettings(indexName, emptySettings); + when(response.getIndexToSettings()).thenReturn(indexToSettings); + when(response.getIndexToDefaultSettings()).thenReturn(indexToDefaultSettings); + when(restClient.indices().getSettings(any(GetSettingsRequest.class), any())) + .thenReturn(response); + + Map indexMaxResultWindows = client.getIndexMaxResultWindows(indexName); + assertEquals(1, indexMaxResultWindows.size()); + assertEquals(maxResultWindow, indexMaxResultWindows.values().iterator().next()); + } + + @Test + void getIndexMaxResultWindowsDefaultSettings() throws IOException { + String indexName = "test"; + Integer maxResultWindow = 10000; + + GetSettingsResponse response = mock(GetSettingsResponse.class); + Settings maxResultWindowSettings = Settings.builder() + .put("index.max_result_window", maxResultWindow) + .build(); + Settings emptySettings = Settings.builder().build(); + ImmutableOpenMap indexToSettings = + mockSettings(indexName, emptySettings); + ImmutableOpenMap indexToDefaultSettings = + mockSettings(indexName, maxResultWindowSettings); + when(response.getIndexToSettings()).thenReturn(indexToSettings); + when(response.getIndexToDefaultSettings()).thenReturn(indexToDefaultSettings); + when(restClient.indices().getSettings(any(GetSettingsRequest.class), any())) + .thenReturn(response); + + Map indexMaxResultWindows = client.getIndexMaxResultWindows(indexName); + assertEquals(1, indexMaxResultWindows.size()); + assertEquals(maxResultWindow, indexMaxResultWindows.values().iterator().next()); + } + + @Test + void getIndexMaxResultWindowsWithIOException() throws IOException { + when(restClient.indices().getSettings(any(GetSettingsRequest.class), any())) + .thenThrow(new IOException()); + assertThrows(IllegalStateException.class, () -> client.getIndexMaxResultWindows("test")); + } + @Test void search() throws IOException { // Mock first scroll request @@ -277,6 +335,12 @@ private Map mockFieldMappings(String indexName, String return ImmutableMap.of(indexName, IndexMetadata.fromXContent(createParser(mappings)).mapping()); } + private ImmutableOpenMap mockSettings(String indexName, Settings settings) { + ImmutableOpenMap.Builder indexToSettingsBuilder = ImmutableOpenMap.builder(); + indexToSettingsBuilder.put(indexName, settings); + return indexToSettingsBuilder.build(); + } + private XContentParser createParser(String mappings) throws IOException { return XContentType.JSON .xContent() diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java index 24c305a75ef..f1a0a7d5d76 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import lombok.RequiredArgsConstructor; import org.junit.jupiter.api.BeforeEach; @@ -126,7 +127,7 @@ void explainSuccessfully() { Settings settings = mock(Settings.class); when(settings.getSettingValue(QUERY_SIZE_LIMIT)).thenReturn(100); PhysicalPlan plan = new OpenSearchIndexScan(mock(OpenSearchClient.class), - settings, "test", mock(OpenSearchExprValueFactory.class)); + settings, "test", 10000, mock(OpenSearchExprValueFactory.class)); AtomicReference result = new AtomicReference<>(); executor.explain(plan, new ResponseListener() { diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java index ee981a4abca..e5c5046b817 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java @@ -88,6 +88,7 @@ public void testProtectIndexScan() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); String indexName = "test"; + Integer maxResultWindow = 10000; NamedExpression include = named("age", ref("age", INTEGER)); ReferenceExpression exclude = ref("name", STRING); ReferenceExpression dedupeField = ref("name", STRING); @@ -124,7 +125,7 @@ public void testProtectIndexScan() { resourceMonitor( new OpenSearchIndexScan( client, settings, indexName, - exprValueFactory)), + maxResultWindow, exprValueFactory)), filterExpr), aggregators, groupByExprs), @@ -152,7 +153,7 @@ public void testProtectIndexScan() { filter( new OpenSearchIndexScan( client, settings, indexName, - exprValueFactory), + maxResultWindow, exprValueFactory), filterExpr), aggregators, groupByExprs), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java new file mode 100644 index 00000000000..43b93531906 --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java @@ -0,0 +1,76 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.opensearch.request; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.when; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.search.builder.SearchSourceBuilder; +import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory; + +@ExtendWith(MockitoExtension.class) +public class OpenSearchRequestBuilderTest { + + public static final TimeValue DEFAULT_QUERY_TIMEOUT = TimeValue.timeValueMinutes(1L); + @Mock + private Settings settings; + + @Mock + private OpenSearchExprValueFactory factory; + + @BeforeEach + void setup() { + when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + } + + @Test + void buildQueryRequest() { + Integer maxResultWindow = 500; + Integer limit = 200; + Integer offset = 0; + OpenSearchRequestBuilder builder = + new OpenSearchRequestBuilder("test", maxResultWindow, settings, factory); + builder.pushDownLimit(limit, offset); + + assertEquals( + new OpenSearchQueryRequest( + new OpenSearchRequest.IndexName("test"), + new SearchSourceBuilder() + .from(offset) + .size(limit) + .timeout(DEFAULT_QUERY_TIMEOUT), + factory), + builder.build()); + } + + @Test + void buildScrollRequestWithCorrectSize() { + Integer maxResultWindow = 500; + Integer limit = 800; + Integer offset = 10; + OpenSearchRequestBuilder builder = + new OpenSearchRequestBuilder("test", maxResultWindow, settings, factory); + builder.pushDownLimit(limit, offset); + + assertEquals( + new OpenSearchScrollRequest( + new OpenSearchRequest.IndexName("test"), + new SearchSourceBuilder() + .from(offset) + .size(maxResultWindow - offset) + .timeout(DEFAULT_QUERY_TIMEOUT), + factory), + builder.build()); + } +} diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchDefaultImplementorTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchDefaultImplementorTest.java index c83172955c1..b85d60c1fb4 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchDefaultImplementorTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchDefaultImplementorTest.java @@ -20,6 +20,7 @@ import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.sql.opensearch.client.OpenSearchClient; +import org.opensearch.sql.opensearch.request.OpenSearchRequestBuilder; import org.opensearch.sql.planner.logical.LogicalAD; import org.opensearch.sql.planner.logical.LogicalHighlight; import org.opensearch.sql.planner.logical.LogicalMLCommons; @@ -76,10 +77,12 @@ public void visitHighlight() { LogicalHighlight node = Mockito.mock(LogicalHighlight.class, Answers.RETURNS_DEEP_STUBS); Mockito.when(node.getChild().get(0)).thenReturn(Mockito.mock(LogicalPlan.class)); + OpenSearchRequestBuilder requestBuilder = Mockito.mock(OpenSearchRequestBuilder.class); + Mockito.when(indexScan.getRequestBuilder()).thenReturn(requestBuilder); OpenSearchIndex.OpenSearchDefaultImplementor implementor = new OpenSearchIndex.OpenSearchDefaultImplementor(indexScan, client); implementor.visitHighlight(node, indexScan); - verify(indexScan).pushDownHighlight(any()); + verify(requestBuilder).pushDownHighlight(any()); } } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java index 41769914d95..a1f2869ca51 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java @@ -62,7 +62,7 @@ void setup() { void queryEmptyResult() { mockResponse(); try (OpenSearchIndexScan indexScan = - new OpenSearchIndexScan(client, settings, "test", exprValueFactory)) { + new OpenSearchIndexScan(client, settings, "test", 3, exprValueFactory)) { indexScan.open(); assertFalse(indexScan.hasNext()); } @@ -70,13 +70,90 @@ void queryEmptyResult() { } @Test - void queryAllResults() { + void queryAllResultsWithQuery() { + mockResponse(new ExprValue[]{ + employee(1, "John", "IT"), + employee(2, "Smith", "HR"), + employee(3, "Allen", "IT")}); + + try (OpenSearchIndexScan indexScan = + new OpenSearchIndexScan(client, settings, "employees", 10, exprValueFactory)) { + indexScan.open(); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(1, "John", "IT"), indexScan.next()); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(2, "Smith", "HR"), indexScan.next()); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(3, "Allen", "IT"), indexScan.next()); + + assertFalse(indexScan.hasNext()); + } + verify(client).cleanup(any()); + } + + @Test + void queryAllResultsWithScroll() { mockResponse( new ExprValue[]{employee(1, "John", "IT"), employee(2, "Smith", "HR")}, new ExprValue[]{employee(3, "Allen", "IT")}); try (OpenSearchIndexScan indexScan = - new OpenSearchIndexScan(client, settings, "employees", exprValueFactory)) { + new OpenSearchIndexScan(client, settings, "employees", 2, exprValueFactory)) { + indexScan.open(); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(1, "John", "IT"), indexScan.next()); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(2, "Smith", "HR"), indexScan.next()); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(3, "Allen", "IT"), indexScan.next()); + + assertFalse(indexScan.hasNext()); + } + verify(client).cleanup(any()); + } + + @Test + void querySomeResultsWithQuery() { + mockResponse(new ExprValue[]{ + employee(1, "John", "IT"), + employee(2, "Smith", "HR"), + employee(3, "Allen", "IT"), + employee(4, "Bob", "HR")}); + + try (OpenSearchIndexScan indexScan = + new OpenSearchIndexScan(client, settings, "employees", 10, exprValueFactory)) { + indexScan.getRequestBuilder().pushDownLimit(3, 0); + indexScan.open(); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(1, "John", "IT"), indexScan.next()); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(2, "Smith", "HR"), indexScan.next()); + + assertTrue(indexScan.hasNext()); + assertEquals(employee(3, "Allen", "IT"), indexScan.next()); + + assertFalse(indexScan.hasNext()); + } + verify(client).cleanup(any()); + } + + @Test + void querySomeResultsWithScroll() { + mockResponse( + new ExprValue[]{employee(1, "John", "IT"), employee(2, "Smith", "HR")}, + new ExprValue[]{employee(3, "Allen", "IT"), employee(4, "Bob", "HR")}); + + try (OpenSearchIndexScan indexScan = + new OpenSearchIndexScan(client, settings, "employees", 2, exprValueFactory)) { + indexScan.getRequestBuilder().pushDownLimit(3, 0); indexScan.open(); assertTrue(indexScan.hasNext()); @@ -135,19 +212,19 @@ public PushDownAssertion(OpenSearchClient client, OpenSearchExprValueFactory valueFactory, Settings settings) { this.client = client; - this.indexScan = new OpenSearchIndexScan(client, settings, "test", valueFactory); + this.indexScan = new OpenSearchIndexScan(client, settings, "test", 10000, valueFactory); this.response = mock(OpenSearchResponse.class); this.factory = valueFactory; when(response.isEmpty()).thenReturn(true); } PushDownAssertion pushDown(QueryBuilder query) { - indexScan.pushDown(query); + indexScan.getRequestBuilder().pushDown(query); return this; } PushDownAssertion pushDownHighlight(String query) { - indexScan.pushDownHighlight(query); + indexScan.getRequestBuilder().pushDownHighlight(query); return this; } @@ -187,10 +264,8 @@ public OpenSearchResponse answer(InvocationOnMock invocation) { when(response.isEmpty()).thenReturn(false); ExprValue[] searchHit = searchHitBatches[batchNum]; when(response.iterator()).thenReturn(Arrays.asList(searchHit).iterator()); - } else if (batchNum == totalBatch) { - when(response.isEmpty()).thenReturn(true); } else { - fail("Search request after empty response returned already"); + when(response.isEmpty()).thenReturn(true); } batchNum++; diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java index 847ac8dfc03..f1754a455dd 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java @@ -70,7 +70,6 @@ import org.opensearch.sql.planner.physical.PhysicalPlan; import org.opensearch.sql.planner.physical.PhysicalPlanDSL; import org.opensearch.sql.planner.physical.ProjectOperator; -import org.opensearch.sql.storage.Table; @ExtendWith(MockitoExtension.class) class OpenSearchIndexTest { @@ -109,7 +108,7 @@ void getFieldTypes() { .put("blob", "binary") .build()))); - Table index = new OpenSearchIndex(client, settings, "test"); + OpenSearchIndex index = new OpenSearchIndex(client, settings, "test"); Map fieldTypes = index.getFieldTypes(); assertThat( fieldTypes, @@ -134,30 +133,35 @@ void getFieldTypes() { @Test void implementRelationOperatorOnly() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); String indexName = "test"; LogicalPlan plan = relation(indexName); - Table index = new OpenSearchIndex(client, settings, indexName); + OpenSearchIndex index = new OpenSearchIndex(client, settings, indexName); + Integer maxResultWindow = index.getMaxResultWindow(); assertEquals( - new OpenSearchIndexScan(client, settings, indexName, exprValueFactory), + new OpenSearchIndexScan(client, settings, indexName, maxResultWindow, exprValueFactory), index.implement(plan)); } @Test void implementRelationOperatorWithOptimization() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); String indexName = "test"; LogicalPlan plan = relation(indexName); - Table index = new OpenSearchIndex(client, settings, indexName); + OpenSearchIndex index = new OpenSearchIndex(client, settings, indexName); + Integer maxResultWindow = index.getMaxResultWindow(); assertEquals( - new OpenSearchIndexScan(client, settings, indexName, exprValueFactory), + new OpenSearchIndexScan(client, settings, indexName, maxResultWindow, exprValueFactory), index.implement(index.optimize(plan))); } @Test void implementOtherLogicalOperators() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); String indexName = "test"; NamedExpression include = named("age", ref("age", INTEGER)); @@ -191,7 +195,8 @@ void implementOtherLogicalOperators() { dedupeField), include); - Table index = new OpenSearchIndex(client, settings, indexName); + OpenSearchIndex index = new OpenSearchIndex(client, settings, indexName); + Integer maxResultWindow = index.getMaxResultWindow(); assertEquals( PhysicalPlanDSL.project( PhysicalPlanDSL.dedupe( @@ -200,7 +205,7 @@ void implementOtherLogicalOperators() { PhysicalPlanDSL.remove( PhysicalPlanDSL.rename( new OpenSearchIndexScan(client, settings, indexName, - exprValueFactory), + maxResultWindow, exprValueFactory), mappings), exclude), newEvalField), @@ -213,6 +218,7 @@ void implementOtherLogicalOperators() { @Test void shouldImplLogicalIndexScan() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); ReferenceExpression field = ref("name", STRING); NamedExpression named = named("n", field); @@ -235,6 +241,7 @@ void shouldImplLogicalIndexScan() { @Test void shouldNotPushDownFilterFarFromRelation() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); ReferenceExpression field = ref("name", STRING); Expression filterExpr = dsl.equal(field, literal("John")); @@ -260,6 +267,7 @@ void shouldNotPushDownFilterFarFromRelation() { @Test void shouldImplLogicalIndexScanAgg() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); ReferenceExpression field = ref("name", STRING); Expression filterExpr = dsl.equal(field, literal("John")); @@ -296,6 +304,7 @@ void shouldImplLogicalIndexScanAgg() { @Test void shouldNotPushDownAggregationFarFromRelation() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); ReferenceExpression field = ref("name", STRING); Expression filterExpr = dsl.equal(field, literal("John")); @@ -320,6 +329,7 @@ void shouldNotPushDownAggregationFarFromRelation() { @Test void shouldImplIndexScanWithSort() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); ReferenceExpression field = ref("name", STRING); NamedExpression named = named("n", field); @@ -342,6 +352,7 @@ void shouldImplIndexScanWithSort() { @Test void shouldImplIndexScanWithLimit() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); ReferenceExpression field = ref("name", STRING); NamedExpression named = named("n", field); @@ -363,6 +374,7 @@ void shouldImplIndexScanWithLimit() { @Test void shouldImplIndexScanWithSortAndLimit() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); ReferenceExpression field = ref("name", STRING); NamedExpression named = named("n", field); @@ -387,6 +399,7 @@ void shouldImplIndexScanWithSortAndLimit() { @Test void shouldNotPushDownLimitFarFromRelationButUpdateScanSize() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); String indexName = "test"; OpenSearchIndex index = new OpenSearchIndex(client, settings, indexName); @@ -411,6 +424,7 @@ void shouldNotPushDownLimitFarFromRelationButUpdateScanSize() { @Test void shouldPushDownProjects() { when(settings.getSettingValue(Settings.Key.QUERY_SIZE_LIMIT)).thenReturn(200); + when(client.getIndexMaxResultWindows("test")).thenReturn(Map.of("test", 10000)); String indexName = "test"; OpenSearchIndex index = new OpenSearchIndex(client, settings, indexName); @@ -425,7 +439,7 @@ indexName, projects(ref("intV", INTEGER)) assertTrue(((ProjectOperator) plan).getInput() instanceof OpenSearchIndexScan); final FetchSourceContext fetchSource = - ((OpenSearchIndexScan) ((ProjectOperator) plan).getInput()).getRequest() + ((OpenSearchIndexScan) ((ProjectOperator) plan).getInput()).getRequestBuilder() .getSourceBuilder().fetchSource(); assertThat(fetchSource.includes(), arrayContaining("intV")); assertThat(fetchSource.excludes(), emptyArray()); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/utils/Utils.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/utils/Utils.java index 15ca9d491ff..2ed9a16434f 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/utils/Utils.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/utils/Utils.java @@ -31,7 +31,9 @@ public class Utils { * Build ElasticsearchLogicalIndexScan. */ public static LogicalPlan indexScan(String tableName, Expression filter) { - return OpenSearchLogicalIndexScan.builder().relationName(tableName).filter(filter).build(); + return OpenSearchLogicalIndexScan.builder().relationName(tableName) + .filter(filter) + .build(); } /** diff --git a/opensearch/src/test/resources/mappings/accounts2.json b/opensearch/src/test/resources/mappings/accounts2.json new file mode 100644 index 00000000000..d300b8c523a --- /dev/null +++ b/opensearch/src/test/resources/mappings/accounts2.json @@ -0,0 +1,93 @@ +{ + "accounts": { + "mappings": { + "_doc": { + "properties": { + "address": { + "type": "text" + }, + "age": { + "type": "integer" + }, + "balance": { + "type": "double" + }, + "city": { + "type": "keyword" + }, + "birthday": { + "type": "date" + }, + "location": { + "type": "geo_point" + }, + "new_field": { + "type": "some_new_es_type_outside_type_system" + }, + "field with spaces": { + "type": "text" + }, + "employer": { + "type": "text", + "fields": { + "raw": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "projects": { + "type": "nested", + "properties": { + "members": { + "type": "nested", + "properties": { + "name": { + "type": "text" + } + } + }, + "active": { + "type": "boolean" + }, + "release": { + "type": "date" + } + } + }, + "manager": { + "properties": { + "name": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "address": { + "type": "keyword" + }, + "salary": { + "type": "long" + } + } + } + } + } + }, + "settings": { + "index": { + "number_of_shards": 5, + "number_of_replicas": 0, + "max_result_window": 100, + "version": { + "created": "6050399" + } + } + }, + "mapping_version": "1", + "settings_version": "1" + } +} \ No newline at end of file diff --git a/plugin/build.gradle b/plugin/build.gradle index 5c4d635ef02..5c3b3974ef8 100644 --- a/plugin/build.gradle +++ b/plugin/build.gradle @@ -82,12 +82,12 @@ configurations.all { // conflict with spring-jcl exclude group: "commons-logging", module: "commons-logging" // enforce 2.12.6, https://github.com/opensearch-project/sql/issues/424 - resolutionStrategy.force 'com.fasterxml.jackson.core:jackson-core:2.13.3' + resolutionStrategy.force "com.fasterxml.jackson.core:jackson-core:${jackson_version}" // enforce 1.1.3, https://www.whitesourcesoftware.com/vulnerability-database/WS-2019-0379 resolutionStrategy.force 'commons-codec:commons-codec:1.13' resolutionStrategy.force 'com.google.guava:guava:31.0.1-jre' - resolutionStrategy.force 'com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.13.3' - resolutionStrategy.force 'com.fasterxml.jackson.core:jackson-databind:2.13.3' + resolutionStrategy.force "com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${jackson_version}" + resolutionStrategy.force "com.fasterxml.jackson.core:jackson-databind:${jackson_version}" } compileJava { options.compilerArgs.addAll(["-processor", 'lombok.launch.AnnotationProcessorHider$AnnotationProcessor']) @@ -98,7 +98,7 @@ compileTestJava { } dependencies { - api group: 'org.springframework', name: 'spring-beans', version: '5.3.22' + api group: 'org.springframework', name: 'spring-beans', version: "${spring_version}" api project(":ppl") api project(':legacy') api project(':opensearch') diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java index 6b5cbd4135b..5b9c792c7dd 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java @@ -22,9 +22,9 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.legacy.executor.format.ErrorMessageFactory; import org.opensearch.sql.legacy.metrics.Metrics; -import org.opensearch.sql.legacy.utils.LogUtils; /** * PPL Node level status. @@ -67,7 +67,7 @@ public List replacedRoutes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { - LogUtils.addRequestId(); + QueryContext.addRequestId(); try { return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.OK, diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java index 0d8ca66cc69..14d06dfc71a 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java @@ -28,8 +28,8 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.legacy.executor.format.ErrorMessageFactory; -import org.opensearch.sql.legacy.utils.LogUtils; public class RestQuerySettingsAction extends BaseRestHandler { private static final Logger LOG = LogManager.getLogger(RestQuerySettingsAction.class); @@ -74,7 +74,7 @@ public List replacedRoutes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - LogUtils.addRequestId(); + QueryContext.addRequestId(); final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = Requests.clusterUpdateSettingsRequest(); clusterUpdateSettingsRequest.timeout(request.paramAsTime( diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java index e4699b6f9f7..3ebf709922b 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java @@ -20,7 +20,7 @@ import org.opensearch.common.inject.Inject; import org.opensearch.sql.common.response.ResponseListener; import org.opensearch.sql.common.setting.Settings; -import org.opensearch.sql.common.utils.LogUtils; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.executor.ExecutionEngine; import org.opensearch.sql.legacy.metrics.MetricName; import org.opensearch.sql.legacy.metrics.Metrics; @@ -77,7 +77,8 @@ protected void doExecute( Metrics.getInstance().getNumericalMetric(MetricName.PPL_REQ_TOTAL).increment(); Metrics.getInstance().getNumericalMetric(MetricName.PPL_REQ_COUNT_TOTAL).increment(); - LogUtils.addRequestId(); + QueryContext.addRequestId(); + QueryContext.recordProcessingStarted(); PPLService pplService = createPPLService(client); TransportPPLQueryRequest transportRequest = TransportPPLQueryRequest.fromActionRequest(request); diff --git a/ppl/build.gradle b/ppl/build.gradle index 1fb4c776425..12b0787efcd 100644 --- a/ppl/build.gradle +++ b/ppl/build.gradle @@ -48,8 +48,8 @@ dependencies { implementation group: 'com.google.guava', name: 'guava', version: '31.0.1-jre' api group: 'org.opensearch', name: 'opensearch-x-content', version: "${opensearch_version}" api group: 'org.json', name: 'json', version: '20180813' - implementation group: 'org.springframework', name: 'spring-context', version: '5.3.22' - implementation group: 'org.springframework', name: 'spring-beans', version: '5.3.22' + implementation group: 'org.springframework', name: 'spring-context', version: "${spring_version}" + implementation group: 'org.springframework', name: 'spring-beans', version: "${spring_version}" implementation group: 'org.apache.logging.log4j', name: 'log4j-core', version:'2.17.1' api project(':common') api project(':core') diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index e01d4337789..ecedcb34f3d 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -223,21 +223,34 @@ TAN: 'TAN'; // DATE AND TIME FUNCTIONS ADDDATE: 'ADDDATE'; +CURDATE: 'CURDATE'; +CURRENT_DATE: 'CURRENT_DATE'; +CURRENT_TIME: 'CURRENT_TIME'; +CURRENT_TIMESTAMP: 'CURRENT_TIMESTAMP'; +CURTIME: 'CURTIME'; DATE: 'DATE'; DATE_ADD: 'DATE_ADD'; +DATE_FORMAT: 'DATE_FORMAT'; DATE_SUB: 'DATE_SUB'; DAYOFMONTH: 'DAYOFMONTH'; DAYOFWEEK: 'DAYOFWEEK'; DAYOFYEAR: 'DAYOFYEAR'; DAYNAME: 'DAYNAME'; FROM_DAYS: 'FROM_DAYS'; +LOCALTIME: 'LOCALTIME'; +LOCALTIMESTAMP: 'LOCALTIMESTAMP'; MONTHNAME: 'MONTHNAME'; +NOW: 'NOW'; SUBDATE: 'SUBDATE'; +SYSDATE: 'SYSDATE'; TIME: 'TIME'; TIME_TO_SEC: 'TIME_TO_SEC'; TIMESTAMP: 'TIMESTAMP'; -DATE_FORMAT: 'DATE_FORMAT'; TO_DAYS: 'TO_DAYS'; +UTC_DATE: 'UTC_DATE'; +UTC_TIME: 'UTC_TIME'; +UTC_TIMESTAMP: 'UTC_TIMESTAMP'; + // TEXT FUNCTIONS SUBSTR: 'SUBSTR'; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 79d0b5f92e4..4292f8c1381 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -375,7 +375,8 @@ trigonometricFunctionName dateAndTimeFunctionBase : ADDDATE | DATE | DATE_ADD | DATE_SUB | DAY | DAYNAME | DAYOFMONTH | DAYOFWEEK | DAYOFYEAR | FROM_DAYS | HOUR | MICROSECOND | MINUTE | MONTH | MONTHNAME | QUARTER | SECOND | SUBDATE | TIME | TIME_TO_SEC - | TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT + | TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT | NOW | CURDATE | CURRENT_DATE | CURTIME | CURRENT_TIME + | LOCALTIME | CURRENT_TIMESTAMP | LOCALTIMESTAMP | SYSDATE | UTC_TIMESTAMP | UTC_DATE | UTC_TIME ; /** condition function return boolean value */ @@ -419,6 +420,7 @@ literalValue | integerLiteral | decimalLiteral | booleanLiteral + | datetimeLiteral //#datetime ; intervalLiteral @@ -441,6 +443,31 @@ booleanLiteral : TRUE | FALSE ; +// Date and Time Literal, follow ANSI 92 +datetimeLiteral + : dateLiteral + | timeLiteral + | timestampLiteral + | datetimeConstantLiteral + ; + +dateLiteral + : DATE date=stringLiteral + ; + +timeLiteral + : TIME time=stringLiteral + ; + +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 + ; + pattern : stringLiteral ; diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/PPLService.java b/ppl/src/main/java/org/opensearch/sql/ppl/PPLService.java index a1a831c7cd9..866326f5627 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/PPLService.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/PPLService.java @@ -16,7 +16,7 @@ import org.opensearch.sql.analysis.Analyzer; import org.opensearch.sql.ast.tree.UnresolvedPlan; import org.opensearch.sql.common.response.ResponseListener; -import org.opensearch.sql.common.utils.LogUtils; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.executor.ExecutionEngine; import org.opensearch.sql.executor.ExecutionEngine.ExplainResponse; import org.opensearch.sql.expression.DSL; @@ -84,7 +84,8 @@ private PhysicalPlan plan(PPLQueryRequest request) { UnresolvedPlan ast = cst.accept( new AstBuilder(new AstExpressionBuilder(), request.getRequest())); - LOG.info("[{}] Incoming request {}", LogUtils.getRequestId(), anonymizer.anonymizeData(ast)); + LOG.info("[{}] Incoming request {}", QueryContext.getRequestId(), + anonymizer.anonymizeData(ast)); // 2.Analyze abstract syntax to generate logical plan LogicalPlan logicalPlan = analyzer.analyze(UnresolvedPlanHelper.addSelectAll(ast), 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 6e5893d6a3d..66c6ea55d74 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 @@ -17,11 +17,13 @@ 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; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.EvalFunctionCallContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.FieldExpressionContext; +import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.FunctionArgsContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.IdentsAsQualifiedNameContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.IdentsAsWildcardQualifiedNameContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.InExprContext; @@ -216,14 +218,8 @@ public UnresolvedExpression visitPercentileAggFunction(PercentileAggFunctionCont @Override public UnresolvedExpression visitBooleanFunctionCall(BooleanFunctionCallContext ctx) { final String functionName = ctx.conditionFunctionBase().getText(); - - return new Function( - FUNCTION_NAME_MAPPING.getOrDefault(functionName, functionName), - ctx.functionArgs() - .functionArg() - .stream() - .map(this::visitFunctionArg) - .collect(Collectors.toList())); + return visitFunction(FUNCTION_NAME_MAPPING.getOrDefault(functionName, functionName), + ctx.functionArgs()); } /** @@ -231,13 +227,7 @@ public UnresolvedExpression visitBooleanFunctionCall(BooleanFunctionCallContext */ @Override public UnresolvedExpression visitEvalFunctionCall(EvalFunctionCallContext ctx) { - return new Function( - ctx.evalFunctionName().getText(), - ctx.functionArgs() - .functionArg() - .stream() - .map(this::visitFunctionArg) - .collect(Collectors.toList())); + return visitFunction(ctx.evalFunctionName().getText(), ctx.functionArgs()); } /** @@ -253,6 +243,23 @@ public UnresolvedExpression visitConvertedDataType(ConvertedDataTypeContext ctx) return AstDSL.stringLiteral(ctx.getText()); } + @Override + public UnresolvedExpression visitDatetimeConstantLiteral(DatetimeConstantLiteralContext ctx) { + return visitFunction(ctx.getText(), null); + } + + private Function visitFunction(String functionName, FunctionArgsContext args) { + return new Function( + functionName, + args == null + ? Collections.emptyList() + : args.functionArg() + .stream() + .map(this::visitFunctionArg) + .collect(Collectors.toList()) + ); + } + @Override public UnresolvedExpression visitSingleFieldRelevanceFunction( SingleFieldRelevanceFunctionContext ctx) { diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/NowLikeFunctionParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/NowLikeFunctionParserTest.java new file mode 100644 index 00000000000..dda404f29a4 --- /dev/null +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/NowLikeFunctionParserTest.java @@ -0,0 +1,71 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.ppl.antlr; + +import static org.junit.Assert.assertNotEquals; + +import java.util.List; +import org.antlr.v4.runtime.tree.ParseTree; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class NowLikeFunctionParserTest { + + private final PPLSyntaxParser parser = new PPLSyntaxParser(); + + /** + * Set parameterized values used in test. + * @param name Function name + * @param hasFsp Whether function has fsp argument + * @param hasShortcut Whether function has shortcut (call without `()`) + */ + public NowLikeFunctionParserTest(String name, Boolean hasFsp, Boolean hasShortcut) { + this.name = name; + this.hasFsp = hasFsp; + this.hasShortcut = hasShortcut; + } + + /** + * Returns function data to test. + * @return An iterable. + */ + @Parameterized.Parameters(name = "{0}") + public static Iterable functionNames() { + return List.of(new Object[][]{ + {"now", true, false}, + {"current_timestamp", true, true}, + {"localtimestamp", true, true}, + {"localtime", true, true}, + {"sysdate", true, false}, + {"curtime", true, false}, + {"current_time", true, true}, + {"curdate", false, false}, + {"current_date", false, true} + }); + } + + private final String name; + private final Boolean hasFsp; + private final Boolean hasShortcut; + + @Test + public void test_now_like_functions() { + for (var call : hasShortcut ? List.of(name, name + "()") : List.of(name + "()")) { + ParseTree tree = parser.parse("source=t | eval r=" + call); + assertNotEquals(null, tree); + + tree = parser.parse("search source=t | where a=" + call); + assertNotEquals(null, tree); + } + if (hasFsp) { + ParseTree tree = parser.parse("search source=t | where a=" + name + "(0)"); + assertNotEquals(null, tree); + } + } +} 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 new file mode 100644 index 00000000000..feb06b996b6 --- /dev/null +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstNowLikeFunctionTest.java @@ -0,0 +1,104 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.ppl.parser; + +import static org.junit.Assert.assertEquals; +import static org.opensearch.sql.ast.dsl.AstDSL.compare; +import static org.opensearch.sql.ast.dsl.AstDSL.eval; +import static org.opensearch.sql.ast.dsl.AstDSL.field; +import static org.opensearch.sql.ast.dsl.AstDSL.filter; +import static org.opensearch.sql.ast.dsl.AstDSL.function; +import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.let; +import static org.opensearch.sql.ast.dsl.AstDSL.relation; + +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.opensearch.sql.ast.Node; +import org.opensearch.sql.ppl.antlr.PPLSyntaxParser; + +@RunWith(Parameterized.class) +public class AstNowLikeFunctionTest { + + private final PPLSyntaxParser parser = new PPLSyntaxParser(); + + /** + * Set parameterized values used in test. + * @param name Function name + * @param hasFsp Whether function has fsp argument + * @param hasShortcut Whether function has shortcut (call without `()`) + */ + public AstNowLikeFunctionTest(String name, Boolean hasFsp, Boolean hasShortcut) { + this.name = name; + this.hasFsp = hasFsp; + this.hasShortcut = hasShortcut; + } + + /** + * Returns function data to test. + * @return An iterable. + */ + @Parameterized.Parameters(name = "{0}") + public static Iterable functionNames() { + return List.of(new Object[][]{ + {"now", true, false}, + {"current_timestamp", true, true}, + {"localtimestamp", true, true}, + {"localtime", true, true}, + {"sysdate", true, false}, + {"curtime", true, false}, + {"current_time", true, true}, + {"curdate", false, false}, + {"current_date", false, true} + }); + } + + private final String name; + private final Boolean hasFsp; + private final Boolean hasShortcut; + + @Test + public void test_now_like_functions() { + for (var call : hasShortcut ? List.of(name, name + "()") : List.of(name + "()")) { + assertEqual("source=t | eval r=" + call, + eval( + relation("t"), + let( + field("r"), + function(name) + ) + )); + + assertEqual("search source=t | where a=" + call, + filter( + relation("t"), + compare("=", field("a"), function(name)) + ) + ); + } + if (hasFsp) { + assertEqual("search source=t | where a=" + name + "(0)", + filter( + relation("t"), + compare("=", field("a"), function(name, intLiteral(0))) + ) + ); + } + } + + protected void assertEqual(String query, Node expectedPlan) { + Node actualPlan = plan(query); + assertEquals(expectedPlan, actualPlan); + } + + private Node plan(String query) { + AstBuilder astBuilder = new AstBuilder(new AstExpressionBuilder(), query); + return astBuilder.visit(parser.parse(query)); + } +} diff --git a/protocol/build.gradle b/protocol/build.gradle index 7cca4aa0a96..fc35b94d343 100644 --- a/protocol/build.gradle +++ b/protocol/build.gradle @@ -30,9 +30,9 @@ plugins { dependencies { implementation group: 'com.google.guava', name: 'guava', version: '31.0.1-jre' - implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: '2.13.2' - implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.13.2.2' - implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: '2.13.2' + implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: "${jackson_version}" + implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: "${jackson_version}" + implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: "${jackson_version}" implementation 'com.google.code.gson:gson:2.8.9' implementation project(':core') implementation project(':opensearch') @@ -44,7 +44,7 @@ dependencies { } configurations.all { - resolutionStrategy.force 'com.fasterxml.jackson.core:jackson-databind:2.13.2.2' + resolutionStrategy.force "com.fasterxml.jackson.core:jackson-databind:${jackson_version}" } test { diff --git a/sql-jdbc/build.gradle b/sql-jdbc/build.gradle index 3dea2b49a7f..dd629e438f6 100644 --- a/sql-jdbc/build.gradle +++ b/sql-jdbc/build.gradle @@ -46,7 +46,7 @@ repositories { dependencies { implementation group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.13' - implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.13.2.2' + implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: "2.13.3" implementation group: 'com.amazonaws', name: 'aws-java-sdk-core', version: '1.11.452' testImplementation('org.junit.jupiter:junit-jupiter-api:5.3.1') diff --git a/sql/build.gradle b/sql/build.gradle index e6d1a65a53a..222ad92ac62 100644 --- a/sql/build.gradle +++ b/sql/build.gradle @@ -47,8 +47,8 @@ dependencies { implementation "org.antlr:antlr4-runtime:4.7.1" implementation group: 'com.google.guava', name: 'guava', version: '31.0.1-jre' implementation group: 'org.json', name: 'json', version:'20180813' - implementation group: 'org.springframework', name: 'spring-context', version: '5.3.22' - implementation group: 'org.springframework', name: 'spring-beans', version: '5.3.22' + implementation group: 'org.springframework', name: 'spring-context', version: "${spring_version}" + implementation group: 'org.springframework', name: 'spring-beans', version: "${spring_version}" implementation project(':common') implementation project(':core') api project(':protocol') diff --git a/sql/src/main/antlr/OpenSearchSQLLexer.g4 b/sql/src/main/antlr/OpenSearchSQLLexer.g4 index 9fca2942cff..2018bcbf6cf 100644 --- a/sql/src/main/antlr/OpenSearchSQLLexer.g4 +++ b/sql/src/main/antlr/OpenSearchSQLLexer.g4 @@ -191,6 +191,10 @@ COSH: 'COSH'; COT: 'COT'; CRC32: 'CRC32'; CURDATE: 'CURDATE'; +CURTIME: 'CURTIME'; +CURRENT_DATE: 'CURRENT_DATE'; +CURRENT_TIME: 'CURRENT_TIME'; +CURRENT_TIMESTAMP: 'CURRENT_TIMESTAMP'; DATE: 'DATE'; DATE_FORMAT: 'DATE_FORMAT'; DATE_ADD: 'DATE_ADD'; @@ -210,6 +214,8 @@ IFNULL: 'IFNULL'; ISNULL: 'ISNULL'; LENGTH: 'LENGTH'; LN: 'LN'; +LOCALTIME: 'LOCALTIME'; +LOCALTIMESTAMP: 'LOCALTIMESTAMP'; LOCATE: 'LOCATE'; LOG: 'LOG'; LOG10: 'LOG10'; @@ -238,13 +244,17 @@ SINH: 'SINH'; SQRT: 'SQRT'; SUBDATE: 'SUBDATE'; SUBTRACT: 'SUBTRACT'; +SYSDATE: 'SYSDATE'; TAN: 'TAN'; TIME: 'TIME'; TIME_TO_SEC: 'TIME_TO_SEC'; TIMESTAMP: 'TIMESTAMP'; TRUNCATE: 'TRUNCATE'; TO_DAYS: 'TO_DAYS'; +UTC_DATE: 'UTC_DATE'; UPPER: 'UPPER'; +UTC_TIME: 'UTC_TIME'; +UTC_TIMESTAMP: 'UTC_TIMESTAMP'; D: 'D'; T: 'T'; diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index a9316b55a4d..f2b7d971739 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -225,6 +225,7 @@ datetimeLiteral : dateLiteral | timeLiteral | timestampLiteral + | datetimeConstantLiteral ; dateLiteral @@ -239,6 +240,11 @@ 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 ; @@ -385,7 +391,8 @@ trigonometricFunctionName dateTimeFunctionName : ADDDATE | DATE | DATE_ADD | DATE_SUB | DAY | DAYNAME | DAYOFMONTH | DAYOFWEEK | DAYOFYEAR | FROM_DAYS | HOUR | MICROSECOND | MINUTE | MONTH | MONTHNAME | QUARTER | SECOND | SUBDATE | TIME | TIME_TO_SEC - | TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT + | TIMESTAMP | TO_DAYS | YEAR | WEEK | DATE_FORMAT | NOW | CURDATE | CURRENT_DATE | CURTIME | CURRENT_TIME + | LOCALTIME | CURRENT_TIMESTAMP | LOCALTIMESTAMP | SYSDATE | UTC_TIMESTAMP | UTC_DATE | UTC_TIME ; textFunctionName 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 453162e3356..1714b608ad5 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 @@ -22,6 +22,7 @@ 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; @@ -386,18 +387,22 @@ public UnresolvedExpression visitMultiFieldRelevanceFunction( } private Function visitFunction(String functionName, FunctionArgsContext args) { - if (args == null) { - return new Function(functionName, Collections.emptyList()); - } return new Function( functionName, - args.functionArg() + args == null + ? Collections.emptyList() + : args.functionArg() .stream() .map(this::visitFunctionArg) .collect(Collectors.toList()) ); } + @Override + public UnresolvedExpression visitDatetimeConstantLiteral(DatetimeConstantLiteralContext ctx) { + return visitFunction(ctx.getText(), null); + } + private QualifiedName visitIdentifiers(List identifiers) { return new QualifiedName( identifiers.stream() diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index 61bedcf7542..af428bdc524 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -22,6 +22,7 @@ import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.opensearch.sql.common.antlr.SyntaxCheckException; @@ -157,6 +158,39 @@ public void canNotParseShowStatementWithoutFilterClause() { assertThrows(SyntaxCheckException.class, () -> parser.parse("SHOW TABLES")); } + private static Stream nowLikeFunctionsData() { + return Stream.of( + Arguments.of("now", true, false), + Arguments.of("current_timestamp", true, true), + Arguments.of("localtimestamp", true, true), + Arguments.of("localtime", true, true), + Arguments.of("sysdate", true, false), + Arguments.of("curtime", true, false), + Arguments.of("current_time", true, true), + Arguments.of("curdate", false, false), + Arguments.of("current_date", false, true) + ); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("nowLikeFunctionsData") + public void can_parse_now_like_functions(String name, Boolean hasFsp, Boolean hasShortcut) { + var calls = new ArrayList() {{ + add(name + "()"); + }}; + if (hasShortcut) { + calls.add(name); + } + if (hasFsp) { + calls.add(name + "(0)"); + } + + assertNotNull(parser.parse("SELECT " + String.join(", ", calls))); + assertNotNull(parser.parse("SELECT " + String.join(", ", calls) + " FROM test")); + assertNotNull(parser.parse("SELECT " + String.join(", ", calls) + ", id FROM test")); + assertNotNull(parser.parse("SELECT id FROM test WHERE " + String.join(" AND ", calls))); + } + @Test public void can_parse_multi_match_relevance_function() { assertNotNull(parser.parse( 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 8bf38b14a67..b3fbffae6e2 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 @@ -32,8 +32,13 @@ import static org.opensearch.sql.utils.SystemIndexUtils.mappingTable; import com.google.common.collect.ImmutableList; +import java.util.List; +import java.util.stream.Stream; import org.antlr.v4.runtime.tree.ParseTree; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AllFields; import org.opensearch.sql.ast.tree.UnresolvedPlan; @@ -669,6 +674,58 @@ public void can_build_limit_clause_with_offset() { buildAST("SELECT name FROM test LIMIT 5, 10")); } + private static Stream nowLikeFunctionsData() { + return Stream.of( + Arguments.of("now", true, false), + Arguments.of("current_timestamp", true, true), + Arguments.of("localtimestamp", true, true), + Arguments.of("localtime", true, true), + Arguments.of("sysdate", true, false), + Arguments.of("curtime", true, false), + Arguments.of("current_time", true, true), + Arguments.of("curdate", false, false), + Arguments.of("current_date", false, true) + ); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("nowLikeFunctionsData") + public void test_now_like_functions(String name, Boolean hasFsp, Boolean hasShortcut) { + for (var call : hasShortcut ? List.of(name, name + "()") : List.of(name + "()")) { + assertEquals( + project( + values(emptyList()), + alias(call, function(name)) + ), + buildAST("SELECT " + call) + ); + + assertEquals( + project( + filter( + relation("test"), + function( + "=", + qualifiedName("data"), + function(name)) + ), + AllFields.of() + ), + buildAST("SELECT * FROM test WHERE data = " + call) + ); + } + + if (hasFsp) { + assertEquals( + project( + values(emptyList()), + alias(name + "(0)", function(name, intLiteral(0))) + ), + buildAST("SELECT " + name + "(0)") + ); + } + } + @Test public void can_build_qualified_name_highlight() { assertEquals( @@ -691,5 +748,4 @@ private UnresolvedPlan buildAST(String query) { ParseTree parseTree = parser.parse(query); return parseTree.accept(new AstBuilder(query)); } - }