From 73a043f1d0a6198f1919bbd417b4c5688f4f096d Mon Sep 17 00:00:00 2001 From: forestmvey Date: Thu, 10 Nov 2022 13:13:41 -0800 Subject: [PATCH 1/4] Fix relevance function fields are permissive when fields are missing. Signed-off-by: forestmvey --- .../org/opensearch/sql/analysis/Analyzer.java | 4 + .../function/OpenSearchFunctions.java | 67 ++++++++- .../opensearch/sql/analysis/AnalyzerTest.java | 129 ++++++++++++++++++ .../java/org/opensearch/sql/sql/MatchIT.java | 11 ++ .../org/opensearch/sql/sql/QueryStringIT.java | 11 ++ 5 files changed, 221 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index 228b54ba0c..a9d917a935 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -68,6 +68,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.LiteralExpression; import org.opensearch.sql.expression.NamedExpression; import org.opensearch.sql.expression.ReferenceExpression; @@ -75,6 +76,7 @@ import org.opensearch.sql.expression.aggregation.NamedAggregator; import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.expression.function.OpenSearchFunctions; import org.opensearch.sql.expression.function.TableFunctionImplementation; import org.opensearch.sql.expression.parse.ParseExpression; import org.opensearch.sql.planner.logical.LogicalAD; @@ -220,6 +222,8 @@ public LogicalPlan visitFilter(Filter node, AnalysisContext context) { LogicalPlan child = node.getChild().get(0).accept(this, context); Expression condition = expressionAnalyzer.analyze(node.getCondition(), context); + OpenSearchFunctions.validateFieldList((FunctionExpression)condition, context); + ExpressionReferenceOptimizer optimizer = new ExpressionReferenceOptimizer(expressionAnalyzer.getRepository(), child); Expression optimized = optimizer.optimize(condition, context); diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index d8efe42640..c3ae8ba673 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -8,10 +8,15 @@ import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.data.type.ExprCoreType.STRUCT; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableList; import java.util.List; import java.util.stream.Collectors; import lombok.experimental.UtilityClass; +import org.opensearch.sql.analysis.AnalysisContext; +import org.opensearch.sql.analysis.TypeEnvironment; +import org.opensearch.sql.analysis.symbol.Namespace; +import org.opensearch.sql.analysis.symbol.Symbol; +import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; @@ -22,6 +27,66 @@ @UtilityClass public class OpenSearchFunctions { + private final List singleFieldFunctionNames = ImmutableList.of( + BuiltinFunctionName.MATCH.name(), + BuiltinFunctionName.MATCH_BOOL_PREFIX.name(), + BuiltinFunctionName.MATCHPHRASE.name(), + BuiltinFunctionName.MATCH_PHRASE_PREFIX.name() + ); + + private final List multiFieldFunctionNames = ImmutableList.of( + BuiltinFunctionName.MULTI_MATCH.name(), + BuiltinFunctionName.SIMPLE_QUERY_STRING.name(), + BuiltinFunctionName.QUERY_STRING.name() + ); + + /** + * Check if supplied function name is valid SingleFieldRelevanceFunction. + * @param funcName : Name of function + * @return : True if function is single-field function + */ + public static boolean isSingleFieldFunction(String funcName) { + return singleFieldFunctionNames.contains(funcName.toUpperCase()); + } + + /** + * Check if supplied function name is valid MultiFieldRelevanceFunction. + * @param funcName : Name of function + * @return : True if function is multi-field function + */ + public static boolean isMultiFieldFunction(String funcName) { + return multiFieldFunctionNames.contains(funcName.toUpperCase()); + } + + /** + * Verify if function queries fields available in type environment. + * @param node : Function used in query. + * @param context : Context of fields querying. + */ + public static void validateFieldList(FunctionExpression node, AnalysisContext context) { + String funcName = node.getFunctionName().toString(); + + TypeEnvironment typeEnv = context.peek(); + if (isSingleFieldFunction(funcName)) { + node.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> + ((arg.getArgName().equals("field") + && !arg.getValue().toString().contains("*")) + )).findFirst().ifPresent(arg -> + typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, + StringUtils.unquoteText(arg.getValue().toString())) + ) + ); + } else if (isMultiFieldFunction(funcName)) { + node.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> + arg.getArgName().equals("fields") + ).findFirst().ifPresent(fields -> + fields.getValue().valueOf(null).tupleValue() + .entrySet().stream().filter(k -> !(k.getKey().contains("*")) + ).forEach(key -> typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, key.getKey()))) + ); + } + } + /** * Add functions specific to OpenSearch to repository. */ diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java index 044949ea35..821093d846 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -59,6 +59,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.apache.commons.lang3.tuple.ImmutablePair; @@ -72,11 +73,14 @@ import org.opensearch.sql.ast.expression.HighlightFunction; import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.ParseMethod; +import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.tree.AD; import org.opensearch.sql.ast.tree.Kmeans; import org.opensearch.sql.ast.tree.ML; import org.opensearch.sql.ast.tree.RareTopN.CommandType; +import org.opensearch.sql.data.model.ExprTupleValue; +import org.opensearch.sql.data.model.ExprValueUtils; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; @@ -265,6 +269,131 @@ public void analyze_filter_aggregation_relation() { aggregate("MIN", qualifiedName("integer_value")), intLiteral(10)))); } + @Test + public void single_field_relevance_query_semantic_exception() { + SemanticCheckException exception = + assertThrows( + SemanticCheckException.class, + () -> + analyze( + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("match", + AstDSL.unresolvedArg("field", stringLiteral("missing_value")), + AstDSL.unresolvedArg("query", stringLiteral("query_value")))))); + assertEquals( + "can't resolve Symbol(namespace=FIELD_NAME, name=missing_value) in type env", + exception.getMessage()); + } + + @Test + public void single_field_relevance_query() { + assertAnalyzeEqual( + LogicalPlanDSL.filter( + LogicalPlanDSL.relation("schema", table), + DSL.match( + DSL.namedArgument("field", DSL.literal("string_value")), + DSL.namedArgument("query", DSL.literal("query_value")))), + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("match", + AstDSL.unresolvedArg("field", stringLiteral("string_value")), + AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); + } + + @Test + public void single_field_wildcard_relevance_query() { + assertAnalyzeEqual( + LogicalPlanDSL.filter( + LogicalPlanDSL.relation("schema", table), + DSL.match( + DSL.namedArgument("field", DSL.literal("wildcard_field*")), + DSL.namedArgument("query", DSL.literal("query_value")))), + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("match", + AstDSL.unresolvedArg("field", stringLiteral("wildcard_field*")), + AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); + } + + @Test + public void multi_field_relevance_query_semantic_exception() { + SemanticCheckException exception = + assertThrows( + SemanticCheckException.class, + () -> + analyze( + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("query_string", + AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( + "missing_value1", 1.F, "missing_value2", .3F))), + AstDSL.unresolvedArg("query", stringLiteral("query_value")))))); + assertEquals( + "can't resolve Symbol(namespace=FIELD_NAME, name=missing_value1) in type env", + exception.getMessage()); + } + + @Test + public void multi_field_relevance_query_mixed_fields_semantic_exception() { + SemanticCheckException exception = + assertThrows( + SemanticCheckException.class, + () -> + analyze( + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("query_string", + AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( + "string_value", 1.F, "missing_value", .3F))), + AstDSL.unresolvedArg("query", stringLiteral("query_value")))))); + assertEquals( + "can't resolve Symbol(namespace=FIELD_NAME, name=missing_value) in type env", + exception.getMessage()); + } + + @Test + public void multi_field_relevance_query() { + assertAnalyzeEqual( + LogicalPlanDSL.filter( + LogicalPlanDSL.relation("schema", table), + DSL.query_string( + DSL.namedArgument("fields", DSL.literal( + new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( + "string_value", ExprValueUtils.floatValue(1.F), + "integer_value", ExprValueUtils.floatValue(.3F)) + )) + )), + DSL.namedArgument("query", DSL.literal("query_value")))), + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("query_string", + AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( + "string_value", 1.F, "integer_value", .3F))), + AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); + } + + @Test + public void multi_field_wildcard_relevance_query() { + assertAnalyzeEqual( + LogicalPlanDSL.filter( + LogicalPlanDSL.relation("schema", table), + DSL.query_string( + DSL.namedArgument("fields", DSL.literal( + new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( + "wildcard_field1*", ExprValueUtils.floatValue(1.F), + "wildcard_field2*", ExprValueUtils.floatValue(.3F)) + )) + )), + DSL.namedArgument("query", DSL.literal("query_value")))), + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("query_string", + AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( + "wildcard_field1*", 1.F, "wildcard_field2*", .3F))), + AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); + } + @Test public void rename_relation() { assertAnalyzeEqual( diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java index 09e3504e4a..c69564e926 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java @@ -15,6 +15,7 @@ import org.json.JSONObject; import org.junit.Test; import org.opensearch.sql.legacy.SQLIntegTestCase; +import org.opensearch.sql.legacy.utils.StringUtils; public class MatchIT extends SQLIntegTestCase { @Override @@ -36,6 +37,16 @@ public void match_in_having() throws IOException { verifyDataRows(result, rows("Bates")); } + @Test + public void missing_field_test() { + String query = StringUtils.format("SELECT * FROM %s WHERE match(invalid, 'Bates')", TEST_INDEX_ACCOUNT); + final RuntimeException exception = + expectThrows(RuntimeException.class, () -> executeJdbcRequest(query)); + assertTrue(exception.getMessage() + .contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env") && + exception.getMessage().contains("SemanticCheckException")); + } + @Test public void matchquery_in_where() throws IOException { JSONObject result = executeJdbcRequest("SELECT firstname FROM " + TEST_INDEX_ACCOUNT + " WHERE matchquery(lastname, 'Bates')"); diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java index 398a7a9d94..8562a001a5 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java @@ -11,6 +11,7 @@ import org.json.JSONObject; import org.junit.Test; import org.opensearch.sql.legacy.SQLIntegTestCase; +import org.opensearch.sql.legacy.utils.StringUtils; public class QueryStringIT extends SQLIntegTestCase { @Override @@ -65,4 +66,14 @@ public void wildcard_test() throws IOException { JSONObject result3 = executeJdbcRequest(query3); assertEquals(10, result3.getInt("total")); } + + @Test + public void missing_field_test() { + String query = StringUtils.format("SELECT * FROM %s WHERE query_string([invalid], 'beer')", TEST_INDEX_BEER); + final RuntimeException exception = + expectThrows(RuntimeException.class, () -> executeJdbcRequest(query)); + assertTrue(exception.getMessage() + .contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env") && + exception.getMessage().contains("SemanticCheckException")); + } } From 63750a14190660e47701aa5631423ad1d42c58f6 Mon Sep 17 00:00:00 2001 From: forestmvey Date: Thu, 17 Nov 2022 14:31:30 -0800 Subject: [PATCH 2/4] Move fields validation to FunctionExpression interface and derived class OpenSearchFunctions. Signed-off-by: forestmvey --- .../org/opensearch/sql/analysis/Analyzer.java | 5 +- .../sql/expression/FunctionExpression.java | 8 +++ .../function/OpenSearchFunctions.java | 64 +++++++++---------- .../opensearch/sql/analysis/AnalyzerTest.java | 36 +++++++++++ 4 files changed, 79 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index a9d917a935..fa86674e24 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -68,7 +68,6 @@ 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.LiteralExpression; import org.opensearch.sql.expression.NamedExpression; import org.opensearch.sql.expression.ReferenceExpression; @@ -222,7 +221,9 @@ public LogicalPlan visitFilter(Filter node, AnalysisContext context) { LogicalPlan child = node.getChild().get(0).accept(this, context); Expression condition = expressionAnalyzer.analyze(node.getCondition(), context); - OpenSearchFunctions.validateFieldList((FunctionExpression)condition, context); + if (condition instanceof OpenSearchFunctions.OpenSearchFunction) { + ((OpenSearchFunctions.OpenSearchFunction)condition).validateParameters(context); + } ExpressionReferenceOptimizer optimizer = new ExpressionReferenceOptimizer(expressionAnalyzer.getRepository(), child); diff --git a/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java b/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java index 2a695f26e6..8d3f3b56b1 100644 --- a/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java +++ b/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java @@ -11,6 +11,7 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; +import org.opensearch.sql.analysis.AnalysisContext; import org.opensearch.sql.expression.function.FunctionImplementation; import org.opensearch.sql.expression.function.FunctionName; @@ -32,4 +33,11 @@ public T accept(ExpressionNodeVisitor visitor, C context) { return visitor.visitFunction(this, context); } + /** + * Verify if function queries fields available in type environment. + * @param context : Context of fields querying. + */ + public void validateParameters(AnalysisContext context) { + return; + } } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index c3ae8ba673..b4a95493ef 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -8,7 +8,6 @@ import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.data.type.ExprCoreType.STRUCT; -import com.google.common.collect.ImmutableList; import java.util.List; import java.util.stream.Collectors; import lombok.experimental.UtilityClass; @@ -27,14 +26,14 @@ @UtilityClass public class OpenSearchFunctions { - private final List singleFieldFunctionNames = ImmutableList.of( + private final List singleFieldFunctionNames = List.of( BuiltinFunctionName.MATCH.name(), BuiltinFunctionName.MATCH_BOOL_PREFIX.name(), BuiltinFunctionName.MATCHPHRASE.name(), BuiltinFunctionName.MATCH_PHRASE_PREFIX.name() ); - private final List multiFieldFunctionNames = ImmutableList.of( + private final List multiFieldFunctionNames = List.of( BuiltinFunctionName.MULTI_MATCH.name(), BuiltinFunctionName.SIMPLE_QUERY_STRING.name(), BuiltinFunctionName.QUERY_STRING.name() @@ -58,35 +57,6 @@ public static boolean isMultiFieldFunction(String funcName) { return multiFieldFunctionNames.contains(funcName.toUpperCase()); } - /** - * Verify if function queries fields available in type environment. - * @param node : Function used in query. - * @param context : Context of fields querying. - */ - public static void validateFieldList(FunctionExpression node, AnalysisContext context) { - String funcName = node.getFunctionName().toString(); - - TypeEnvironment typeEnv = context.peek(); - if (isSingleFieldFunction(funcName)) { - node.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> - ((arg.getArgName().equals("field") - && !arg.getValue().toString().contains("*")) - )).findFirst().ifPresent(arg -> - typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, - StringUtils.unquoteText(arg.getValue().toString())) - ) - ); - } else if (isMultiFieldFunction(funcName)) { - node.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> - arg.getArgName().equals("fields") - ).findFirst().ifPresent(fields -> - fields.getValue().valueOf(null).tupleValue() - .entrySet().stream().filter(k -> !(k.getKey().contains("*")) - ).forEach(key -> typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, key.getKey()))) - ); - } - } - /** * Add functions specific to OpenSearch to repository. */ @@ -190,5 +160,35 @@ public String toString() { .collect(Collectors.toList()); return String.format("%s(%s)", functionName, String.join(", ", args)); } + + /** + * Verify if function queries fields available in type environment. + * @param context : Context of fields querying. + */ + @Override + public void validateParameters(AnalysisContext context) { + String funcName = this.getFunctionName().toString(); + + TypeEnvironment typeEnv = context.peek(); + if (isSingleFieldFunction(funcName)) { + this.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> + ((arg.getArgName().equals("field") + && !arg.getValue().toString().contains("*")) + )).findFirst().ifPresent(arg -> + typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, + StringUtils.unquoteText(arg.getValue().toString())) + ) + ); + } else if (isMultiFieldFunction(funcName)) { + this.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> + arg.getArgName().equals("fields") + ).findFirst().ifPresent(fields -> + fields.getValue().valueOf(null).tupleValue() + .entrySet().stream().filter(k -> !(k.getKey().contains("*")) + ).forEach(key -> typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, key.getKey()))) + ); + } + } + } } diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java index 821093d846..a21a61efe9 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -80,11 +80,17 @@ import org.opensearch.sql.ast.tree.ML; import org.opensearch.sql.ast.tree.RareTopN.CommandType; import org.opensearch.sql.data.model.ExprTupleValue; +import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.ExpressionEvaluationException; 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.env.Environment; +import org.opensearch.sql.expression.function.FunctionName; import org.opensearch.sql.expression.window.WindowDefinition; import org.opensearch.sql.planner.logical.LogicalAD; import org.opensearch.sql.planner.logical.LogicalMLCommons; @@ -269,6 +275,23 @@ public void analyze_filter_aggregation_relation() { aggregate("MIN", qualifiedName("integer_value")), intLiteral(10)))); } + @Test + public void test_base_class_validate_parameters_method_does_nothing() { + var funcExpr = new FunctionExpression(FunctionName.of("func_name"), + List.of()) { + @Override + public ExprValue valueOf(Environment valueEnv) { + return null; + } + + @Override + public ExprType type() { + return null; + } + }; + funcExpr.validateParameters(analysisContext); + } + @Test public void single_field_relevance_query_semantic_exception() { SemanticCheckException exception = @@ -352,6 +375,19 @@ public void multi_field_relevance_query_mixed_fields_semantic_exception() { exception.getMessage()); } + @Test + public void no_field_relevance_query_semantic_exception() { + assertAnalyzeEqual( + LogicalPlanDSL.filter( + LogicalPlanDSL.relation("schema", table), + DSL.query( + DSL.namedArgument("query", DSL.literal("string_value:query_value")))), + AstDSL.filter( + AstDSL.relation("schema"), + AstDSL.function("query", + AstDSL.unresolvedArg("query", stringLiteral("string_value:query_value"))))); + } + @Test public void multi_field_relevance_query() { assertAnalyzeEqual( From 2d25614ad1287a0ab48c0250478ef3213d87f641 Mon Sep 17 00:00:00 2001 From: forestmvey Date: Thu, 8 Dec 2022 11:32:51 -0800 Subject: [PATCH 3/4] Validating field and fields parameters in TypeEnv as part of ExpressionAnalyzer. Signed-off-by: forestmvey --- .../org/opensearch/sql/analysis/Analyzer.java | 5 - .../sql/expression/FunctionExpression.java | 8 - .../function/OpenSearchFunctions.java | 87 +-------- .../function/RelevanceFunctionResolver.java | 11 -- .../opensearch/sql/analysis/AnalyzerTest.java | 165 ------------------ .../sql/analysis/ExpressionAnalyzerTest.java | 50 +++--- .../org/opensearch/sql/config/TestConfig.java | 2 + .../RelevanceFunctionResolverTest.java | 11 +- .../java/org/opensearch/sql/sql/MatchIT.java | 6 +- .../org/opensearch/sql/sql/QueryStringIT.java | 11 -- .../lucene/relevance/SingleFieldQuery.java | 2 +- .../script/filter/FilterQueryBuilderTest.java | 55 +++--- .../relevance/SingleFieldQueryTest.java | 6 +- .../sql/ppl/parser/AstExpressionBuilder.java | 2 +- .../ppl/parser/AstExpressionBuilderTest.java | 2 +- .../sql/sql/parser/AstExpressionBuilder.java | 2 +- .../sql/parser/AstExpressionBuilderTest.java | 18 +- 17 files changed, 90 insertions(+), 353 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index fa86674e24..228b54ba0c 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -75,7 +75,6 @@ import org.opensearch.sql.expression.aggregation.NamedAggregator; import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.FunctionName; -import org.opensearch.sql.expression.function.OpenSearchFunctions; import org.opensearch.sql.expression.function.TableFunctionImplementation; import org.opensearch.sql.expression.parse.ParseExpression; import org.opensearch.sql.planner.logical.LogicalAD; @@ -221,10 +220,6 @@ public LogicalPlan visitFilter(Filter node, AnalysisContext context) { LogicalPlan child = node.getChild().get(0).accept(this, context); Expression condition = expressionAnalyzer.analyze(node.getCondition(), context); - if (condition instanceof OpenSearchFunctions.OpenSearchFunction) { - ((OpenSearchFunctions.OpenSearchFunction)condition).validateParameters(context); - } - ExpressionReferenceOptimizer optimizer = new ExpressionReferenceOptimizer(expressionAnalyzer.getRepository(), child); Expression optimized = optimizer.optimize(condition, context); diff --git a/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java b/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java index 8d3f3b56b1..2a695f26e6 100644 --- a/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java +++ b/core/src/main/java/org/opensearch/sql/expression/FunctionExpression.java @@ -11,7 +11,6 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; -import org.opensearch.sql.analysis.AnalysisContext; import org.opensearch.sql.expression.function.FunctionImplementation; import org.opensearch.sql.expression.function.FunctionName; @@ -33,11 +32,4 @@ public T accept(ExpressionNodeVisitor visitor, C context) { return visitor.visitFunction(this, context); } - /** - * Verify if function queries fields available in type environment. - * @param context : Context of fields querying. - */ - public void validateParameters(AnalysisContext context) { - return; - } } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index b4a95493ef..842cf25cd6 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -5,17 +5,9 @@ package org.opensearch.sql.expression.function; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; -import static org.opensearch.sql.data.type.ExprCoreType.STRUCT; - import java.util.List; import java.util.stream.Collectors; import lombok.experimental.UtilityClass; -import org.opensearch.sql.analysis.AnalysisContext; -import org.opensearch.sql.analysis.TypeEnvironment; -import org.opensearch.sql.analysis.symbol.Namespace; -import org.opensearch.sql.analysis.symbol.Symbol; -import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; @@ -26,37 +18,6 @@ @UtilityClass public class OpenSearchFunctions { - private final List singleFieldFunctionNames = List.of( - BuiltinFunctionName.MATCH.name(), - BuiltinFunctionName.MATCH_BOOL_PREFIX.name(), - BuiltinFunctionName.MATCHPHRASE.name(), - BuiltinFunctionName.MATCH_PHRASE_PREFIX.name() - ); - - private final List multiFieldFunctionNames = List.of( - BuiltinFunctionName.MULTI_MATCH.name(), - BuiltinFunctionName.SIMPLE_QUERY_STRING.name(), - BuiltinFunctionName.QUERY_STRING.name() - ); - - /** - * Check if supplied function name is valid SingleFieldRelevanceFunction. - * @param funcName : Name of function - * @return : True if function is single-field function - */ - public static boolean isSingleFieldFunction(String funcName) { - return singleFieldFunctionNames.contains(funcName.toUpperCase()); - } - - /** - * Check if supplied function name is valid MultiFieldRelevanceFunction. - * @param funcName : Name of function - * @return : True if function is multi-field function - */ - public static boolean isMultiFieldFunction(String funcName) { - return multiFieldFunctionNames.contains(funcName.toUpperCase()); - } - /** * Add functions specific to OpenSearch to repository. */ @@ -83,46 +44,46 @@ public void register(BuiltinFunctionRepository repository) { private static FunctionResolver match_bool_prefix() { FunctionName name = BuiltinFunctionName.MATCH_BOOL_PREFIX.getName(); - return new RelevanceFunctionResolver(name, STRING); + return new RelevanceFunctionResolver(name); } private static FunctionResolver match(BuiltinFunctionName match) { FunctionName funcName = match.getName(); - return new RelevanceFunctionResolver(funcName, STRING); + return new RelevanceFunctionResolver(funcName); } private static FunctionResolver match_phrase_prefix() { FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE_PREFIX.getName(); - return new RelevanceFunctionResolver(funcName, STRING); + return new RelevanceFunctionResolver(funcName); } private static FunctionResolver match_phrase(BuiltinFunctionName matchPhrase) { FunctionName funcName = matchPhrase.getName(); - return new RelevanceFunctionResolver(funcName, STRING); + return new RelevanceFunctionResolver(funcName); } private static FunctionResolver multi_match(BuiltinFunctionName multiMatchName) { - return new RelevanceFunctionResolver(multiMatchName.getName(), STRUCT); + return new RelevanceFunctionResolver(multiMatchName.getName()); } private static FunctionResolver simple_query_string() { FunctionName funcName = BuiltinFunctionName.SIMPLE_QUERY_STRING.getName(); - return new RelevanceFunctionResolver(funcName, STRUCT); + return new RelevanceFunctionResolver(funcName); } private static FunctionResolver query() { FunctionName funcName = BuiltinFunctionName.QUERY.getName(); - return new RelevanceFunctionResolver(funcName, STRING); + return new RelevanceFunctionResolver(funcName); } private static FunctionResolver query_string() { FunctionName funcName = BuiltinFunctionName.QUERY_STRING.getName(); - return new RelevanceFunctionResolver(funcName, STRUCT); + return new RelevanceFunctionResolver(funcName); } private static FunctionResolver wildcard_query(BuiltinFunctionName wildcardQuery) { FunctionName funcName = wildcardQuery.getName(); - return new RelevanceFunctionResolver(funcName, STRING); + return new RelevanceFunctionResolver(funcName); } public static class OpenSearchFunction extends FunctionExpression { @@ -160,35 +121,5 @@ public String toString() { .collect(Collectors.toList()); return String.format("%s(%s)", functionName, String.join(", ", args)); } - - /** - * Verify if function queries fields available in type environment. - * @param context : Context of fields querying. - */ - @Override - public void validateParameters(AnalysisContext context) { - String funcName = this.getFunctionName().toString(); - - TypeEnvironment typeEnv = context.peek(); - if (isSingleFieldFunction(funcName)) { - this.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> - ((arg.getArgName().equals("field") - && !arg.getValue().toString().contains("*")) - )).findFirst().ifPresent(arg -> - typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, - StringUtils.unquoteText(arg.getValue().toString())) - ) - ); - } else if (isMultiFieldFunction(funcName)) { - this.getArguments().stream().map(NamedArgumentExpression.class::cast).filter(arg -> - arg.getArgName().equals("fields") - ).findFirst().ifPresent(fields -> - fields.getValue().valueOf(null).tupleValue() - .entrySet().stream().filter(k -> !(k.getKey().contains("*")) - ).forEach(key -> typeEnv.resolve(new Symbol(Namespace.FIELD_NAME, key.getKey()))) - ); - } - } - } } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/RelevanceFunctionResolver.java b/core/src/main/java/org/opensearch/sql/expression/function/RelevanceFunctionResolver.java index 7066622e1b..ef0ac9226c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/RelevanceFunctionResolver.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/RelevanceFunctionResolver.java @@ -20,9 +20,6 @@ public class RelevanceFunctionResolver @Getter private final FunctionName functionName; - @Getter - private final ExprType declaredFirstParamType; - @Override public Pair resolve(FunctionSignature unresolvedSignature) { if (!unresolvedSignature.getFunctionName().equals(functionName)) { @@ -30,14 +27,6 @@ public Pair resolve(FunctionSignature unreso functionName.getFunctionName(), unresolvedSignature.getFunctionName().getFunctionName())); } List paramTypes = unresolvedSignature.getParamTypeList(); - ExprType providedFirstParamType = paramTypes.get(0); - - // Check if the first parameter is of the specified type. - if (!declaredFirstParamType.equals(providedFirstParamType)) { - throw new SemanticCheckException( - getWrongParameterErrorMessage(0, providedFirstParamType, declaredFirstParamType)); - } - // Check if all but the first parameter are of type STRING. for (int i = 1; i < paramTypes.size(); i++) { ExprType paramType = paramTypes.get(i); diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java index a21a61efe9..044949ea35 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -59,7 +59,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.apache.commons.lang3.tuple.ImmutablePair; @@ -73,24 +72,15 @@ import org.opensearch.sql.ast.expression.HighlightFunction; import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.ParseMethod; -import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.tree.AD; import org.opensearch.sql.ast.tree.Kmeans; import org.opensearch.sql.ast.tree.ML; import org.opensearch.sql.ast.tree.RareTopN.CommandType; -import org.opensearch.sql.data.model.ExprTupleValue; -import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.model.ExprValueUtils; -import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.ExpressionEvaluationException; 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.env.Environment; -import org.opensearch.sql.expression.function.FunctionName; import org.opensearch.sql.expression.window.WindowDefinition; import org.opensearch.sql.planner.logical.LogicalAD; import org.opensearch.sql.planner.logical.LogicalMLCommons; @@ -275,161 +265,6 @@ public void analyze_filter_aggregation_relation() { aggregate("MIN", qualifiedName("integer_value")), intLiteral(10)))); } - @Test - public void test_base_class_validate_parameters_method_does_nothing() { - var funcExpr = new FunctionExpression(FunctionName.of("func_name"), - List.of()) { - @Override - public ExprValue valueOf(Environment valueEnv) { - return null; - } - - @Override - public ExprType type() { - return null; - } - }; - funcExpr.validateParameters(analysisContext); - } - - @Test - public void single_field_relevance_query_semantic_exception() { - SemanticCheckException exception = - assertThrows( - SemanticCheckException.class, - () -> - analyze( - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("match", - AstDSL.unresolvedArg("field", stringLiteral("missing_value")), - AstDSL.unresolvedArg("query", stringLiteral("query_value")))))); - assertEquals( - "can't resolve Symbol(namespace=FIELD_NAME, name=missing_value) in type env", - exception.getMessage()); - } - - @Test - public void single_field_relevance_query() { - assertAnalyzeEqual( - LogicalPlanDSL.filter( - LogicalPlanDSL.relation("schema", table), - DSL.match( - DSL.namedArgument("field", DSL.literal("string_value")), - DSL.namedArgument("query", DSL.literal("query_value")))), - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("match", - AstDSL.unresolvedArg("field", stringLiteral("string_value")), - AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); - } - - @Test - public void single_field_wildcard_relevance_query() { - assertAnalyzeEqual( - LogicalPlanDSL.filter( - LogicalPlanDSL.relation("schema", table), - DSL.match( - DSL.namedArgument("field", DSL.literal("wildcard_field*")), - DSL.namedArgument("query", DSL.literal("query_value")))), - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("match", - AstDSL.unresolvedArg("field", stringLiteral("wildcard_field*")), - AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); - } - - @Test - public void multi_field_relevance_query_semantic_exception() { - SemanticCheckException exception = - assertThrows( - SemanticCheckException.class, - () -> - analyze( - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("query_string", - AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( - "missing_value1", 1.F, "missing_value2", .3F))), - AstDSL.unresolvedArg("query", stringLiteral("query_value")))))); - assertEquals( - "can't resolve Symbol(namespace=FIELD_NAME, name=missing_value1) in type env", - exception.getMessage()); - } - - @Test - public void multi_field_relevance_query_mixed_fields_semantic_exception() { - SemanticCheckException exception = - assertThrows( - SemanticCheckException.class, - () -> - analyze( - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("query_string", - AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( - "string_value", 1.F, "missing_value", .3F))), - AstDSL.unresolvedArg("query", stringLiteral("query_value")))))); - assertEquals( - "can't resolve Symbol(namespace=FIELD_NAME, name=missing_value) in type env", - exception.getMessage()); - } - - @Test - public void no_field_relevance_query_semantic_exception() { - assertAnalyzeEqual( - LogicalPlanDSL.filter( - LogicalPlanDSL.relation("schema", table), - DSL.query( - DSL.namedArgument("query", DSL.literal("string_value:query_value")))), - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("query", - AstDSL.unresolvedArg("query", stringLiteral("string_value:query_value"))))); - } - - @Test - public void multi_field_relevance_query() { - assertAnalyzeEqual( - LogicalPlanDSL.filter( - LogicalPlanDSL.relation("schema", table), - DSL.query_string( - DSL.namedArgument("fields", DSL.literal( - new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "string_value", ExprValueUtils.floatValue(1.F), - "integer_value", ExprValueUtils.floatValue(.3F)) - )) - )), - DSL.namedArgument("query", DSL.literal("query_value")))), - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("query_string", - AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( - "string_value", 1.F, "integer_value", .3F))), - AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); - } - - @Test - public void multi_field_wildcard_relevance_query() { - assertAnalyzeEqual( - LogicalPlanDSL.filter( - LogicalPlanDSL.relation("schema", table), - DSL.query_string( - DSL.namedArgument("fields", DSL.literal( - new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "wildcard_field1*", ExprValueUtils.floatValue(1.F), - "wildcard_field2*", ExprValueUtils.floatValue(.3F)) - )) - )), - DSL.namedArgument("query", DSL.literal("query_value")))), - AstDSL.filter( - AstDSL.relation("schema"), - AstDSL.function("query_string", - AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( - "wildcard_field1*", 1.F, "wildcard_field2*", .3F))), - AstDSL.unresolvedArg("query", stringLiteral("query_value"))))); - } - @Test public void rename_relation() { assertAnalyzeEqual( 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 7114b220ab..c8b40d1562 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java @@ -358,10 +358,10 @@ public void named_non_parse_expression() { void match_bool_prefix_expression() { assertAnalyzeEqual( DSL.match_bool_prefix( - DSL.namedArgument("field", DSL.literal("fieldA")), + DSL.namedArgument("field", DSL.literal("field_value1")), DSL.namedArgument("query", DSL.literal("sample query"))), AstDSL.function("match_bool_prefix", - AstDSL.unresolvedArg("field", stringLiteral("fieldA")), + AstDSL.unresolvedArg("field", stringLiteral("field_value1")), AstDSL.unresolvedArg("query", stringLiteral("sample query")))); } @@ -402,11 +402,11 @@ void multi_match_expression() { DSL.multi_match( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field", ExprValueUtils.floatValue(1.F)))))), + "field_value1", ExprValueUtils.floatValue(1.F)))))), DSL.namedArgument("query", DSL.literal("sample query"))), AstDSL.function("multi_match", AstDSL.unresolvedArg("fields", new RelevanceFieldList(Map.of( - "field", 1.F))), + "field_value1", 1.F))), AstDSL.unresolvedArg("query", stringLiteral("sample query")))); } @@ -416,12 +416,12 @@ void multi_match_expression_with_params() { DSL.multi_match( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field", ExprValueUtils.floatValue(1.F)))))), + "field_value1", ExprValueUtils.floatValue(1.F)))))), DSL.namedArgument("query", DSL.literal("sample query")), DSL.namedArgument("analyzer", DSL.literal("keyword"))), AstDSL.function("multi_match", AstDSL.unresolvedArg("fields", new RelevanceFieldList(Map.of( - "field", 1.F))), + "field_value1", 1.F))), AstDSL.unresolvedArg("query", stringLiteral("sample query")), AstDSL.unresolvedArg("analyzer", stringLiteral("keyword")))); } @@ -432,12 +432,12 @@ void multi_match_expression_two_fields() { DSL.multi_match( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field1", ExprValueUtils.floatValue(1.F), - "field2", ExprValueUtils.floatValue(.3F)))))), + "field_value1", ExprValueUtils.floatValue(1.F), + "field_value2", ExprValueUtils.floatValue(.3F)))))), DSL.namedArgument("query", DSL.literal("sample query"))), AstDSL.function("multi_match", AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( - "field1", 1.F, "field2", .3F))), + "field_value1", 1.F, "field_value2", .3F))), AstDSL.unresolvedArg("query", stringLiteral("sample query")))); } @@ -447,11 +447,11 @@ void simple_query_string_expression() { DSL.simple_query_string( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field", ExprValueUtils.floatValue(1.F)))))), + "field_value1", ExprValueUtils.floatValue(1.F)))))), DSL.namedArgument("query", DSL.literal("sample query"))), AstDSL.function("simple_query_string", AstDSL.unresolvedArg("fields", new RelevanceFieldList(Map.of( - "field", 1.F))), + "field_value1", 1.F))), AstDSL.unresolvedArg("query", stringLiteral("sample query")))); } @@ -461,12 +461,12 @@ void simple_query_string_expression_with_params() { DSL.simple_query_string( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field", ExprValueUtils.floatValue(1.F)))))), + "field_value1", ExprValueUtils.floatValue(1.F)))))), DSL.namedArgument("query", DSL.literal("sample query")), DSL.namedArgument("analyzer", DSL.literal("keyword"))), AstDSL.function("simple_query_string", AstDSL.unresolvedArg("fields", new RelevanceFieldList(Map.of( - "field", 1.F))), + "field_value1", 1.F))), AstDSL.unresolvedArg("query", stringLiteral("sample query")), AstDSL.unresolvedArg("analyzer", stringLiteral("keyword")))); } @@ -477,12 +477,12 @@ void simple_query_string_expression_two_fields() { DSL.simple_query_string( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field1", ExprValueUtils.floatValue(1.F), - "field2", ExprValueUtils.floatValue(.3F)))))), + "field_value1", ExprValueUtils.floatValue(1.F), + "field_value2", ExprValueUtils.floatValue(.3F)))))), DSL.namedArgument("query", DSL.literal("sample query"))), AstDSL.function("simple_query_string", AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( - "field1", 1.F, "field2", .3F))), + "field_value1", 1.F, "field_value2", .3F))), AstDSL.unresolvedArg("query", stringLiteral("sample query")))); } @@ -501,11 +501,11 @@ void query_string_expression() { DSL.query_string( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field", ExprValueUtils.floatValue(1.F)))))), + "field_value1", ExprValueUtils.floatValue(1.F)))))), DSL.namedArgument("query", DSL.literal("query_value"))), AstDSL.function("query_string", AstDSL.unresolvedArg("fields", new RelevanceFieldList(Map.of( - "field", 1.F))), + "field_value1", 1.F))), AstDSL.unresolvedArg("query", stringLiteral("query_value")))); } @@ -515,12 +515,12 @@ void query_string_expression_with_params() { DSL.query_string( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field", ExprValueUtils.floatValue(1.F)))))), + "field_value1", ExprValueUtils.floatValue(1.F)))))), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("escape", DSL.literal("false"))), AstDSL.function("query_string", AstDSL.unresolvedArg("fields", new RelevanceFieldList(Map.of( - "field", 1.F))), + "field_value1", 1.F))), AstDSL.unresolvedArg("query", stringLiteral("query_value")), AstDSL.unresolvedArg("escape", stringLiteral("false")))); } @@ -531,12 +531,12 @@ void query_string_expression_two_fields() { DSL.query_string( DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( - "field1", ExprValueUtils.floatValue(1.F), - "field2", ExprValueUtils.floatValue(.3F)))))), + "field_value1", ExprValueUtils.floatValue(1.F), + "field_value2", ExprValueUtils.floatValue(.3F)))))), DSL.namedArgument("query", DSL.literal("query_value"))), AstDSL.function("query_string", AstDSL.unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of( - "field1", 1.F, "field2", .3F))), + "field_value1", 1.F, "field_value2", .3F))), AstDSL.unresolvedArg("query", stringLiteral("query_value")))); } @@ -572,7 +572,7 @@ void wildcard_query_expression_all_params() { public void match_phrase_prefix_all_params() { assertAnalyzeEqual( DSL.match_phrase_prefix( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", "field_value1"), DSL.namedArgument("query", "search query"), DSL.namedArgument("slop", "3"), DSL.namedArgument("boost", "1.5"), @@ -581,7 +581,7 @@ public void match_phrase_prefix_all_params() { DSL.namedArgument("zero_terms_query", "NONE") ), AstDSL.function("match_phrase_prefix", - unresolvedArg("field", stringLiteral("test")), + unresolvedArg("field", stringLiteral("field_value1")), unresolvedArg("query", stringLiteral("search query")), unresolvedArg("slop", stringLiteral("3")), unresolvedArg("boost", stringLiteral("1.5")), diff --git a/core/src/test/java/org/opensearch/sql/config/TestConfig.java b/core/src/test/java/org/opensearch/sql/config/TestConfig.java index a0ef436162..4159ae12ff 100644 --- a/core/src/test/java/org/opensearch/sql/config/TestConfig.java +++ b/core/src/test/java/org/opensearch/sql/config/TestConfig.java @@ -57,6 +57,8 @@ public class TestConfig { .put("struct_value", ExprCoreType.STRUCT) .put("array_value", ExprCoreType.ARRAY) .put("timestamp_value", ExprCoreType.TIMESTAMP) + .put("field_value1", ExprCoreType.STRING) + .put("field_value2", ExprCoreType.STRING) .build(); @Bean diff --git a/core/src/test/java/org/opensearch/sql/expression/function/RelevanceFunctionResolverTest.java b/core/src/test/java/org/opensearch/sql/expression/function/RelevanceFunctionResolverTest.java index d8547057c4..deba721481 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/RelevanceFunctionResolverTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/RelevanceFunctionResolverTest.java @@ -24,7 +24,7 @@ class RelevanceFunctionResolverTest { @BeforeEach void setUp() { - resolver = new RelevanceFunctionResolver(sampleFuncName, STRING); + resolver = new RelevanceFunctionResolver(sampleFuncName); } @Test @@ -44,15 +44,6 @@ void resolve_invalid_name_test() { exception.getMessage()); } - @Test - void resolve_invalid_first_param_type_test() { - var sig = new FunctionSignature(sampleFuncName, List.of(INTEGER)); - Exception exception = assertThrows(SemanticCheckException.class, - () -> resolver.resolve(sig)); - assertEquals("Expected type STRING instead of INTEGER for parameter #1", - exception.getMessage()); - } - @Test void resolve_invalid_third_param_type_test() { var sig = new FunctionSignature(sampleFuncName, List.of(STRING, STRING, INTEGER, STRING)); diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java index c69564e926..007ed6b711 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java @@ -42,9 +42,11 @@ public void missing_field_test() { String query = StringUtils.format("SELECT * FROM %s WHERE match(invalid, 'Bates')", TEST_INDEX_ACCOUNT); final RuntimeException exception = expectThrows(RuntimeException.class, () -> executeJdbcRequest(query)); + assertTrue(exception.getMessage() - .contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env") && - exception.getMessage().contains("SemanticCheckException")); + .contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env")); + + assertTrue(exception.getMessage().contains("SemanticCheckException")); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java index 8562a001a5..398a7a9d94 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java @@ -11,7 +11,6 @@ import org.json.JSONObject; import org.junit.Test; import org.opensearch.sql.legacy.SQLIntegTestCase; -import org.opensearch.sql.legacy.utils.StringUtils; public class QueryStringIT extends SQLIntegTestCase { @Override @@ -66,14 +65,4 @@ public void wildcard_test() throws IOException { JSONObject result3 = executeJdbcRequest(query3); assertEquals(10, result3.getInt("total")); } - - @Test - public void missing_field_test() { - String query = StringUtils.format("SELECT * FROM %s WHERE query_string([invalid], 'beer')", TEST_INDEX_BEER); - final RuntimeException exception = - expectThrows(RuntimeException.class, () -> executeJdbcRequest(query)); - assertTrue(exception.getMessage() - .contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env") && - exception.getMessage().contains("SemanticCheckException")); - } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java index a7d7584d4f..43db8d7408 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java @@ -36,7 +36,7 @@ protected T createQueryBuilder(List arguments) { .orElseThrow(() -> new SemanticCheckException("'query' parameter is missing")); return createBuilder( - field.getValue().valueOf().stringValue(), + field.getValue().toString(), query.getValue().valueOf().stringValue()); } 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 cea4e2488a..a1555c2806 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 @@ -53,6 +53,8 @@ import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.LiteralExpression; +import org.opensearch.sql.expression.ReferenceExpression; +import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.storage.serialization.ExpressionSerializer; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) @@ -311,7 +313,8 @@ void should_build_match_query_with_default_parameters() { + "}", buildQuery( DSL.match( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query"))))); } @@ -339,7 +342,8 @@ void should_build_match_query_with_custom_parameters() { + "}", buildQuery( DSL.match( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query")), DSL.namedArgument("operator", literal("AND")), DSL.namedArgument("analyzer", literal("keyword")), @@ -431,7 +435,8 @@ void should_build_match_phrase_query_with_default_parameters() { + "}", buildQuery( DSL.match_phrase( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query"))))); } @@ -623,8 +628,9 @@ void should_build_match_phrase_query_with_custom_parameters() { + "}", buildQuery( DSL.match_phrase( + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("boost", literal("1.2")), - DSL.namedArgument("field", literal("message")), DSL.namedArgument("query", literal("search query")), DSL.namedArgument("analyzer", literal("keyword")), DSL.namedArgument("slop", literal("2")), @@ -653,7 +659,8 @@ void wildcard_query_convert_sql_wildcard_to_lucene() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query%"))))); assertJsonEquals("{\n" @@ -665,7 +672,8 @@ void wildcard_query_convert_sql_wildcard_to_lucene() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query_"))))); } @@ -680,7 +688,8 @@ void wildcard_query_escape_wildcards_characters() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query\\%"))))); assertJsonEquals("{\n" @@ -692,7 +701,8 @@ void wildcard_query_escape_wildcards_characters() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query\\_"))))); assertJsonEquals("{\n" @@ -704,7 +714,8 @@ void wildcard_query_escape_wildcards_characters() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query\\*"))))); assertJsonEquals("{\n" @@ -716,7 +727,8 @@ void wildcard_query_escape_wildcards_characters() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query\\?"))))); } @@ -731,7 +743,8 @@ void should_build_wildcard_query_with_default_parameters() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query*"))))); } @@ -748,7 +761,8 @@ void should_build_wildcard_query_query_with_custom_parameters() { + " }\n" + "}", buildQuery(DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query*")), DSL.namedArgument("boost", literal("0.6")), DSL.namedArgument("case_insensitive", literal("true")), @@ -1173,18 +1187,11 @@ void should_build_match_bool_prefix_query_with_default_parameters() { + "}", buildQuery( DSL.match_bool_prefix( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), 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( @@ -1225,7 +1232,8 @@ void should_build_match_phrase_prefix_query_with_default_parameters() { + "}", buildQuery( DSL.match_phrase_prefix( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query"))))); } @@ -1246,7 +1254,8 @@ void should_build_match_phrase_prefix_query_with_non_default_parameters() { + "}", buildQuery( DSL.match_phrase_prefix( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query")), DSL.namedArgument("boost", literal("1.2")), DSL.namedArgument("max_expansions", literal("42")), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java index b2d650602b..7211cc1b8b 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java @@ -19,6 +19,8 @@ import org.opensearch.sql.data.model.ExprValueUtils; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.LiteralExpression; +import org.opensearch.sql.expression.ReferenceExpression; +import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; class SingleFieldQueryTest { SingleFieldQuery query; @@ -35,12 +37,12 @@ void setUp() { } @Test - void createQueryBuilderTest() { + void createQueryBuilderQualifiedNameTest() { String sampleQuery = "sample query"; String sampleField = "fieldA"; query.createQueryBuilder(List.of(DSL.namedArgument("field", - new LiteralExpression(ExprValueUtils.stringValue(sampleField))), + new ReferenceExpression(sampleField, OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", new LiteralExpression(ExprValueUtils.stringValue(sampleQuery))))); 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 68608e23ad..a8a27cee84 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 @@ -372,7 +372,7 @@ private List singleFieldRelevanceArguments( // to skip environment resolving and function signature resolving ImmutableList.Builder builder = ImmutableList.builder(); builder.add(new UnresolvedArgument("field", - new Literal(StringUtils.unquoteText(ctx.field.getText()), DataType.STRING))); + new QualifiedName(StringUtils.unquoteText(ctx.field.getText())))); builder.add(new UnresolvedArgument("query", new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); ctx.relevanceArg().forEach(v -> builder.add(new UnresolvedArgument( diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java index dbdfb71aa7..4fdee48308 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java @@ -705,7 +705,7 @@ public void canBuildMatchRelevanceFunctionWithArguments() { relation("test"), function( "match", - unresolvedArg("field", stringLiteral("message")), + unresolvedArg("field", qualifiedName("message")), unresolvedArg("query", stringLiteral("test query")), unresolvedArg("analyzer", stringLiteral("keyword")) ) 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 bae22595ca..9045870c04 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 @@ -475,7 +475,7 @@ private List singleFieldRelevanceArguments( // to skip environment resolving and function signature resolving ImmutableList.Builder builder = ImmutableList.builder(); builder.add(new UnresolvedArgument("field", - new Literal(StringUtils.unquoteText(ctx.field.getText()), DataType.STRING))); + new QualifiedName(StringUtils.unquoteText(ctx.field.getText())))); builder.add(new UnresolvedArgument("query", new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); fillRelevanceArgs(ctx.relevanceArg(), builder); diff --git a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java index 9af4119fdf..c4223fa3aa 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java @@ -463,7 +463,7 @@ public void filteredDistinctCount() { public void matchPhraseQueryAllParameters() { assertEquals( AstDSL.function("matchphrasequery", - unresolvedArg("field", stringLiteral("test")), + unresolvedArg("field", qualifiedName("test")), unresolvedArg("query", stringLiteral("search query")), unresolvedArg("slop", stringLiteral("3")), unresolvedArg("analyzer", stringLiteral("standard")), @@ -479,7 +479,7 @@ public void matchPhraseQueryAllParameters() { public void matchPhrasePrefixAllParameters() { assertEquals( AstDSL.function("match_phrase_prefix", - unresolvedArg("field", stringLiteral("test")), + unresolvedArg("field", qualifiedName("test")), unresolvedArg("query", stringLiteral("search query")), unresolvedArg("slop", stringLiteral("3")), unresolvedArg("boost", stringLiteral("1.5")), @@ -496,13 +496,13 @@ public void matchPhrasePrefixAllParameters() { @Test public void relevanceMatch() { assertEquals(AstDSL.function("match", - unresolvedArg("field", stringLiteral("message")), + unresolvedArg("field", qualifiedName("message")), unresolvedArg("query", stringLiteral("search query"))), buildExprAst("match('message', 'search query')") ); assertEquals(AstDSL.function("match", - unresolvedArg("field", stringLiteral("message")), + unresolvedArg("field", qualifiedName("message")), unresolvedArg("query", stringLiteral("search query")), unresolvedArg("analyzer", stringLiteral("keyword")), unresolvedArg("operator", stringLiteral("AND"))), @@ -512,13 +512,13 @@ public void relevanceMatch() { @Test public void relevanceMatchQuery() { assertEquals(AstDSL.function("matchquery", - unresolvedArg("field", stringLiteral("message")), + unresolvedArg("field", qualifiedName("message")), unresolvedArg("query", stringLiteral("search query"))), buildExprAst("matchquery('message', 'search query')") ); assertEquals(AstDSL.function("matchquery", - unresolvedArg("field", stringLiteral("message")), + unresolvedArg("field", qualifiedName("message")), unresolvedArg("query", stringLiteral("search query")), unresolvedArg("analyzer", stringLiteral("keyword")), unresolvedArg("operator", stringLiteral("AND"))), @@ -528,13 +528,13 @@ public void relevanceMatchQuery() { @Test public void relevanceMatch_Query() { assertEquals(AstDSL.function("match_query", - unresolvedArg("field", stringLiteral("message")), + unresolvedArg("field", qualifiedName("message")), unresolvedArg("query", stringLiteral("search query"))), buildExprAst("match_query('message', 'search query')") ); assertEquals(AstDSL.function("match_query", - unresolvedArg("field", stringLiteral("message")), + unresolvedArg("field", qualifiedName("message")), unresolvedArg("query", stringLiteral("search query")), unresolvedArg("analyzer", stringLiteral("keyword")), unresolvedArg("operator", stringLiteral("AND"))), @@ -640,7 +640,7 @@ public void relevanceQuery_string() { @Test public void relevanceWildcard_query() { assertEquals(AstDSL.function("wildcard_query", - unresolvedArg("field", stringLiteral("field")), + unresolvedArg("field", qualifiedName("field")), unresolvedArg("query", stringLiteral("search query*")), unresolvedArg("boost", stringLiteral("1.5")), unresolvedArg("case_insensitive", stringLiteral("true")), From 534f52b1a9cfe422f3c6350444ff550d67a388bc Mon Sep 17 00:00:00 2001 From: forestmvey Date: Fri, 9 Dec 2022 11:29:50 -0800 Subject: [PATCH 4/4] Updating tests to reflect using qualified name for single-field relevance function field parameters. Signed-off-by: forestmvey --- .../java/org/opensearch/sql/sql/MatchIT.java | 24 +++++++ .../lucene/relevance/SingleFieldQuery.java | 3 +- .../script/filter/FilterQueryBuilderTest.java | 13 ++-- .../lucene/MatchBoolPrefixQueryTest.java | 11 +++- .../lucene/MatchPhrasePrefixQueryTest.java | 26 +++++--- .../filter/lucene/MatchPhraseQueryTest.java | 65 +++++++++++++------ .../script/filter/lucene/MatchQueryTest.java | 53 ++++++++++----- .../filter/lucene/WildcardQueryTest.java | 11 +++- .../relevance/SingleFieldQueryTest.java | 16 ++++- 9 files changed, 163 insertions(+), 59 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java index 007ed6b711..b113e83477 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java @@ -49,6 +49,30 @@ public void missing_field_test() { assertTrue(exception.getMessage().contains("SemanticCheckException")); } + @Test + public void missing_quoted_field_test() { + String query = StringUtils.format("SELECT * FROM %s WHERE match('invalid', 'Bates')", TEST_INDEX_ACCOUNT); + final RuntimeException exception = + expectThrows(RuntimeException.class, () -> executeJdbcRequest(query)); + + assertTrue(exception.getMessage() + .contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env")); + + assertTrue(exception.getMessage().contains("SemanticCheckException")); + } + + @Test + public void missing_backtick_field_test() { + String query = StringUtils.format("SELECT * FROM %s WHERE match(`invalid`, 'Bates')", TEST_INDEX_ACCOUNT); + final RuntimeException exception = + expectThrows(RuntimeException.class, () -> executeJdbcRequest(query)); + + assertTrue(exception.getMessage() + .contains("can't resolve Symbol(namespace=FIELD_NAME, name=invalid) in type env")); + + assertTrue(exception.getMessage().contains("SemanticCheckException")); + } + @Test public void matchquery_in_where() throws IOException { JSONObject result = executeJdbcRequest("SELECT firstname FROM " + TEST_INDEX_ACCOUNT + " WHERE matchquery(lastname, 'Bates')"); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java index 43db8d7408..ec110dfd8b 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQuery.java @@ -10,6 +10,7 @@ import org.opensearch.index.query.QueryBuilder; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.NamedArgumentExpression; +import org.opensearch.sql.expression.ReferenceExpression; /** * Base class to represent builder class for relevance queries like match_query, match_bool_prefix, @@ -36,7 +37,7 @@ protected T createQueryBuilder(List arguments) { .orElseThrow(() -> new SemanticCheckException("'query' parameter is missing")); return createBuilder( - field.getValue().toString(), + ((ReferenceExpression)field.getValue()).getAttr(), query.getValue().valueOf().stringValue()); } 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 a1555c2806..692fbbb79f 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 @@ -54,7 +54,6 @@ import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.LiteralExpression; import org.opensearch.sql.expression.ReferenceExpression; -import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.storage.serialization.ExpressionSerializer; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) @@ -362,7 +361,8 @@ void should_build_match_query_with_custom_parameters() { @Test void match_invalid_parameter() { FunctionExpression expr = DSL.match( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query")), DSL.namedArgument("invalid_parameter", literal("invalid_value"))); var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); @@ -640,7 +640,8 @@ void should_build_match_phrase_query_with_custom_parameters() { @Test void wildcard_query_invalid_parameter() { FunctionExpression expr = DSL.wildcard_query( - DSL.namedArgument("field", literal("field")), + DSL.namedArgument("field", + new ReferenceExpression("field", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query*")), DSL.namedArgument("invalid_parameter", literal("invalid_value"))); assertThrows(SemanticCheckException.class, () -> buildQuery(expr), @@ -1100,7 +1101,8 @@ void simple_query_string_invalid_parameter() { @Test void match_phrase_invalid_parameter() { FunctionExpression expr = DSL.match_phrase( - DSL.namedArgument("field", literal("message")), + DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", literal("search query")), DSL.namedArgument("invalid_parameter", literal("invalid_value"))); var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); @@ -1109,7 +1111,8 @@ void match_phrase_invalid_parameter() { @Test void relevancy_func_invalid_arg_values() { - final var field = DSL.namedArgument("field", literal("message")); + final var field = DSL.namedArgument("field", + new ReferenceExpression("message", OPENSEARCH_TEXT_KEYWORD)); final var fields = DSL.namedArgument("fields", DSL.literal( new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( "field1", ExprValueUtils.floatValue(1.F), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchBoolPrefixQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchBoolPrefixQueryTest.java index 162c55fcaf..34ef29f091 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchBoolPrefixQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchBoolPrefixQueryTest.java @@ -23,8 +23,10 @@ import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.NamedArgumentExpression; +import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.env.Environment; import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchBoolPrefixQuery; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) @@ -33,7 +35,8 @@ public class MatchBoolPrefixQueryTest { private final FunctionName matchBoolPrefix = FunctionName.of("match_bool_prefix"); static Stream> generateValidData() { - NamedArgumentExpression field = DSL.namedArgument("field", DSL.literal("field_value")); + NamedArgumentExpression field = DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)); NamedArgumentExpression query = DSL.namedArgument("query", DSL.literal("query_value")); return List.of( DSL.namedArgument("fuzziness", DSL.literal("AUTO")), @@ -58,7 +61,8 @@ public void test_valid_arguments(List validArgs) { @Test public void test_valid_when_two_arguments() { List arguments = List.of( - DSL.namedArgument("field", "field_value"), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "query_value")); Assertions.assertNotNull(matchBoolPrefixQuery.build(new MatchExpression(arguments))); } @@ -80,7 +84,8 @@ public void test_SyntaxCheckException_when_one_argument() { @Test public void test_SemanticCheckException_when_invalid_argument() { List arguments = List.of( - DSL.namedArgument("field", "field_value"), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "query_value"), DSL.namedArgument("unsupported", "unsupported_value")); Assertions.assertThrows(SemanticCheckException.class, diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhrasePrefixQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhrasePrefixQueryTest.java index a0b9e5f318..e02f112677 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhrasePrefixQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhrasePrefixQueryTest.java @@ -20,8 +20,10 @@ import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.env.Environment; import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchPhrasePrefixQuery; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) @@ -39,7 +41,8 @@ public void test_SyntaxCheckException_when_no_arguments() { @Test public void test_SyntaxCheckException_when_one_argument() { - List arguments = List.of(DSL.namedArgument("field", "test")); + List arguments = List.of(DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD))); assertThrows(SyntaxCheckException.class, () -> matchPhrasePrefixQuery.build(new MatchPhraseExpression(arguments))); } @@ -47,7 +50,8 @@ public void test_SyntaxCheckException_when_one_argument() { @Test public void test_SyntaxCheckException_when_invalid_parameter() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2"), DSL.namedArgument("unsupported", "3")); Assertions.assertThrows(SemanticCheckException.class, @@ -57,7 +61,8 @@ public void test_SyntaxCheckException_when_invalid_parameter() { @Test public void test_analyzer_parameter() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("analyzer", "standard") ); @@ -67,7 +72,8 @@ public void test_analyzer_parameter() { @Test public void build_succeeds_with_two_arguments() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2")); Assertions.assertNotNull(matchPhrasePrefixQuery.build(new MatchPhraseExpression(arguments))); } @@ -75,7 +81,8 @@ public void build_succeeds_with_two_arguments() { @Test public void test_slop_parameter() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("slop", "2") ); @@ -85,7 +92,8 @@ public void test_slop_parameter() { @Test public void test_zero_terms_query_parameter() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "ALL") ); @@ -95,7 +103,8 @@ public void test_zero_terms_query_parameter() { @Test public void test_zero_terms_query_parameter_lower_case() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "all") ); @@ -105,7 +114,8 @@ public void test_zero_terms_query_parameter_lower_case() { @Test public void test_boost_parameter() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("boost", "0.1") ); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java index 6a298326b7..dd6296279c 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java @@ -20,8 +20,10 @@ import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.env.Environment; import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchPhraseQuery; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) @@ -41,7 +43,8 @@ public void test_SyntaxCheckException_when_no_arguments() { @Test public void test_SyntaxCheckException_when_one_argument() { - List arguments = List.of(DSL.namedArgument("field", "test")); + List arguments = List.of(DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD))); assertThrows(SyntaxCheckException.class, () -> matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } @@ -49,7 +52,8 @@ public void test_SyntaxCheckException_when_one_argument() { @Test public void test_SyntaxCheckException_when_invalid_parameter() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2"), DSL.namedArgument("unsupported", "3")); Assertions.assertThrows(SemanticCheckException.class, @@ -59,7 +63,8 @@ public void test_SyntaxCheckException_when_invalid_parameter() { @Test public void test_analyzer_parameter() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("analyzer", "standard") ); @@ -69,7 +74,8 @@ public void test_analyzer_parameter() { @Test public void build_succeeds_with_two_arguments() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2")); Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } @@ -77,7 +83,8 @@ public void build_succeeds_with_two_arguments() { @Test public void test_slop_parameter() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("slop", "2") ); @@ -87,7 +94,8 @@ public void test_slop_parameter() { @Test public void test_zero_terms_query_parameter() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "ALL") ); @@ -97,7 +105,8 @@ public void test_zero_terms_query_parameter() { @Test public void test_zero_terms_query_parameter_lower_case() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "all") ); @@ -114,7 +123,8 @@ public void test_SyntaxCheckException_when_no_arguments_match_phrase_syntax() { @Test public void test_SyntaxCheckException_when_one_argument_match_phrase_syntax() { - List arguments = List.of(DSL.namedArgument("field", "test")); + List arguments = List.of(DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD))); assertThrows(SyntaxCheckException.class, () -> matchPhraseQuery.build(new MatchPhraseExpression( arguments, matchPhraseWithUnderscoreName))); @@ -124,7 +134,8 @@ public void test_SyntaxCheckException_when_one_argument_match_phrase_syntax() { @Test public void test_SyntaxCheckException_when_invalid_parameter_match_phrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2"), DSL.namedArgument("unsupported", "3")); Assertions.assertThrows(SemanticCheckException.class, @@ -135,7 +146,8 @@ public void test_SyntaxCheckException_when_invalid_parameter_match_phrase_syntax @Test public void test_analyzer_parameter_match_phrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("analyzer", "standard") ); @@ -146,7 +158,8 @@ public void test_analyzer_parameter_match_phrase_syntax() { @Test public void build_succeeds_with_two_arguments_match_phrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2")); Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression( arguments, matchPhraseWithUnderscoreName))); @@ -155,7 +168,8 @@ public void build_succeeds_with_two_arguments_match_phrase_syntax() { @Test public void test_slop_parameter_match_phrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("slop", "2") ); @@ -166,7 +180,8 @@ public void test_slop_parameter_match_phrase_syntax() { @Test public void test_zero_terms_query_parameter_match_phrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "ALL") ); @@ -177,7 +192,8 @@ public void test_zero_terms_query_parameter_match_phrase_syntax() { @Test public void test_zero_terms_query_parameter_lower_case_match_phrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "all") ); @@ -195,7 +211,8 @@ public void test_SyntaxCheckException_when_no_arguments_matchphrase_syntax() { @Test public void test_SyntaxCheckException_when_one_argument_matchphrase_syntax() { - List arguments = List.of(DSL.namedArgument("field", "test")); + List arguments = List.of(DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD))); assertThrows(SyntaxCheckException.class, () -> matchPhraseQuery.build(new MatchPhraseExpression( arguments, matchPhraseQueryName))); @@ -205,7 +222,8 @@ public void test_SyntaxCheckException_when_one_argument_matchphrase_syntax() { @Test public void test_SyntaxCheckException_when_invalid_parameter_matchphrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2"), DSL.namedArgument("unsupported", "3")); Assertions.assertThrows(SemanticCheckException.class, @@ -216,7 +234,8 @@ public void test_SyntaxCheckException_when_invalid_parameter_matchphrase_syntax( @Test public void test_analyzer_parameter_matchphrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("analyzer", "standard") ); @@ -227,7 +246,8 @@ public void test_analyzer_parameter_matchphrase_syntax() { @Test public void build_succeeds_with_two_arguments_matchphrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "test"), + DSL.namedArgument("field", + new ReferenceExpression("test", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "test2")); Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression( arguments, matchPhraseQueryName))); @@ -236,7 +256,8 @@ public void build_succeeds_with_two_arguments_matchphrase_syntax() { @Test public void test_slop_parameter_matchphrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("slop", "2") ); @@ -247,7 +268,8 @@ public void test_slop_parameter_matchphrase_syntax() { @Test public void test_zero_terms_query_parameter_matchphrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "ALL") ); @@ -258,7 +280,8 @@ public void test_zero_terms_query_parameter_matchphrase_syntax() { @Test public void test_zero_terms_query_parameter_lower_case_matchphrase_syntax() { List arguments = List.of( - DSL.namedArgument("field", "t1"), + DSL.namedArgument("field", + new ReferenceExpression("t1", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", "t2"), DSL.namedArgument("zero_terms_query", "all") ); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java index e18b477745..f7d1f1aa9a 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -23,8 +23,10 @@ import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.NamedArgumentExpression; +import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.env.Environment; import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchQuery; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) @@ -39,71 +41,85 @@ public class MatchQueryTest { static Stream> generateValidData() { return Stream.of( List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("analyzer", DSL.literal("standard")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("auto_generate_synonyms_phrase_query", DSL.literal("true")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("fuzziness", DSL.literal("AUTO")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("max_expansions", DSL.literal("50")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("prefix_length", DSL.literal("0")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("fuzzy_transpositions", DSL.literal("true")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("fuzzy_rewrite", DSL.literal("constant_score")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("lenient", DSL.literal("false")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("operator", DSL.literal("OR")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("minimum_should_match", DSL.literal("3")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("zero_terms_query", DSL.literal("NONE")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("zero_terms_query", DSL.literal("none")) ), List.of( - DSL.namedArgument("field", DSL.literal("field_value")), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), DSL.namedArgument("query", DSL.literal("query_value")), DSL.namedArgument("boost", DSL.literal("1")) ) @@ -133,7 +149,8 @@ public void test_SyntaxCheckException_when_one_argument() { @Test public void test_SemanticCheckException_when_invalid_parameter() { List arguments = List.of( - namedArgument("field", "field_value"), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), namedArgument("query", "query_value"), namedArgument("unsupported", "unsupported_value")); Assertions.assertThrows(SemanticCheckException.class, @@ -166,7 +183,8 @@ public void test_SyntaxCheckException_when_one_argument_matchquery_syntax() { @Test public void test_SemanticCheckException_when_invalid_parameter_matchquery_syntax() { List arguments = List.of( - namedArgument("field", "field_value"), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), namedArgument("query", "query_value"), namedArgument("unsupported", "unsupported_value")); Assertions.assertThrows(SemanticCheckException.class, @@ -200,7 +218,8 @@ public void test_SyntaxCheckException_when_one_argument_match_query_syntax() { @Test public void test_SemanticCheckException_when_invalid_parameter_match_query_syntax() { List arguments = List.of( - namedArgument("field", "field_value"), + DSL.namedArgument("field", + new ReferenceExpression("field_value", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), namedArgument("query", "query_value"), namedArgument("unsupported", "unsupported_value")); Assertions.assertThrows(SemanticCheckException.class, diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/WildcardQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/WildcardQueryTest.java index ce7a39d91a..684036595c 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/WildcardQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/WildcardQueryTest.java @@ -22,8 +22,10 @@ import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.env.Environment; import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.WildcardQuery; @DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) @@ -34,7 +36,8 @@ class WildcardQueryTest { static Stream> generateValidData() { return Stream.of( List.of( - namedArgument("field", "title"), + namedArgument("field", + new ReferenceExpression("title", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), namedArgument("query", "query_value*"), namedArgument("boost", "0.7"), namedArgument("case_insensitive", "false"), @@ -59,7 +62,8 @@ public void test_SyntaxCheckException_when_no_arguments() { @Test public void test_SyntaxCheckException_when_one_argument() { - List arguments = List.of(namedArgument("field", "title")); + List arguments = List.of(namedArgument("field", + new ReferenceExpression("title", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD))); assertThrows(SyntaxCheckException.class, () -> wildcardQueryQuery.build(new WildcardQueryExpression(arguments))); } @@ -67,7 +71,8 @@ public void test_SyntaxCheckException_when_one_argument() { @Test public void test_SemanticCheckException_when_invalid_parameter() { List arguments = List.of( - namedArgument("field", "title"), + namedArgument("field", + new ReferenceExpression("title", OpenSearchDataType.OPENSEARCH_TEXT_KEYWORD)), namedArgument("query", "query_value*"), namedArgument("unsupported", "unsupported_value")); Assertions.assertThrows(SemanticCheckException.class, diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java index 7211cc1b8b..67f22178bc 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SingleFieldQueryTest.java @@ -37,7 +37,7 @@ void setUp() { } @Test - void createQueryBuilderQualifiedNameTest() { + void createQueryBuilderTestTypeTextKeyword() { String sampleQuery = "sample query"; String sampleField = "fieldA"; @@ -49,4 +49,18 @@ void createQueryBuilderQualifiedNameTest() { verify(query).createBuilder(eq(sampleField), eq(sampleQuery)); } + + @Test + void createQueryBuilderTestTypeText() { + String sampleQuery = "sample query"; + String sampleField = "fieldA"; + + query.createQueryBuilder(List.of(DSL.namedArgument("field", + new ReferenceExpression(sampleField, OpenSearchDataType.OPENSEARCH_TEXT)), + DSL.namedArgument("query", + new LiteralExpression(ExprValueUtils.stringValue(sampleQuery))))); + + verify(query).createBuilder(eq(sampleField), + eq(sampleQuery)); + } }