Simplified Linear & RRF Retrievers - Return error on empty fields param (#129962)

This commit is contained in:
Mike Pellegrini 2025-06-24 19:25:24 -04:00 committed by GitHub
parent c3e4f4c189
commit 651bc39565
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 46 additions and 8 deletions

View File

@ -0,0 +1,5 @@
pr: 129962
summary: Simplified Linear & RRF Retrievers - Return error on empty fields param
area: Search
type: bug
issues: []

View File

@ -56,7 +56,7 @@ public class MultiFieldsInnerRetrieverUtils {
*/ */
public static ActionRequestValidationException validateParams( public static ActionRequestValidationException validateParams(
List<CompoundRetrieverBuilder.RetrieverSource> innerRetrievers, List<CompoundRetrieverBuilder.RetrieverSource> innerRetrievers,
List<String> fields, @Nullable List<String> fields,
@Nullable String query, @Nullable String query,
String retrieverName, String retrieverName,
String retrieversParamName, String retrieversParamName,
@ -64,7 +64,7 @@ public class MultiFieldsInnerRetrieverUtils {
String queryParamName, String queryParamName,
ActionRequestValidationException validationException ActionRequestValidationException validationException
) { ) {
if (fields.isEmpty() == false || query != null) { if (fields != null || query != null) {
// Using the multi-fields query format // Using the multi-fields query format
if (query == null) { if (query == null) {
// Return early here because the following validation checks assume a query param value is provided // Return early here because the following validation checks assume a query param value is provided
@ -87,6 +87,13 @@ public class MultiFieldsInnerRetrieverUtils {
); );
} }
if (fields != null && fields.isEmpty()) {
validationException = addValidationError(
String.format(Locale.ROOT, "[%s] [%s] cannot be empty", retrieverName, fieldsParamName),
validationException
);
}
if (innerRetrievers.isEmpty() == false) { if (innerRetrievers.isEmpty() == false) {
validationException = addValidationError( validationException = addValidationError(
String.format(Locale.ROOT, "[%s] cannot combine [%s] and [%s]", retrieverName, retrieversParamName, queryParamName), String.format(Locale.ROOT, "[%s] cannot combine [%s] and [%s]", retrieverName, retrieversParamName, queryParamName),
@ -131,13 +138,15 @@ public class MultiFieldsInnerRetrieverUtils {
* @return The inner retriever tree as a {@code RetrieverBuilder} list * @return The inner retriever tree as a {@code RetrieverBuilder} list
*/ */
public static List<RetrieverBuilder> generateInnerRetrievers( public static List<RetrieverBuilder> generateInnerRetrievers(
List<String> fieldsAndWeights, @Nullable List<String> fieldsAndWeights,
String query, String query,
Collection<IndexMetadata> indicesMetadata, Collection<IndexMetadata> indicesMetadata,
Function<List<WeightedRetrieverSource>, CompoundRetrieverBuilder<?>> innerNormalizerGenerator, Function<List<WeightedRetrieverSource>, CompoundRetrieverBuilder<?>> innerNormalizerGenerator,
@Nullable Consumer<Float> weightValidator @Nullable Consumer<Float> weightValidator
) { ) {
Map<String, Float> parsedFieldsAndWeights = QueryParserHelper.parseFieldsAndWeights(fieldsAndWeights); Map<String, Float> parsedFieldsAndWeights = fieldsAndWeights != null
? QueryParserHelper.parseFieldsAndWeights(fieldsAndWeights)
: Map.of();
if (weightValidator != null) { if (weightValidator != null) {
parsedFieldsAndWeights.values().forEach(weightValidator); parsedFieldsAndWeights.values().forEach(weightValidator);
} }

View File

@ -163,7 +163,7 @@ public final class LinearRetrieverBuilder extends CompoundRetrieverBuilder<Linea
throw new IllegalArgumentException("The number of normalizers must match the number of inner retrievers"); throw new IllegalArgumentException("The number of normalizers must match the number of inner retrievers");
} }
this.fields = fields == null ? List.of() : List.copyOf(fields); this.fields = fields == null ? null : List.copyOf(fields);
this.query = query; this.query = query;
this.normalizer = normalizer; this.normalizer = normalizer;
this.weights = weights; this.weights = weights;
@ -400,7 +400,7 @@ public final class LinearRetrieverBuilder extends CompoundRetrieverBuilder<Linea
builder.endArray(); builder.endArray();
} }
if (fields.isEmpty() == false) { if (fields != null) {
builder.startArray(FIELDS_FIELD.getPreferredName()); builder.startArray(FIELDS_FIELD.getPreferredName());
for (String field : fields) { for (String field : fields) {
builder.value(field); builder.value(field);

View File

@ -115,7 +115,7 @@ public final class RRFRetrieverBuilder extends CompoundRetrieverBuilder<RRFRetri
) { ) {
// Use a mutable list for childRetrievers so that we can use addChild // Use a mutable list for childRetrievers so that we can use addChild
super(childRetrievers == null ? new ArrayList<>() : new ArrayList<>(childRetrievers), rankWindowSize); super(childRetrievers == null ? new ArrayList<>() : new ArrayList<>(childRetrievers), rankWindowSize);
this.fields = fields == null ? List.of() : List.copyOf(fields); this.fields = fields == null ? null : List.copyOf(fields);
this.query = query; this.query = query;
this.rankConstant = rankConstant; this.rankConstant = rankConstant;
} }
@ -293,7 +293,7 @@ public final class RRFRetrieverBuilder extends CompoundRetrieverBuilder<RRFRetri
builder.endArray(); builder.endArray();
} }
if (fields.isEmpty() == false) { if (fields != null) {
builder.startArray(FIELDS_FIELD.getPreferredName()); builder.startArray(FIELDS_FIELD.getPreferredName());
for (String field : fields) { for (String field : fields) {
builder.value(field); builder.value(field);

View File

@ -465,6 +465,18 @@ setup:
- contains: { error.root_cause.0.reason: "[linear] [query] cannot be empty" } - contains: { error.root_cause.0.reason: "[linear] [query] cannot be empty" }
- do:
catch: bad_request
search:
index: test-index
body:
retriever:
linear:
fields: [ ]
query: "foo"
- contains: { error.root_cause.0.reason: "[linear] [fields] cannot be empty" }
- do: - do:
catch: bad_request catch: bad_request
search: search:

View File

@ -359,6 +359,18 @@ setup:
- contains: { error.root_cause.0.reason: "[rrf] [query] cannot be empty" } - contains: { error.root_cause.0.reason: "[rrf] [query] cannot be empty" }
- do:
catch: bad_request
search:
index: test-index
body:
retriever:
rrf:
fields: [ ]
query: "foo"
- contains: { error.root_cause.0.reason: "[rrf] [fields] cannot be empty" }
- do: - do:
catch: bad_request catch: bad_request
search: search: