Skip to content

Commit 7d2c62a

Browse files
committed
Add null-arg-name guard and make storage engine test less brittle
- validateNamedArgs() now rejects null/empty arg names defensively, closing a potential NPE if the shared table-function path is later wired into PPL - OpenSearchStorageEngineTest uses contains-check instead of exact collection size assertion - Add testNullArgNameThrows test Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent c183c01 commit 7d2c62a

3 files changed

Lines changed: 26 additions & 4 deletions

File tree

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/VectorSearchTableFunctionImplementation.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ static Map<String, String> parseOptions(String optionStr) {
146146
return options;
147147
}
148148

149-
/** Reject non-named arguments early. vectorSearch() requires named args (key=value). */
149+
/** Reject non-named arguments and null arg names early. */
150150
private void validateNamedArgs() {
151151
for (Expression arg : arguments) {
152152
if (!(arg instanceof NamedArgumentExpression)) {
@@ -155,6 +155,12 @@ private void validateNamedArgs() {
155155
+ "but received: "
156156
+ arg.getClass().getSimpleName());
157157
}
158+
String name = ((NamedArgumentExpression) arg).getArgName();
159+
if (name == null || name.isEmpty()) {
160+
throw new ExpressionEvaluationException(
161+
"vectorSearch() requires named arguments (e.g., table='index'), "
162+
+ "but received an argument with no name");
163+
}
158164
}
159165
}
160166

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package org.opensearch.sql.opensearch.storage;
77

88
import static org.junit.jupiter.api.Assertions.assertAll;
9-
import static org.junit.jupiter.api.Assertions.assertEquals;
109
import static org.junit.jupiter.api.Assertions.assertNotNull;
1110
import static org.junit.jupiter.api.Assertions.assertTrue;
1211
import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME;
@@ -43,8 +42,9 @@ public void getTable() {
4342
public void getFunctionsReturnsVectorSearchResolver() {
4443
OpenSearchStorageEngine engine = new OpenSearchStorageEngine(client, settings);
4544
Collection<FunctionResolver> functions = engine.getFunctions();
46-
assertEquals(1, functions.size());
47-
assertTrue(functions.iterator().next() instanceof VectorSearchTableFunctionResolver);
45+
assertTrue(
46+
functions.stream().anyMatch(f -> f instanceof VectorSearchTableFunctionResolver),
47+
"getFunctions() should contain a VectorSearchTableFunctionResolver");
4848
}
4949

5050
@Test

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,22 @@ void testNonNamedArgThrows() {
235235
assertTrue(ex.getMessage().contains("requires named arguments"));
236236
}
237237

238+
@Test
239+
void testNullArgNameThrows() {
240+
FunctionName functionName = FunctionName.of("vectorsearch");
241+
List<Expression> args =
242+
List.of(
243+
DSL.namedArgument(null, DSL.literal("my-index")),
244+
DSL.namedArgument("field", DSL.literal("embedding")),
245+
DSL.namedArgument("vector", DSL.literal("[1.0, 2.0]")),
246+
DSL.namedArgument("option", DSL.literal("k=5")));
247+
VectorSearchTableFunctionImplementation impl =
248+
new VectorSearchTableFunctionImplementation(functionName, args, client, settings);
249+
ExpressionEvaluationException ex =
250+
assertThrows(ExpressionEvaluationException.class, () -> impl.applyArguments());
251+
assertTrue(ex.getMessage().contains("requires named arguments"));
252+
}
253+
238254
private VectorSearchTableFunctionImplementation createImpl() {
239255
return createImplWithArgs("my-index", "embedding", "[1.0, 2.0, 3.0]", "k=5");
240256
}

0 commit comments

Comments
 (0)