diff --git a/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java b/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java index 372dbae387..d3c4299876 100644 --- a/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java +++ b/common/src/main/java/org/opensearch/sql/common/utils/QueryContext.java @@ -7,7 +7,6 @@ package org.opensearch.sql.common.utils; import java.util.Map; -import java.util.Optional; import java.util.UUID; import org.apache.logging.log4j.ThreadContext; @@ -21,6 +20,11 @@ public class QueryContext { */ private static final String REQUEST_ID_KEY = "request_id"; + /** + * The key of the error message in the context map. + */ + private static final String ERROR_KEY = "error"; + /** * Generates a random UUID and adds to the {@link ThreadContext} as the request id. *

@@ -48,6 +52,24 @@ public static String getRequestId() { return id; } + /** + * Capture error message to aggregate diagnostics + * for both legacy and new SQL engines. + * Can be deleted once the + * legacy SQL engine is deprecated. + */ + public static void setError(String error) { + ThreadContext.put(ERROR_KEY, error); + } + + /** + * Get all captured error messages. + */ + public static String getError() { + return ThreadContext.get(ERROR_KEY); + } + + /** * Wraps a given instance of {@link Runnable} into a new one which gets all the * entries from current ThreadContext map. 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 469f7e2011..baefac3efc 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 @@ -39,7 +39,6 @@ import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.DefaultFunctionResolver; import org.opensearch.sql.expression.function.FunctionName; -import org.opensearch.sql.expression.function.FunctionResolver; /** * The definition of date and time functions. @@ -243,12 +242,12 @@ private DefaultFunctionResolver hour() { ); } - private FunctionResolver makedate() { + private DefaultFunctionResolver makedate() { return define(BuiltinFunctionName.MAKEDATE.getName(), impl(nullMissingHandling(DateTimeFunction::exprMakeDate), DATE, DOUBLE, DOUBLE)); } - private FunctionResolver maketime() { + private DefaultFunctionResolver maketime() { return define(BuiltinFunctionName.MAKETIME.getName(), impl(nullMissingHandling(DateTimeFunction::exprMakeTime), TIME, DOUBLE, DOUBLE, DOUBLE)); } diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/AggregationOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/AggregationOperator.java index 5e05286bbc..d71089d990 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/AggregationOperator.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/AggregationOperator.java @@ -55,14 +55,19 @@ public AggregationOperator(PhysicalPlan input, List aggregatorL List groupByExprList) { this.input = input; this.aggregatorList = aggregatorList; + this.groupByExprList = groupByExprList; if (hasSpan(groupByExprList)) { + // span expression is always the first expression in group list if exist. this.span = groupByExprList.get(0); - this.groupByExprList = groupByExprList.subList(1, groupByExprList.size()); + this.collector = + Collector.Builder.build( + this.span, groupByExprList.subList(1, groupByExprList.size()), this.aggregatorList); + } else { this.span = null; - this.groupByExprList = groupByExprList; + this.collector = + Collector.Builder.build(this.span, this.groupByExprList, this.aggregatorList); } - this.collector = Collector.Builder.build(this.span, this.groupByExprList, this.aggregatorList); } @Override diff --git a/core/src/test/java/org/opensearch/sql/planner/physical/AggregationOperatorTest.java b/core/src/test/java/org/opensearch/sql/planner/physical/AggregationOperatorTest.java index 3b45a11c6c..318499c075 100644 --- a/core/src/test/java/org/opensearch/sql/planner/physical/AggregationOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/planner/physical/AggregationOperatorTest.java @@ -495,4 +495,17 @@ public void twoBucketsSpanAndLong() { "span", new ExprDateValue("2021-01-07"), "region","iad", "host", "h2", "max", 8)) )); } + + @Test + public void copyOfAggregationOperatorShouldSame() { + AggregationOperator plan = new AggregationOperator(testScan(datetimeInputs), + Collections.singletonList(DSL + .named("count", dsl.count(DSL.ref("second", TIMESTAMP)))), + Collections.singletonList(DSL + .named("span", DSL.span(DSL.ref("second", TIMESTAMP), DSL.literal(6 * 1000), "ms")))); + AggregationOperator copy = new AggregationOperator(plan.getInput(), plan.getAggregatorList(), + plan.getGroupByExprList()); + + assertEquals(plan, copy); + } } diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index b5da4e28e2..1d07dde380 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -305,7 +305,7 @@ SQL query:: { "error": { "reason": "Invalid SQL query", - "details": "DELETE clause is disabled by default and will be deprecated. Using the plugins.sql.delete.enabled setting to enable it", + "details": "Failed to parse query due to offending symbol [DELETE] at: 'DELETE' <--- HERE... More details: Expecting tokens in {, 'DESCRIBE', 'SELECT', 'SHOW', ';'}\nQuery failed on both V1 and V2 SQL engines. V1 SQL engine error following: \nDELETE clause is disabled by default and will be deprecated. Using the plugins.sql.delete.enabled setting to enable it", "type": "SQLFeatureDisabledException" }, "status": 400 diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java index a0032e7e6a..5843e5bb5b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java @@ -70,8 +70,12 @@ public void sqlDeleteSettingsTest() throws IOException { "{\n" + " \"error\": {\n" + " \"reason\": \"Invalid SQL query\",\n" - + " \"details\": \"DELETE clause is disabled by default and will be deprecated. Using " - + "the plugins.sql.delete.enabled setting to enable it\",\n" + + " \"details\": \"" + + "Query request is not supported. Either unsupported fields are present, the " + + "request is not a cursor request, or the response format is not supported.\\n" + + "Query failed on both V1 and V2 SQL engines. V1 SQL engine error following: \\n" + + "DELETE clause is disabled by default and will be deprecated." + + " Using the plugins.sql.delete.enabled setting to enable it\",\n" + " \"type\": \"SQLFeatureDisabledException\"\n" + " },\n" + " \"status\": 400\n" diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/ErrorMessage.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/ErrorMessage.java index 1ba9490916..b172d86ae8 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/ErrorMessage.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/ErrorMessage.java @@ -6,6 +6,7 @@ package org.opensearch.sql.legacy.executor.format; +import lombok.Setter; import org.json.JSONObject; import org.opensearch.rest.RestStatus; @@ -16,8 +17,13 @@ public class ErrorMessage { private int status; private String type; private String reason; + @Setter private String details; + public String getDetails() { + return details; + } + public ErrorMessage(E exception, int status) { this.exception = exception; this.status = status; diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java index 51484feda7..ae19cf1517 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSQLQueryAction.java @@ -14,6 +14,7 @@ import java.io.IOException; import java.security.PrivilegedExceptionAction; import java.util.List; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.client.node.NodeClient; @@ -26,6 +27,7 @@ import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.common.response.ResponseListener; import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.executor.ExecutionEngine.ExplainResponse; import org.opensearch.sql.legacy.metrics.MetricName; import org.opensearch.sql.legacy.metrics.Metrics; @@ -93,6 +95,9 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient nod */ public RestChannelConsumer prepareRequest(SQLQueryRequest request, NodeClient nodeClient) { if (!request.isSupported()) { + QueryContext.setError( + "Query request is not supported. Either unsupported fields are present," + + " the request is not a cursor request, or the response format is not supported."); return NOT_SUPPORTED_YET; } @@ -109,6 +114,12 @@ public RestChannelConsumer prepareRequest(SQLQueryRequest request, NodeClient no if (request.isExplainRequest()) { LOG.info("Request is falling back to old SQL engine due to: " + e.getMessage()); } + + /* + * Setting error to aggregate error messages when both legacy and new SQL engines fail. + * This implementation can be removed when the legacy SQL engine is deprecated. + */ + QueryContext.setError(e.getMessage()); return NOT_SUPPORTED_YET; } 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 06d1ba1c73..bd3456a23f 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 @@ -13,6 +13,9 @@ import com.alibaba.druid.sql.parser.ParserException; import com.google.common.collect.ImmutableList; + +import java.io.PrintWriter; +import java.io.StringWriter; import java.sql.SQLFeatureNotSupportedException; import java.util.Arrays; import java.util.HashMap; @@ -53,6 +56,7 @@ import org.opensearch.sql.legacy.executor.RestExecutor; import org.opensearch.sql.legacy.executor.cursor.CursorActionRequestRestExecutorFactory; import org.opensearch.sql.legacy.executor.cursor.CursorAsyncRestExecutor; +import org.opensearch.sql.legacy.executor.format.ErrorMessage; import org.opensearch.sql.legacy.executor.format.ErrorMessageFactory; import org.opensearch.sql.legacy.metrics.MetricName; import org.opensearch.sql.legacy.metrics.Metrics; @@ -119,6 +123,13 @@ public String getName() { return "sql_action"; } + /** + * Prepare and execute rest SQL request. In the event the V2 SQL engine fails, the V1 + * engine attempts the query. + * @param request : Rest request being made. + * @param client : Rest client for making the request. + * @return : Resulting values for request. + */ @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { Metrics.getInstance().getNumericalMetric(MetricName.REQ_TOTAL).increment(); @@ -170,6 +181,12 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } }); } catch (Exception e) { + if (null != QueryContext.getError()) { + LOG.error(QueryContext.getRequestId() + " V2 SQL error during query execution", e, + "previously collected error(s):", QueryDataAnonymizer.anonymizeData(QueryContext.getError())); + } else { + LOG.error(QueryContext.getRequestId() + " V2 SQL error during query execution", e); + } logAndPublishMetrics(e); return channel -> reportError(channel, e, isClientError(e) ? BAD_REQUEST : SERVICE_UNAVAILABLE); } @@ -189,6 +206,10 @@ private void handleCursorRequest(final RestRequest request, final String cursor, cursorRestExecutor.execute(client, request.params(), channel); } + /** + * Log error message for exception and increment failure statistics. + * @param e : Caught exception. + */ private static void logAndPublishMetrics(final Exception e) { if (isClientError(e)) { LOG.error(QueryContext.getRequestId() + " Client side error during query execution", e); @@ -197,6 +218,16 @@ private static void logAndPublishMetrics(final Exception e) { LOG.error(QueryContext.getRequestId() + " Server side error during query execution", e); Metrics.getInstance().getNumericalMetric(MetricName.FAILED_REQ_COUNT_SYS).increment(); } + + /** + * Use PrintWriter to copy the stack trace for logging. This is used to anonymize + * log messages, and can be reverted to the simpler implementation when + * the anonymizer is fixed. + */ + StringWriter sw = new StringWriter(); + e.printStackTrace(new PrintWriter(sw)); + String stackTrace = sw.toString(); + LOG.error(stackTrace); } private static QueryAction explainRequest(final NodeClient client, final SqlRequest sqlRequest, Format format) @@ -243,7 +274,7 @@ private static boolean isExplainRequest(final RestRequest request) { return request.path().endsWith("/_explain"); } - private static boolean isClientError(Exception e) { + public static boolean isClientError(Exception e) { return e instanceof NullPointerException // NPE is hard to differentiate but more likely caused by bad query || e instanceof SqlParseException || e instanceof ParserException @@ -262,8 +293,20 @@ private void sendResponse(final RestChannel channel, final String message, final channel.sendResponse(new BytesRestResponse(status, message)); } + /** + * Report Error message to user. + * @param channel : Rest channel to sent response through. + * @param e : Exception caught when attempting query. + * @param status : Status for rest request made. + */ private void reportError(final RestChannel channel, final Exception e, final RestStatus status) { - sendResponse(channel, ErrorMessageFactory.createErrorMessage(e, status.getStatus()).toString(), status); + var message = ErrorMessageFactory.createErrorMessage(e, status.getStatus()); + if (null != QueryContext.getError()) { + message.setDetails(QueryContext.getError() + + "\nQuery failed on both V1 and V2 SQL engines. V1 SQL engine error following: \n" + + message.getDetails()); + } + sendResponse(channel, message.toString(), status); } private boolean isSQLFeatureEnabled() { diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/utils/QueryDataAnonymizer.java b/legacy/src/main/java/org/opensearch/sql/legacy/utils/QueryDataAnonymizer.java index 91406333ae..b58691c022 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/utils/QueryDataAnonymizer.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/utils/QueryDataAnonymizer.java @@ -26,7 +26,8 @@ public class QueryDataAnonymizer { * Sensitive data includes index names, column names etc., * which in druid parser are parsed to SQLIdentifierExpr instances * @param query entire sql query string - * @return sql query string with all identifiers replaced with "***" + * @return sql query string with all identifiers replaced with "***" on success + * and failure string otherwise to ensure no non-anonymized data is logged in production. */ public static String anonymizeData(String query) { String resultQuery; @@ -38,8 +39,9 @@ public static String anonymizeData(String query) { .replaceAll("false", "boolean_literal") .replaceAll("[\\n][\\t]+", " "); } catch (Exception e) { - LOG.warn("Caught an exception when anonymizing sensitive data"); - resultQuery = query; + LOG.warn("Caught an exception when anonymizing sensitive data."); + LOG.debug("String {} failed anonymization.", query); + resultQuery = "Failed to anonymize data."; } return resultQuery; } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/FunctionParameterRepository.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/FunctionParameterRepository.java new file mode 100644 index 0000000000..5eb7f1fb96 --- /dev/null +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/FunctionParameterRepository.java @@ -0,0 +1,209 @@ +package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; + +import com.google.common.collect.ImmutableMap; +import java.time.ZoneId; +import java.util.Arrays; +import java.util.Map; +import java.util.stream.Collectors; +import org.opensearch.common.unit.Fuzziness; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.index.query.MultiMatchQueryBuilder; +import org.opensearch.index.query.Operator; +import org.opensearch.index.query.SimpleQueryStringFlag; +import org.opensearch.index.query.support.QueryParsers; +import org.opensearch.index.search.MatchQuery; +import org.opensearch.sql.data.model.ExprValue; + +public class FunctionParameterRepository { + private FunctionParameterRepository() { + } + + public static final Map ArgumentLimitations; + + static { + ArgumentLimitations = ImmutableMap.builder() + .put("boost", "Accepts only floating point values greater than 0.") + .put("tie_breaker", "Accepts only floating point values in range 0 to 1.") + .put("rewrite", "Available values are: constant_score, " + + "scoring_boolean, constant_score_boolean, top_terms_X, top_terms_boost_X, " + + "top_terms_blended_freqs_X, where X is an integer value.") + .put("flags", String.format( + "Available values are: %s and any combinations of these separated by '|'.", + Arrays.stream(SimpleQueryStringFlag.class.getEnumConstants()) + .map(Enum::toString).collect(Collectors.joining(", ")))) + .put("time_zone", "For more information, follow this link: " + + "https://docs.oracle.com/javase/8/docs/api/java/time/ZoneId.html#of-java.lang.String-") + .put("fuzziness", "Available values are: " + + "'AUTO', 'AUTO:x,y' or z, where x, y, z - integer values.") + .put("operator", String.format("Available values are: %s.", + Arrays.stream(Operator.class.getEnumConstants()) + .map(Enum::toString).collect(Collectors.joining(", ")))) + .put("type", String.format("Available values are: %s.", + Arrays.stream(MultiMatchQueryBuilder.Type.class.getEnumConstants()) + .map(Enum::toString).collect(Collectors.joining(", ")))) + .put("zero_terms_query", String.format("Available values are: %s.", + Arrays.stream(MatchQuery.ZeroTermsQuery.class.getEnumConstants()) + .map(Enum::toString).collect(Collectors.joining(", ")))) + .put("int", "Accepts only integer values.") + .put("float", "Accepts only floating point values.") + .put("bool", "Accepts only boolean values: 'true' or 'false'.") + .build(); + } + + private static String formatErrorMessage(String name, String value) { + return formatErrorMessage(name, value, name); + } + + private static String formatErrorMessage(String name, String value, String limitationName) { + return String.format("Invalid %s value: '%s'. %s", + name, value, ArgumentLimitations.containsKey(name) ? ArgumentLimitations.get(name) + : ArgumentLimitations.getOrDefault(limitationName, "")); + } + + /** + * Check whether value is valid for 'rewrite' or 'fuzzy_rewrite'. + * @param value Value + * @param name Value name + * @return Converted + */ + public static String checkRewrite(ExprValue value, String name) { + try { + QueryParsers.parseRewriteMethod( + value.stringValue().toLowerCase(), null, LoggingDeprecationHandler.INSTANCE); + return value.stringValue(); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage(name, value.stringValue(), "rewrite")); + } + } + + /** + * Convert ExprValue to Flags. + * @param value Value + * @return Array of flags + */ + public static SimpleQueryStringFlag[] convertFlags(ExprValue value) { + try { + return Arrays.stream(value.stringValue().toUpperCase().split("\\|")) + .map(SimpleQueryStringFlag::valueOf) + .toArray(SimpleQueryStringFlag[]::new); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage("flags", value.stringValue()), e); + } + } + + /** + * Check whether ExprValue could be converted to timezone object. + * @param value Value + * @return Converted to string + */ + public static String checkTimeZone(ExprValue value) { + try { + ZoneId.of(value.stringValue()); + return value.stringValue(); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage("time_zone", value.stringValue()), e); + } + } + + /** + * Convert ExprValue to Fuzziness object. + * @param value Value + * @return Fuzziness + */ + public static Fuzziness convertFuzziness(ExprValue value) { + try { + return Fuzziness.build(value.stringValue().toUpperCase()); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage("fuzziness", value.stringValue()), e); + } + } + + /** + * Convert ExprValue to Operator object, could be used for 'operator' and 'default_operator'. + * @param value Value + * @param name Value name + * @return Operator + */ + public static Operator convertOperator(ExprValue value, String name) { + try { + return Operator.fromString(value.stringValue().toUpperCase()); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage(name, value.stringValue(), "operator")); + } + } + + /** + * Convert ExprValue to Type object. + * @param value Value + * @return Type + */ + public static MultiMatchQueryBuilder.Type convertType(ExprValue value) { + try { + return MultiMatchQueryBuilder.Type.parse(value.stringValue().toLowerCase(), + LoggingDeprecationHandler.INSTANCE); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage("type", value.stringValue()), e); + } + } + + /** + * Convert ExprValue to ZeroTermsQuery object. + * @param value Value + * @return ZeroTermsQuery + */ + public static MatchQuery.ZeroTermsQuery convertZeroTermsQuery(ExprValue value) { + try { + return MatchQuery.ZeroTermsQuery.valueOf(value.stringValue().toUpperCase()); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage("zero_terms_query", value.stringValue()), e); + } + } + + /** + * Convert ExprValue to int. + * @param value Value + * @param name Value name + * @return int + */ + public static int convertIntValue(ExprValue value, String name) { + try { + return Integer.parseInt(value.stringValue()); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage(name, value.stringValue(), "int"), e); + } + } + + /** + * Convert ExprValue to float. + * @param value Value + * @param name Value name + * @return float + */ + public static float convertFloatValue(ExprValue value, String name) { + try { + return Float.parseFloat(value.stringValue()); + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage(name, value.stringValue(), "float"), e); + } + } + + /** + * Convert ExprValue to bool. + * @param value Value + * @param name Value name + * @return bool + */ + public static boolean convertBoolValue(ExprValue value, String name) { + try { + // Boolean.parseBoolean interprets integers or any other stuff as a valid value + Boolean res = Boolean.parseBoolean(value.stringValue()); + if (value.stringValue().equalsIgnoreCase(res.toString())) { + return res; + } else { + throw new Exception("Invalid boolean value"); + } + } catch (Exception e) { + throw new RuntimeException(formatErrorMessage(name, value.stringValue(), "bool"), e); + } + } +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchBoolPrefixQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchBoolPrefixQuery.java index 33e357afe3..1c67916c36 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchBoolPrefixQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchBoolPrefixQuery.java @@ -21,16 +21,21 @@ public class MatchBoolPrefixQuery */ public MatchBoolPrefixQuery() { super(ImmutableMap.>builder() - .put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue())) - .put("fuzziness", (b, v) -> b.fuzziness(v.stringValue())) - .put("prefix_length", (b, v) -> b.prefixLength(Integer.parseInt(v.stringValue()))) - .put("max_expansions", (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue()))) - .put("fuzzy_transpositions", - (b, v) -> b.fuzzyTranspositions(Boolean.parseBoolean(v.stringValue()))) - .put("fuzzy_rewrite", (b, v) -> b.fuzzyRewrite(v.stringValue())) - .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) - .put("operator", (b,v) -> b.operator(Operator.fromString(v.stringValue()))) + .put("boost", (b, v) -> b.boost( + FunctionParameterRepository.convertFloatValue(v, "boost"))) + .put("fuzziness", (b, v) -> b.fuzziness(FunctionParameterRepository.convertFuzziness(v))) + .put("fuzzy_rewrite", (b, v) -> b.fuzzyRewrite( + FunctionParameterRepository.checkRewrite(v, "fuzzy_rewrite"))) + .put("fuzzy_transpositions", (b, v) -> b.fuzzyTranspositions( + FunctionParameterRepository.convertBoolValue(v, "fuzzy_transpositions"))) + .put("max_expansions", (b, v) -> b.maxExpansions( + FunctionParameterRepository.convertIntValue(v, "max_expansions"))) + .put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue())) + .put("operator", (b, v) -> b.operator( + FunctionParameterRepository.convertOperator(v, "operator"))) + .put("prefix_length", (b, v) -> b.prefixLength( + FunctionParameterRepository.convertIntValue(v, "prefix_length"))) .build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhrasePrefixQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhrasePrefixQuery.java index 6d181daa4c..7f62b9b706 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhrasePrefixQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhrasePrefixQuery.java @@ -20,11 +20,14 @@ public class MatchPhrasePrefixQuery extends SingleFieldQuery>builder() .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) - .put("slop", (b, v) -> b.slop(Integer.parseInt(v.stringValue()))) - .put("max_expansions", (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue()))) + .put("boost", (b, v) -> b.boost( + FunctionParameterRepository.convertFloatValue(v, "boost"))) + .put("max_expansions", (b, v) -> b.maxExpansions( + FunctionParameterRepository.convertIntValue(v, "max_expansions"))) + .put("slop", (b, v) -> b.slop( + FunctionParameterRepository.convertIntValue(v, "slop"))) .put("zero_terms_query", (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v)))) - .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) + FunctionParameterRepository.convertZeroTermsQuery(v))) .build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java index 6a7694f629..2a93fe4657 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java @@ -30,11 +30,13 @@ public class MatchPhraseQuery extends SingleFieldQuery */ public MatchPhraseQuery() { super(ImmutableMap.>builder() - .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) - .put("slop", (b, v) -> b.slop(Integer.parseInt(v.stringValue()))) + .put("boost", (b, v) -> b.boost( + FunctionParameterRepository.convertFloatValue(v, "boost"))) + .put("slop", (b, v) -> b.slop( + FunctionParameterRepository.convertIntValue(v, "slop"))) .put("zero_terms_query", (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v)))) + FunctionParameterRepository.convertZeroTermsQuery(v))) .build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java index f6d88013e4..67e583052c 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java @@ -21,20 +21,26 @@ public class MatchQuery extends SingleFieldQuery { public MatchQuery() { super(ImmutableMap.>builder() .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) - .put("auto_generate_synonyms_phrase_query", - (b, v) -> b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue()))) - .put("fuzziness", (b, v) -> b.fuzziness(valueOfToUpper(v))) - .put("max_expansions", (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue()))) - .put("prefix_length", (b, v) -> b.prefixLength(Integer.parseInt(v.stringValue()))) - .put("fuzzy_transpositions", - (b, v) -> b.fuzzyTranspositions(Boolean.parseBoolean(v.stringValue()))) - .put("fuzzy_rewrite", (b, v) -> b.fuzzyRewrite(v.stringValue())) - .put("lenient", (b, v) -> b.lenient(Boolean.parseBoolean(v.stringValue()))) - .put("operator", (b, v) -> b.operator(Operator.fromString(v.stringValue()))) + .put("auto_generate_synonyms_phrase_query", (b, v) -> b.autoGenerateSynonymsPhraseQuery( + FunctionParameterRepository.convertBoolValue(v,"auto_generate_synonyms_phrase_query"))) + .put("boost", (b, v) -> b.boost( + FunctionParameterRepository.convertFloatValue(v, "boost"))) + .put("fuzziness", (b, v) -> b.fuzziness(FunctionParameterRepository.convertFuzziness(v))) + .put("fuzzy_rewrite", (b, v) -> b.fuzzyRewrite( + FunctionParameterRepository.checkRewrite(v, "fuzzy_rewrite"))) + .put("fuzzy_transpositions", (b, v) -> b.fuzzyTranspositions( + FunctionParameterRepository.convertBoolValue(v, "fuzzy_transpositions"))) + .put("lenient", (b, v) -> b.lenient( + FunctionParameterRepository.convertBoolValue(v, "lenient"))) .put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue())) + .put("max_expansions", (b, v) -> b.maxExpansions( + FunctionParameterRepository.convertIntValue(v, "max_expansions"))) + .put("operator", (b, v) -> b.operator( + FunctionParameterRepository.convertOperator(v, "operator"))) + .put("prefix_length", (b, v) -> b.prefixLength( + FunctionParameterRepository.convertIntValue(v, "prefix_length"))) .put("zero_terms_query", (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v)))) - .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) + FunctionParameterRepository.convertZeroTermsQuery(v))) .build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java index 549f58cb19..00e35bd061 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java @@ -7,7 +7,6 @@ import com.google.common.collect.ImmutableMap; import org.opensearch.index.query.MultiMatchQueryBuilder; -import org.opensearch.index.query.Operator; import org.opensearch.index.query.QueryBuilders; public class MultiMatchQuery extends MultiFieldQuery { @@ -19,22 +18,31 @@ public MultiMatchQuery() { super(ImmutableMap.>builder() .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) .put("auto_generate_synonyms_phrase_query", (b, v) -> - b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue()))) - .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) - .put("cutoff_frequency", (b, v) -> b.cutoffFrequency(Float.parseFloat(v.stringValue()))) - .put("fuzziness", (b, v) -> b.fuzziness(v.stringValue())) - .put("fuzzy_transpositions", (b, v) -> - b.fuzzyTranspositions(Boolean.parseBoolean(v.stringValue()))) - .put("lenient", (b, v) -> b.lenient(Boolean.parseBoolean(v.stringValue()))) - .put("max_expansions", (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue()))) + b.autoGenerateSynonymsPhraseQuery(FunctionParameterRepository + .convertBoolValue(v, "auto_generate_synonyms_phrase_query"))) + .put("boost", (b, v) -> b.boost( + FunctionParameterRepository.convertFloatValue(v, "boost"))) + .put("cutoff_frequency", (b, v) -> b.cutoffFrequency( + FunctionParameterRepository.convertFloatValue(v, "cutoff_frequency"))) + .put("fuzziness", (b, v) -> b.fuzziness(FunctionParameterRepository.convertFuzziness(v))) + .put("fuzzy_transpositions", (b, v) -> b.fuzzyTranspositions( + FunctionParameterRepository.convertBoolValue(v, "fuzzy_transpositions"))) + .put("lenient", (b, v) -> b.lenient( + FunctionParameterRepository.convertBoolValue(v, "lenient"))) + .put("max_expansions", (b, v) -> b.maxExpansions( + FunctionParameterRepository.convertIntValue(v, "max_expansions"))) .put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue())) - .put("operator", (b, v) -> b.operator(Operator.fromString(v.stringValue()))) - .put("prefix_length", (b, v) -> b.prefixLength(Integer.parseInt(v.stringValue()))) - .put("tie_breaker", (b, v) -> b.tieBreaker(Float.parseFloat(v.stringValue()))) - .put("type", (b, v) -> b.type(v.stringValue())) - .put("slop", (b, v) -> b.slop(Integer.parseInt(v.stringValue()))) + .put("operator", (b, v) -> b.operator( + FunctionParameterRepository.convertOperator(v, "operator"))) + .put("prefix_length", (b, v) -> b.prefixLength( + FunctionParameterRepository.convertIntValue(v, "prefix_length"))) + .put("slop", (b, v) -> b.slop( + FunctionParameterRepository.convertIntValue(v, "slop"))) + .put("tie_breaker", (b, v) -> b.tieBreaker( + FunctionParameterRepository.convertFloatValue(v, "tie_breaker"))) + .put("type", (b, v) -> b.type(FunctionParameterRepository.convertType(v))) .put("zero_terms_query", (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(valueOfToUpper(v)))) + FunctionParameterRepository.convertZeroTermsQuery(v))) .build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java index 21eb3f8837..b1e6063d22 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java @@ -30,39 +30,45 @@ public class QueryStringQuery extends MultiFieldQuery { */ public QueryStringQuery() { super(ImmutableMap.>builder() + .put("allow_leading_wildcard", (b, v) -> b.allowLeadingWildcard( + FunctionParameterRepository.convertBoolValue(v, "allow_leading_wildcard"))) .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) - .put("allow_leading_wildcard", (b, v) -> - b.allowLeadingWildcard(Boolean.parseBoolean(v.stringValue()))) - .put("analyze_wildcard", (b, v) -> - b.analyzeWildcard(Boolean.parseBoolean(v.stringValue()))) - .put("auto_generate_synonyms_phrase_query", (b, v) -> - b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue()))) - .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) - .put("default_operator", (b, v) -> - b.defaultOperator(Operator.fromString(v.stringValue()))) - .put("enable_position_increments", (b, v) -> - b.enablePositionIncrements(Boolean.parseBoolean(v.stringValue()))) - .put("fuzziness", (b, v) -> b.fuzziness(Fuzziness.build(v.stringValue()))) - .put("fuzzy_rewrite", (b, v) -> b.fuzzyRewrite(v.stringValue())) - .put("escape", (b, v) -> b.escape(Boolean.parseBoolean(v.stringValue()))) - .put("fuzzy_max_expansions", (b, v) -> - b.fuzzyMaxExpansions(Integer.parseInt(v.stringValue()))) - .put("fuzzy_prefix_length", (b, v) -> - b.fuzzyPrefixLength(Integer.parseInt(v.stringValue()))) - .put("fuzzy_transpositions", (b, v) -> - b.fuzzyTranspositions(Boolean.parseBoolean(v.stringValue()))) - .put("lenient", (b, v) -> b.lenient(Boolean.parseBoolean(v.stringValue()))) - .put("max_determinized_states", (b, v) -> - b.maxDeterminizedStates(Integer.parseInt(v.stringValue()))) + .put("analyze_wildcard", (b, v) -> b.analyzeWildcard( + FunctionParameterRepository.convertBoolValue(v, "analyze_wildcard"))) + .put("auto_generate_synonyms_phrase_query", (b, v) -> b.autoGenerateSynonymsPhraseQuery( + FunctionParameterRepository.convertBoolValue(v,"auto_generate_synonyms_phrase_query"))) + .put("boost", (b, v) -> b.boost( + FunctionParameterRepository.convertFloatValue(v, "boost"))) + .put("default_operator", (b, v) -> b.defaultOperator( + FunctionParameterRepository.convertOperator(v, "default_operator"))) + .put("enable_position_increments", (b, v) -> b.enablePositionIncrements( + FunctionParameterRepository.convertBoolValue(v, "enable_position_increments"))) + .put("escape", (b, v) -> b.escape( + FunctionParameterRepository.convertBoolValue(v, "escape"))) + .put("fuzziness", (b, v) -> b.fuzziness(FunctionParameterRepository.convertFuzziness(v))) + .put("fuzzy_max_expansions", (b, v) -> b.fuzzyMaxExpansions( + FunctionParameterRepository.convertIntValue(v, "fuzzy_max_expansions"))) + .put("fuzzy_prefix_length", (b, v) -> b.fuzzyPrefixLength( + FunctionParameterRepository.convertIntValue(v, "fuzzy_prefix_length"))) + .put("fuzzy_rewrite", (b, v) -> b.fuzzyRewrite( + FunctionParameterRepository.checkRewrite(v, "fuzzy_rewrite"))) + .put("fuzzy_transpositions", (b, v) -> b.fuzzyTranspositions( + FunctionParameterRepository.convertBoolValue(v, "fuzzy_transpositions"))) + .put("lenient", (b, v) -> b.lenient( + FunctionParameterRepository.convertBoolValue(v, "lenient"))) + .put("max_determinized_states", (b, v) -> b.maxDeterminizedStates( + FunctionParameterRepository.convertIntValue(v, "max_determinized_states"))) .put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue())) + .put("phrase_slop", (b, v) -> b.phraseSlop( + FunctionParameterRepository.convertIntValue(v, "phrase_slop"))) .put("quote_analyzer", (b, v) -> b.quoteAnalyzer(v.stringValue())) - .put("phrase_slop", (b, v) -> b.phraseSlop(Integer.parseInt(v.stringValue()))) .put("quote_field_suffix", (b, v) -> b.quoteFieldSuffix(v.stringValue())) - .put("rewrite", (b, v) -> b.rewrite(v.stringValue())) - .put("type", (b, v) -> b.type(MultiMatchQueryBuilder.Type.parse(valueOfToLower(v), - LoggingDeprecationHandler.INSTANCE))) - .put("tie_breaker", (b, v) -> b.tieBreaker(Float.parseFloat(v.stringValue()))) - .put("time_zone", (b, v) -> b.timeZone(v.stringValue())) + .put("rewrite", (b, v) -> b.rewrite( + FunctionParameterRepository.checkRewrite(v, "rewrite"))) + .put("tie_breaker", (b, v) -> b.tieBreaker( + FunctionParameterRepository.convertFloatValue(v, "tie_breaker"))) + .put("time_zone", (b, v) -> b.timeZone(FunctionParameterRepository.checkTimeZone(v))) + .put("type", (b, v) -> b.type(FunctionParameterRepository.convertType(v))) .build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java index 282c5478b4..2a8f06f672 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java @@ -5,19 +5,15 @@ package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.function.BiFunction; +import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import org.opensearch.index.query.QueryBuilder; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.exception.SemanticCheckException; -import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.NamedArgumentExpression; import org.opensearch.sql.opensearch.storage.script.filter.lucene.LuceneQuery; @@ -31,26 +27,52 @@ public abstract class RelevanceQuery extends LuceneQuery @Override public QueryBuilder build(FunctionExpression func) { - List arguments = func.getArguments(); + var arguments = func.getArguments().stream() + .map(a -> (NamedArgumentExpression)a).collect(Collectors.toList()); if (arguments.size() < 2) { throw new SyntaxCheckException( String.format("%s requires at least two parameters", getQueryName())); } - NamedArgumentExpression field = (NamedArgumentExpression) arguments.get(0); - NamedArgumentExpression query = (NamedArgumentExpression) arguments.get(1); - T queryBuilder = createQueryBuilder(field, query); - Iterator iterator = arguments.listIterator(2); - Set visitedParms = new HashSet(); + // Aggregate parameters by name, so getting a Map + arguments.stream().collect(Collectors.groupingBy(a -> a.getArgName().toLowerCase())) + .forEach((k, v) -> { + if (v.size() > 1) { + throw new SemanticCheckException( + String.format("Parameter '%s' can only be specified once.", k)); + } + }); + + // Extract 'field'/'fields' and 'query' + var field = arguments.stream() + .filter(a -> a.getArgName().equalsIgnoreCase("field")) + .findFirst() + .orElse(null); + var fields = arguments.stream() + .filter(a -> a.getArgName().equalsIgnoreCase("fields")) + .findFirst() + .orElse(null); + if (fields == null && field == null) { + throw new SemanticCheckException("Both 'field' and 'fields' parameters are missing."); + } + if (fields != null && field != null) { + throw new SemanticCheckException("Both 'field' and 'fields' parameters are given."); + } + + var query = arguments.stream() + .filter(a -> a.getArgName().equalsIgnoreCase("query")) + .findFirst() + .orElseThrow(() -> new SemanticCheckException("'query' parameter is missing")); + T queryBuilder = createQueryBuilder((field != null ? field : fields), query); + + arguments.removeIf(a -> a.getArgName().equalsIgnoreCase("field") + || a.getArgName().equalsIgnoreCase("fields") + || a.getArgName().equalsIgnoreCase("query")); + + var iterator = arguments.listIterator(); while (iterator.hasNext()) { - NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); + NamedArgumentExpression arg = iterator.next(); String argNormalized = arg.getArgName().toLowerCase(); - if (visitedParms.contains(argNormalized)) { - throw new SemanticCheckException(String.format("Parameter '%s' can only be specified once.", - argNormalized)); - } else { - visitedParms.add(argNormalized); - } if (!queryBuildActions.containsKey(argNormalized)) { throw new SemanticCheckException( @@ -79,12 +101,4 @@ protected abstract T createQueryBuilder(NamedArgumentExpression field, protected interface QueryBuilderStep extends BiFunction { } - - public static String valueOfToUpper(ExprValue v) { - return v.stringValue().toUpperCase(); - } - - public static String valueOfToLower(ExprValue v) { - return v.stringValue().toLowerCase(); - } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java index 1b7c18cb2c..6e8b7352a7 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java @@ -21,22 +21,24 @@ public class SimpleQueryStringQuery extends MultiFieldQuery>builder() - .put("analyze_wildcard", (b, v) -> b.analyzeWildcard(Boolean.parseBoolean(v.stringValue()))) .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) - .put("auto_generate_synonyms_phrase_query", (b, v) -> - b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue()))) - .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) - .put("default_operator", (b, v) -> b.defaultOperator(Operator.fromString(v.stringValue()))) - .put("flags", (b, v) -> b.flags(Arrays.stream(valueOfToUpper(v).split("\\|")) - .map(SimpleQueryStringFlag::valueOf) - .toArray(SimpleQueryStringFlag[]::new))) - .put("fuzzy_max_expansions", (b, v) -> - b.fuzzyMaxExpansions(Integer.parseInt(v.stringValue()))) - .put("fuzzy_prefix_length", (b, v) -> - b.fuzzyPrefixLength(Integer.parseInt(v.stringValue()))) - .put("fuzzy_transpositions", (b, v) -> - b.fuzzyTranspositions(Boolean.parseBoolean(v.stringValue()))) - .put("lenient", (b, v) -> b.lenient(Boolean.parseBoolean(v.stringValue()))) + .put("analyze_wildcard", (b, v) -> b.analyzeWildcard( + FunctionParameterRepository.convertBoolValue(v, "analyze_wildcard"))) + .put("auto_generate_synonyms_phrase_query", (b, v) -> b.autoGenerateSynonymsPhraseQuery( + FunctionParameterRepository.convertBoolValue(v,"auto_generate_synonyms_phrase_query"))) + .put("boost", (b, v) -> b.boost( + FunctionParameterRepository.convertFloatValue(v, "boost"))) + .put("default_operator", (b, v) -> b.defaultOperator( + FunctionParameterRepository.convertOperator(v, "default_operator"))) + .put("flags", (b, v) -> b.flags(FunctionParameterRepository.convertFlags(v))) + .put("fuzzy_max_expansions", (b, v) -> b.fuzzyMaxExpansions( + FunctionParameterRepository.convertIntValue(v, "fuzzy_max_expansions"))) + .put("fuzzy_prefix_length", (b, v) -> b.fuzzyPrefixLength( + FunctionParameterRepository.convertIntValue(v, "fuzzy_prefix_length"))) + .put("fuzzy_transpositions", (b, v) -> b.fuzzyTranspositions( + FunctionParameterRepository.convertBoolValue(v, "fuzzy_transpositions"))) + .put("lenient", (b, v) -> b.lenient( + FunctionParameterRepository.convertBoolValue(v, "lenient"))) .put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue())) .put("quote_field_suffix", (b, v) -> b.quoteFieldSuffix(v.stringValue())) .build()); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java index 75ddd1dd93..006ce5e1e9 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java @@ -332,7 +332,7 @@ void should_build_match_query_with_custom_parameters() { + " \"prefix_length\" : 0,\n" + " \"max_expansions\" : 50,\n" + " \"minimum_should_match\" : \"3\"," - + " \"fuzzy_rewrite\" : \"top_terms_N\"," + + " \"fuzzy_rewrite\" : \"top_terms_1\"," + " \"fuzzy_transpositions\" : false,\n" + " \"lenient\" : false,\n" + " \"zero_terms_query\" : \"ALL\",\n" @@ -352,7 +352,7 @@ void should_build_match_query_with_custom_parameters() { dsl.namedArgument("max_expansions", literal("50")), dsl.namedArgument("prefix_length", literal("0")), dsl.namedArgument("fuzzy_transpositions", literal("false")), - dsl.namedArgument("fuzzy_rewrite", literal("top_terms_N")), + dsl.namedArgument("fuzzy_rewrite", literal("top_terms_1")), dsl.namedArgument("lenient", literal("false")), dsl.namedArgument("minimum_should_match", literal("3")), dsl.namedArgument("zero_terms_query", literal("ALL")), @@ -366,7 +366,49 @@ void match_invalid_parameter() { dsl.namedArgument("query", literal("search query")), dsl.namedArgument("invalid_parameter", literal("invalid_value"))); var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); - assertEquals("Parameter invalid_parameter is invalid for match function.", msg); + assertTrue(msg.startsWith("Parameter invalid_parameter is invalid for match function.")); + } + + @Test + void match_disallow_duplicate_parameter() { + FunctionExpression expr = dsl.match( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("analyzer", literal("keyword")), + dsl.namedArgument("AnalYzer", literal("english"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Parameter 'analyzer' can only be specified once.", msg); + } + + @Test + void match_disallow_duplicate_query() { + FunctionExpression expr = dsl.match( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("analyzer", literal("keyword")), + dsl.namedArgument("QUERY", literal("something"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Parameter 'query' can only be specified once.", msg); + } + + @Test + void match_disallow_duplicate_field() { + FunctionExpression expr = dsl.match( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("analyzer", literal("keyword")), + dsl.namedArgument("Field", literal("something"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Parameter 'field' can only be specified once.", msg); + } + + @Test + void match_missing_field() { + FunctionExpression expr = dsl.match( + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("analyzer", literal("keyword"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Both 'field' and 'fields' parameters are missing.", msg); } @Test @@ -570,12 +612,13 @@ void should_build_match_phrase_query_with_custom_parameters() { + " \"analyzer\" : \"keyword\"," + " \"slop\" : 2,\n" + " \"zero_terms_query\" : \"ALL\",\n" - + " \"boost\" : 1.0\n" + + " \"boost\" : 1.2\n" + " }\n" + " }\n" + "}", buildQuery( dsl.match_phrase( + dsl.namedArgument("boost", literal("1.2")), dsl.namedArgument("field", literal("message")), dsl.namedArgument("query", literal("search query")), dsl.namedArgument("analyzer", literal("keyword")), @@ -831,32 +874,72 @@ void match_phrase_invalid_parameter() { dsl.namedArgument("query", literal("search query")), dsl.namedArgument("invalid_parameter", literal("invalid_value"))); var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); - assertEquals("Parameter invalid_parameter is invalid for match_phrase function.", msg); + assertTrue(msg.startsWith("Parameter invalid_parameter is invalid for match_phrase function.")); } @Test - void match_phrase_invalid_value_slop() { - FunctionExpression expr = dsl.match_phrase( - dsl.namedArgument("field", literal("message")), - dsl.namedArgument("query", literal("search query")), + void relevancy_func_invalid_arg_values() { + final var field = dsl.namedArgument("field", literal("message")); + final var fields = dsl.namedArgument("fields", DSL.literal( + new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( + "field1", ExprValueUtils.floatValue(1.F), + "field2", ExprValueUtils.floatValue(.3F)))))); + final var query = dsl.namedArgument("query", literal("search query")); + + var slopTest = dsl.match_phrase(field, query, dsl.namedArgument("slop", literal("1.5"))); - var msg = assertThrows(NumberFormatException.class, () -> buildQuery(expr)).getMessage(); - assertEquals("For input string: \"1.5\"", msg); - } + var msg = assertThrows(RuntimeException.class, () -> buildQuery(slopTest)).getMessage(); + assertEquals("Invalid slop value: '1.5'. Accepts only integer values.", msg); - @Test - void match_phrase_invalid_value_ztq() { - FunctionExpression expr = dsl.match_phrase( - dsl.namedArgument("field", literal("message")), - dsl.namedArgument("query", literal("search query")), + var ztqTest = dsl.match_phrase(field, query, dsl.namedArgument("zero_terms_query", literal("meow"))); - var msg = assertThrows(IllegalArgumentException.class, () -> buildQuery(expr)).getMessage(); - assertEquals("No enum constant org.opensearch.index.search.MatchQuery.ZeroTermsQuery.MEOW", - msg); + msg = assertThrows(RuntimeException.class, () -> buildQuery(ztqTest)).getMessage(); + assertEquals( + "Invalid zero_terms_query value: 'meow'. Available values are: NONE, ALL, NULL.", msg); + + var boostTest = dsl.match(field, query, + dsl.namedArgument("boost", literal("pewpew"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(boostTest)).getMessage(); + assertEquals( + "Invalid boost value: 'pewpew'. Accepts only floating point values greater than 0.", msg); + + var boolTest = dsl.query_string(fields, query, + dsl.namedArgument("escape", literal("42"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(boolTest)).getMessage(); + assertEquals( + "Invalid escape value: '42'. Accepts only boolean values: 'true' or 'false'.", msg); + + var typeTest = dsl.multi_match(fields, query, + dsl.namedArgument("type", literal("42"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(typeTest)).getMessage(); + assertTrue(msg.startsWith("Invalid type value: '42'. Available values are:")); + + var operatorTest = dsl.simple_query_string(fields, query, + dsl.namedArgument("default_operator", literal("42"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(operatorTest)).getMessage(); + assertTrue(msg.startsWith("Invalid default_operator value: '42'. Available values are:")); + + var flagsTest = dsl.simple_query_string(fields, query, + dsl.namedArgument("flags", literal("42"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(flagsTest)).getMessage(); + assertTrue(msg.startsWith("Invalid flags value: '42'. Available values are:")); + + var fuzzinessTest = dsl.match_bool_prefix(field, query, + dsl.namedArgument("fuzziness", literal("AUTO:"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(fuzzinessTest)).getMessage(); + assertTrue(msg.startsWith("Invalid fuzziness value: 'AUTO:'. Available values are:")); + + var rewriteTest = dsl.match_bool_prefix(field, query, + dsl.namedArgument("fuzzy_rewrite", literal("42"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(rewriteTest)).getMessage(); + assertTrue(msg.startsWith("Invalid fuzzy_rewrite value: '42'. Available values are:")); + + var timezoneTest = dsl.query_string(fields, query, + dsl.namedArgument("time_zone", literal("42"))); + msg = assertThrows(RuntimeException.class, () -> buildQuery(timezoneTest)).getMessage(); + assertTrue(msg.startsWith("Invalid time_zone value: '42'.")); } - - @Test void should_build_match_bool_prefix_query_with_default_parameters() { assertJsonEquals( @@ -878,6 +961,38 @@ void should_build_match_bool_prefix_query_with_default_parameters() { dsl.namedArgument("query", literal("search query"))))); } + @Test + void multi_match_missing_fields() { + var msg = assertThrows(SemanticCheckException.class, () -> + dsl.multi_match( + dsl.namedArgument("query", literal("search query")))).getMessage(); + assertEquals("Expected type STRUCT instead of STRING for parameter #1", msg); + } + + @Test + void multi_match_missing_fields_even_with_struct() { + FunctionExpression expr = dsl.multi_match( + dsl.namedArgument("something-but-not-fields", DSL.literal( + new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( + "pewpew", ExprValueUtils.integerValue(42)))))), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("analyzer", literal("keyword"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Both 'field' and 'fields' parameters are missing.", msg); + } + + @Test + void multi_match_both_field_and_fields() { + FunctionExpression expr = dsl.multi_match( + dsl.namedArgument("fields", DSL.literal( + new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( + "field1", ExprValueUtils.floatValue(1f)))))), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("field", literal("field2"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Both 'field' and 'fields' parameters are given.", msg); + } + @Test void should_build_match_phrase_prefix_query_with_default_parameters() { assertJsonEquals( @@ -899,7 +1014,7 @@ void should_build_match_phrase_prefix_query_with_default_parameters() { } @Test - void should_build_match_phrase_prefix_query_with_analyzer() { + void should_build_match_phrase_prefix_query_with_non_default_parameters() { assertJsonEquals( "{\n" + " \"match_phrase_prefix\" : {\n" @@ -907,8 +1022,8 @@ void should_build_match_phrase_prefix_query_with_analyzer() { + " \"query\" : \"search query\",\n" + " \"slop\" : 0,\n" + " \"zero_terms_query\" : \"NONE\",\n" - + " \"max_expansions\" : 50,\n" - + " \"boost\" : 1.0,\n" + + " \"max_expansions\" : 42,\n" + + " \"boost\" : 1.2,\n" + " \"analyzer\": english\n" + " }\n" + " }\n" @@ -917,6 +1032,8 @@ void should_build_match_phrase_prefix_query_with_analyzer() { dsl.match_phrase_prefix( dsl.namedArgument("field", literal("message")), dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("boost", literal("1.2")), + dsl.namedArgument("max_expansions", literal("42")), dsl.namedArgument("analyzer", literal("english"))))); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java index 261870ca17..748384f4c8 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java @@ -80,7 +80,7 @@ static Stream> generateValidData() { List.of( dsl.namedArgument("fields", fields_value), dsl.namedArgument("query", query_value), - dsl.namedArgument("fuzzy_transpositions", DSL.literal("42")) + dsl.namedArgument("fuzzy_transpositions", DSL.literal("true")) ), List.of( dsl.namedArgument("fields", fields_value), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java index 21b03abab0..d9b973c89c 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java @@ -62,7 +62,7 @@ static Stream> generateValidData() { dsl.namedArgument("fuzzy_rewrite", DSL.literal("constant_score")), dsl.namedArgument("fuzzy_max_expansions", DSL.literal("42")), dsl.namedArgument("fuzzy_prefix_length", DSL.literal("42")), - dsl.namedArgument("fuzzy_transpositions", DSL.literal("42")), + dsl.namedArgument("fuzzy_transpositions", DSL.literal("true")), dsl.namedArgument("lenient", DSL.literal("true")), dsl.namedArgument("max_determinized_states", DSL.literal("10000")), dsl.namedArgument("minimum_should_match", DSL.literal("4")), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java index 8f06f48727..de8576e9d4 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java @@ -105,7 +105,7 @@ static Stream> generateValidData() { List.of( dsl.namedArgument("fields", fields_value), dsl.namedArgument("query", query_value), - dsl.namedArgument("fuzzy_transpositions", DSL.literal("42")) + dsl.namedArgument("fuzzy_transpositions", DSL.literal("true")) ), List.of( dsl.namedArgument("fields", fields_value), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java index fa6a43474a..912f48b982 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java @@ -16,6 +16,7 @@ import com.google.common.collect.ImmutableMap; import java.util.List; +import java.util.Map; import java.util.stream.Stream; import org.apache.commons.lang3.NotImplementedException; import org.junit.jupiter.api.BeforeEach; @@ -45,12 +46,13 @@ class RelevanceQueryBuildTest { public static final NamedArgumentExpression QUERY_ARG = namedArgument("query", "find me"); private RelevanceQuery query; private QueryBuilder queryBuilder; + private final Map> queryBuildActions = + ImmutableMap.>builder() + .put("boost", (k, v) -> k.boost(Float.parseFloat(v.stringValue()))).build(); @BeforeEach public void setUp() { - query = mock(RelevanceQuery.class, withSettings().useConstructor( - ImmutableMap.>builder() - .put("boost", (k, v) -> k.boost(Float.parseFloat(v.stringValue()))).build()) + query = mock(RelevanceQuery.class, withSettings().useConstructor(queryBuildActions) .defaultAnswer(Mockito.CALLS_REAL_METHODS)); queryBuilder = mock(QueryBuilder.class); when(query.createQueryBuilder(any(), any())).thenReturn(queryBuilder);