Adding support for quoted relevance arg parameter keys. Adding multim…

…atchquery alternate syntax support for fields as a list with weights.

Signed-off-by: forestmvey <[email protected]>

core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java

@@ -62,7 +62,7 @@ private static FunctionResolver match_phrase(BuiltinFunctionName matchPhrase) {
   }
 
   private static FunctionResolver multi_match(BuiltinFunctionName matchPhrase) {
-    FunctionName funcName = matchPhrase.getName();
+    FunctionName funcName = BuiltinFunctionName.MULTI_MATCH.getName();
     return new RelevanceFunctionResolver(funcName, STRUCT);
   }
 

sql/src/main/antlr/OpenSearchSQLParser.g4

@@ -348,8 +348,7 @@ multiFieldRelevanceFunction
         LT_SQR_PRTHS field=relevanceFieldAndWeight (COMMA field=relevanceFieldAndWeight)* RT_SQR_PRTHS
         COMMA query=relevanceQuery (COMMA relevanceArg)* RR_BRACKET
     | multiFieldRelevanceFunctionName LR_BRACKET
-              alternateMultiMatchQuery  COMMA alternateMultiMatchField
-              (COMMA alternateMultiMatchOptionalArg)* RR_BRACKET
+        alternateMultiMatchQuery  COMMA alternateMultiMatchField (COMMA relevanceArg)* RR_BRACKET
     ;
 
 convertedDataType
@@ -454,6 +453,7 @@ functionArg
 
 relevanceArg
     : relevanceArgName EQUAL_SYMBOL relevanceArgValue
+    | argName=stringLiteral EQUAL_SYMBOL argVal=relevanceArgValue
     ;
 
 highlightArg
@@ -514,11 +514,6 @@ alternateMultiMatchArgVal
     | stringLiteral
     ;
 
-alternateMultiMatchOptionalArg
-    : relevanceArg
-    | argName=stringLiteral EQUAL_SYMBOL argVal=relevanceArgValue
-    ;
-
 alternateMultiMatchQuery
     : argName=alternateMultiMatchArgName EQUAL_SYMBOL argVal=alternateMultiMatchArgVal
     ;

sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java

@@ -52,7 +52,9 @@
 import com.google.common.collect.ImmutableMap;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 import org.antlr.v4.runtime.RuleContext;
 import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -79,6 +81,7 @@
 import org.opensearch.sql.ast.expression.When;
 import org.opensearch.sql.ast.expression.WindowFunction;
 import org.opensearch.sql.ast.tree.Sort.SortOption;
+import org.opensearch.sql.catalog.model.auth.AuthenticationType;
 import org.opensearch.sql.common.utils.StringUtils;
 import org.opensearch.sql.exception.SemanticCheckException;
 import org.opensearch.sql.expression.function.BuiltinFunctionName;
@@ -403,14 +406,11 @@ public UnresolvedExpression visitMultiFieldRelevanceFunction(
       MultiFieldRelevanceFunctionContext ctx) {
     // To support alternate syntax for MULTI_MATCH like
     // 'MULTI_MATCH('query'='query_val', 'fields'='*fields_val')'
-    if ((StringUtils.unquoteText(ctx.multiFieldRelevanceFunctionName().getText().toUpperCase())
-            .equals(BuiltinFunctionName.MULTI_MATCH.toString())
-        || StringUtils.unquoteText(ctx.multiFieldRelevanceFunctionName().getText().toUpperCase())
-            .equals(BuiltinFunctionName.MULTIMATCH.toString())
-        || StringUtils.unquoteText(ctx.multiFieldRelevanceFunctionName().getText().toUpperCase())
-            .equals(BuiltinFunctionName.MULTIMATCHQUERY.toString()))
-        && ! ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchQueryContext.class)
-        .isEmpty()) {
+    String funcName = StringUtils.unquoteText(ctx.multiFieldRelevanceFunctionName().getText());
+    if ((funcName.equalsIgnoreCase(BuiltinFunctionName.MULTI_MATCH.toString())
+        || funcName.equalsIgnoreCase(BuiltinFunctionName.MULTIMATCH.toString())
+        || funcName.equalsIgnoreCase(BuiltinFunctionName.MULTIMATCHQUERY.toString()))
+        && ! ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchQueryContext.class).isEmpty()) {
       return new Function(
           ctx.multiFieldRelevanceFunctionName().getText().toLowerCase(),
           alternateMultiMatchArguments(ctx));
@@ -456,9 +456,13 @@ private QualifiedName visitIdentifiers(List<IdentContext> identifiers) {
 
   private void fillRelevanceArgs(List<OpenSearchSQLParser.RelevanceArgContext> args,
                                  ImmutableList.Builder<UnresolvedExpression> builder) {
-    args.forEach(v -> builder.add(new UnresolvedArgument(
-            v.relevanceArgName().getText().toLowerCase(), new Literal(StringUtils.unquoteText(
-                    v.relevanceArgValue().getText()), DataType.STRING))));
+    // To support old syntax we must support argument keys as quoted strings.
+    args.forEach(v -> builder.add(v.relevanceArgName() != null
+        ? new UnresolvedArgument(v.relevanceArgName().getText().toLowerCase(),
+            new Literal(StringUtils.unquoteText(v.relevanceArgValue().getText()),
+            DataType.STRING))
+        : new UnresolvedArgument(StringUtils.unquoteText(v.argName.getText()).toLowerCase(),
+            new Literal(StringUtils.unquoteText(v.argVal.getText()), DataType.STRING))));
   }
 
   private List<UnresolvedExpression> noFieldRelevanceArguments(
@@ -516,13 +520,19 @@ private List<UnresolvedExpression> alternateMultiMatchArguments(
     // all the arguments are defaulted to string values
     // to skip environment resolving and function signature resolving
     ImmutableList.Builder<UnresolvedExpression> builder = ImmutableList.builder();
-    ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchFieldContext.class)
-        .stream().findFirst().ifPresent(
-          arg ->
-          builder.add(new UnresolvedArgument("fields",
-              new RelevanceFieldList(
-                  ImmutableMap.of(StringUtils.unquoteText(arg.argVal.getText()), 1F))))
-        );
+    Map<String, Float> fieldAndWeightMap = new HashMap<>();
+
+    String[] fieldAndWeights = StringUtils.unquoteText(
+        ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchFieldContext.class)
+        .stream().findFirst().get().argVal.getText()).split(",");
+
+    for (var fieldAndWeight : fieldAndWeights) {
+      String[] splitFieldAndWeights = fieldAndWeight.split("\\^");
+      fieldAndWeightMap.put(splitFieldAndWeights[0],
+          splitFieldAndWeights.length > 1 ? Float.parseFloat(splitFieldAndWeights[1]) : 1F);
+    }
+    builder.add(new UnresolvedArgument("fields",
+        new RelevanceFieldList(fieldAndWeightMap)));
 
     ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchQueryContext.class)
         .stream().findFirst().ifPresent(
@@ -531,14 +541,7 @@ private List<UnresolvedExpression> alternateMultiMatchArguments(
                 new Literal(StringUtils.unquoteText(arg.argVal.getText()), DataType.STRING)))
         );
 
-    // To support old syntax we must support argument keys as quoted strings.
-    ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchOptionalArgContext.class)
-        .forEach(v -> builder.add(v.relevanceArg() != null
-            ? new UnresolvedArgument(v.relevanceArg().relevanceArgName().getText().toLowerCase(),
-                new Literal(StringUtils.unquoteText(v.relevanceArg().relevanceArgValue().getText()),
-                    DataType.STRING))
-            : new UnresolvedArgument(StringUtils.unquoteText(v.argName.getText()).toLowerCase(),
-                new Literal(StringUtils.unquoteText(v.argVal.getText()), DataType.STRING))));
+    fillRelevanceArgs(ctx.relevanceArg(), builder);
 
     return builder.build();
   }

sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java

@@ -7,7 +7,6 @@
 package org.opensearch.sql.sql.parser;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.opensearch.sql.ast.dsl.AstDSL.aggregate;
 import static org.opensearch.sql.ast.dsl.AstDSL.and;
 import static org.opensearch.sql.ast.dsl.AstDSL.booleanLiteral;
@@ -38,7 +37,6 @@
 import java.util.HashMap;
 import org.antlr.v4.runtime.CommonTokenStream;
 import org.apache.commons.lang3.tuple.ImmutablePair;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.opensearch.sql.ast.Node;
 import org.opensearch.sql.ast.dsl.AstDSL;
@@ -48,7 +46,6 @@
 import org.opensearch.sql.ast.tree.Sort.SortOption;
 import org.opensearch.sql.common.antlr.CaseInsensitiveCharStream;
 import org.opensearch.sql.common.antlr.SyntaxAnalysisErrorListener;
-import org.opensearch.sql.exception.SemanticCheckException;
 import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLLexer;
 import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser;
 
@@ -504,25 +501,25 @@ public void relevanceMulti_match() {
             unresolvedArg("analyzer", stringLiteral("keyword")),
             unresolvedArg("operator", stringLiteral("AND"))),
         buildExprAst("multi_match(['field1', 'field2' ^ 3.2], 'search query',"
-            + "analyzer='keyword', operator='AND')"));
+            + "analyzer='keyword', 'operator'='AND')"));
   }
 
   @Test
   public void relevanceMultimatch_alternate_parameter_syntax() {
     assertEquals(AstDSL.function("multimatch",
             unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of(
-                "field", 1F))),
+                "field1", 1F, "field2", 2F))),
             unresolvedArg("query", stringLiteral("search query"))),
-        buildExprAst("multimatch(query='search query', fields='field')")
+        buildExprAst("multimatch(query='search query', fields=['field1^1.0,field2^2.0'])")
     );
 
     assertEquals(AstDSL.function("multimatch",
             unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of(
-                "field", 1F))),
+                "field1", 1F, "field2", 2F))),
             unresolvedArg("query", stringLiteral("search query")),
             unresolvedArg("analyzer", stringLiteral("keyword")),
             unresolvedArg("operator", stringLiteral("AND"))),
-        buildExprAst("multimatch(query='search query', fields='field',"
+        buildExprAst("multimatch(query='search query', fields=['field1^1.0,field2^2.0'],"
             + "analyzer='keyword', operator='AND')"));
   }