[BugFix] Return 400 instead of 500 on vectorSearch() arg-count mismatch

A vectorSearch() call with more arguments than its resolved signature
declares (for example a duplicate named argument such as
table='x', table='x') crashed with an unchecked IndexOutOfBoundsException
surfaced as HTTP 500. The resolver returns a fixed-arity signature
regardless of how many arguments it was called with, so castArguments
looped over the supplied arguments while indexing into the shorter
resolved-type list and ran off the end.

Guard the argument count against the resolved signature in
castArguments and throw an ExpressionEvaluationException, which maps to a
clean 400, before any indexing. This protects every custom function
resolver, not only vectorSearch().

Signed-off-by: Eric Wei <mengwei.eric@gmail.com>

Author: mengweieric

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

@@ -162,7 +162,7 @@ private Optional<FunctionBuilder> resolve(
           || sourceTypes.equals(targetTypes)) {
         return Optional.of(funcBuilder);
       }
-      return Optional.of(castArguments(sourceTypes, targetTypes, funcBuilder));
+      return Optional.of(castArguments(functionName, sourceTypes, targetTypes, funcBuilder));
     } else {
       return Optional.empty();
     }
@@ -175,8 +175,22 @@ private Optional<FunctionBuilder> resolve(
    * this case, wrap F and return equal(BOOL, cast_to_bool(STRING)).
    */
   private FunctionBuilder castArguments(
-      List<ExprType> sourceTypes, List<ExprType> targetTypes, FunctionBuilder funcBuilder) {
+      FunctionName funcName,
+      List<ExprType> sourceTypes,
+      List<ExprType> targetTypes,
+      FunctionBuilder funcBuilder) {
     return (fp, arguments) -> {
+      // A resolver may return a fixed-arity resolved signature regardless of how many arguments it
+      // was called with (e.g. a table function called with a duplicate named argument). Reject the
+      // mismatch with a clean ExpressionEvaluationException instead of indexing past the end of the
+      // resolved type list, which would surface as an unchecked IndexOutOfBoundsException (HTTP
+      // 500).
+      if (arguments.size() != targetTypes.size()) {
+        throw new ExpressionEvaluationException(
+            String.format(
+                "Expected %d arguments for function %s but received %d",
+                targetTypes.size(), funcName.getFunctionName(), arguments.size()));
+      }
       List<Expression> argsCasted = new ArrayList<>();
       for (int i = 0; i < arguments.size(); i++) {
         Expression arg = arguments.get(i);

core/src/test/java/org/opensearch/sql/expression/function/BuiltinFunctionRepositoryTest.java

@@ -181,6 +181,36 @@ void resolve_should_throw_exception_for_unsupported_conversion() {
     assertEquals(error.getMessage(), "Type conversion to type STRUCT is not supported");
   }
 
+  @Test
+  @DisplayName("resolve should not crash when resolved signature is shorter than the arguments")
+  void resolve_should_not_index_past_resolved_signature() {
+    // A custom FunctionResolver (e.g. vectorSearch) may return a fixed-arity resolved signature
+    // regardless of the number of arguments it was called with. When more arguments are supplied
+    // than the resolved signature declares (e.g. a duplicate named argument), castArguments must
+    // not index past the end of the resolved type list. Before the fix this threw an unchecked
+    // IndexOutOfBoundsException, surfacing as an HTTP 500 instead of the resolver's clean 400.
+    when(mockFunctionName.getFunctionName()).thenReturn("mock");
+
+    FunctionName funcName = mockFunctionName;
+    FunctionSignature sourceSignature =
+        new FunctionSignature(funcName, ImmutableList.of(STRING, STRING));
+    FunctionSignature resolvedSignature = new FunctionSignature(funcName, ImmutableList.of(STRING));
+
+    DefaultFunctionResolver funcResolver = mock(DefaultFunctionResolver.class);
+    FunctionBuilder funcBuilder = mock(FunctionBuilder.class);
+    when(mockMap.containsKey(eq(funcName))).thenReturn(true);
+    when(mockMap.get(eq(funcName))).thenReturn(funcResolver);
+    when(funcResolver.resolve(any())).thenReturn(Pair.of(resolvedSignature, funcBuilder));
+
+    ExpressionEvaluationException error =
+        assertThrows(
+            ExpressionEvaluationException.class,
+            () ->
+                repo.resolve(Collections.emptyList(), sourceSignature)
+                    .apply(functionProperties, ImmutableList.of(mockExpression, mockExpression)));
+    assertEquals("Expected 1 arguments for function mock but received 2", error.getMessage());
+  }
+
   @Test
   @DisplayName("resolve unregistered function should throw exception")
   void resolve_unregistered() {

integ-test/src/test/java/org/opensearch/sql/sql/VectorSearchIT.java

@@ -110,6 +110,26 @@ public void testEmptyVectorRejects() throws IOException {
     assertThat(ex.getMessage(), containsString("must not be empty"));
   }
 
+  @Test
+  public void testDuplicateNamedArgumentRejectsWith400() throws IOException {
+    // A duplicate named argument on top of all four distinct names sends five arguments to the
+    // resolver, exceeding its fixed four-arg signature. This previously crashed with an unchecked
+    // IndexOutOfBoundsException surfaced as HTTP 500; it must now be a clean 400 with a
+    // user-facing message.
+    ResponseException ex =
+        expectThrows(
+            ResponseException.class,
+            () ->
+                executeQuery(
+                    "SELECT v._id FROM vectorSearch(table='t', table='t', field='f', "
+                        + "vector='[1.0]', option='k=5') AS v"));
+
+    assertEquals(400, ex.getResponse().getStatusLine().getStatusCode());
+    // The five-argument case is caught by the repository arity guard before the resolver's
+    // duplicate-name check, so the message reports the argument-count mismatch.
+    assertThat(ex.getMessage(), containsString("Expected 4 arguments"));
+  }
+
   @Test
   public void testInvalidFieldNameRejects() throws IOException {
     ResponseException ex =

opensearch/src/test/java/org/opensearch/sql/opensearch/storage/VectorSearchTableFunctionResolverTest.java

@@ -21,6 +21,7 @@
 import org.opensearch.sql.exception.ExpressionEvaluationException;
 import org.opensearch.sql.expression.DSL;
 import org.opensearch.sql.expression.Expression;
+import org.opensearch.sql.expression.function.BuiltinFunctionRepository;
 import org.opensearch.sql.expression.function.FunctionBuilder;
 import org.opensearch.sql.expression.function.FunctionName;
 import org.opensearch.sql.expression.function.FunctionProperties;
@@ -182,6 +183,64 @@ void resolve_rejectsDuplicateNamedArgument() {
     assertTrue(ex.getMessage().contains("table"));
   }
 
+  @Test
+  void resolverArityGuardRejectsExtraNamedArgument() {
+    // The resolver's own builder runs validateArguments() first, so applying it directly to a
+    // duplicate named argument plus all four required arguments (five total) throws a clean
+    // ExpressionEvaluationException. This covers the resolver's arity guard only; the
+    // repository-level cast path that originally crashed is covered by
+    // resolveThroughRepositoryRejectsExtraNamedArgument and BuiltinFunctionRepositoryTest.
+    VectorSearchTableFunctionResolver resolver =
+        new VectorSearchTableFunctionResolver(client, settings);
+    FunctionName functionName = FunctionName.of("vectorsearch");
+    List<Expression> expressions =
+        List.of(
+            DSL.namedArgument("table", DSL.literal("a")),
+            DSL.namedArgument("table", DSL.literal("b")),
+            DSL.namedArgument("field", DSL.literal("embedding")),
+            DSL.namedArgument("vector", DSL.literal("[1.0]")),
+            DSL.namedArgument("option", DSL.literal("k=5")));
+    FunctionSignature functionSignature =
+        new FunctionSignature(
+            functionName, expressions.stream().map(Expression::type).collect(Collectors.toList()));
+    FunctionBuilder builder = resolver.resolve(functionSignature).getValue();
+
+    ExpressionEvaluationException ex =
+        assertThrows(
+            ExpressionEvaluationException.class,
+            () -> builder.apply(functionProperties, expressions));
+    assertTrue(ex.getMessage().contains("requires 4 arguments"));
+  }
+
+  @Test
+  void resolveThroughRepositoryRejectsExtraNamedArgument() {
+    // Routes the vectorSearch resolver through BuiltinFunctionRepository the way production does
+    // (storage-engine resolver registered as a datasource function). A duplicate named argument
+    // plus all four required arguments gives five arguments against the resolver's fixed four-arg
+    // signature, which exercises the cast-wrapper path that originally crashed with an unchecked
+    // IndexOutOfBoundsException (surfacing as HTTP 500). It must now fail with a clean
+    // ExpressionEvaluationException instead.
+    VectorSearchTableFunctionResolver resolver =
+        new VectorSearchTableFunctionResolver(client, settings);
+    BuiltinFunctionRepository repository = BuiltinFunctionRepository.getInstance();
+    List<Expression> expressions =
+        List.of(
+            DSL.namedArgument("table", DSL.literal("a")),
+            DSL.namedArgument("table", DSL.literal("b")),
+            DSL.namedArgument("field", DSL.literal("embedding")),
+            DSL.namedArgument("vector", DSL.literal("[1.0]")),
+            DSL.namedArgument("option", DSL.literal("k=5")));
+
+    assertThrows(
+        ExpressionEvaluationException.class,
+        () ->
+            repository.compile(
+                functionProperties,
+                List.of(resolver),
+                FunctionName.of("vectorsearch"),
+                expressions));
+  }
+
   @Test
   void resolve_rejectsUnknownArgumentName() {
     VectorSearchTableFunctionResolver resolver =