From dcc5e173b895011fa0be645395cf429b1f3561f9 Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 9 Nov 2026 14:34:30 -0800 Subject: [PATCH 1/7] parse findAndRerank single leg sorts before planning The command keeps the sort as the user sent it, in FindAndRerankSortDefinition: $hybrid is still read by the deserializer, any other sort is kept as raw JSON. The resolver parses the sort with FindAndRerankSortParser before it calls the planner, so a syntax error in a single leg sort is reported with a SortException code, before the errors the planner finds. FindAndRerankSort is now a sum type: MultiLeg for $hybrid, and VectorLeg, LexicalLeg or FilterLeg, built from SortExpression2, for a single leg sort. RerankLegMode is removed. Single leg sorts are still rejected with UNSUPPORTED_FIND_AND_RERANK_SORT until GH #2602 and GH #2603. --- .../builders/CollectionSortClauseBuilder.java | 26 +- .../command/builders/SortClauseBuilder.java | 15 +- .../clause/sort/FindAndRerankSort.java | 288 ++++++------- .../FindAndRerankSortClauseDeserializer.java | 75 +++- .../sort/FindAndRerankSortDefinition.java | 154 +++++++ .../clause/sort/FindAndRerankSortParser.java | 152 +++++++ .../command/clause/sort/RerankLegMode.java | 13 - .../command/clause/sort/SortExpression2.java | 142 +++++++ .../command/impl/FindAndRerankCommand.java | 6 +- .../api/v1/mcp/CollectionCommandTools.java | 4 +- .../FindAndRerankCommandResolver.java | 6 +- .../resolver/FindAndRerankPlanner.java | 82 ++-- src/main/resources/errors.yaml | 4 +- ...ndAndRerankSortClauseDeserializerTest.java | 287 +++++++------ .../sort/FindAndRerankSortParserTest.java | 381 ++++++++++++++++++ .../clause/sort/SortExpression2Test.java | 50 +++ .../v1/mcp/McpClauseSchemaContractTest.java | 18 +- .../FindAndRerankCommandResolverTest.java | 83 ++++ .../resolver/FindAndRerankPlannerTest.java | 88 +++- 19 files changed, 1529 insertions(+), 345 deletions(-) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java delete mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/RerankLegMode.java create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2Test.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java index f1822cc558..316397352e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortClause; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; @@ -136,6 +137,20 @@ public SortClause buildClauseFromDefinition(ObjectNode sortNode) { * @return {@link SortExpression} for the regular sort */ private SortExpression buildRegularSortExpression(String path, JsonNode innerValue) { + return SortExpression.sort(path, parseAscending(path, innerValue)); + } + + /** + * Validates the value of a "non-special" sort expression, which must be 1 or -1. + * + *

Public and static so the findAndRerank sort parser checks field sorts the same way, see + * {@link FindAndRerankSortParser}. + * + * @param path Path to the field to sort by, used in the error message + * @param innerValue JSON value of the sort expression + * @return true for 1 (ascending), false for -1 (descending) + */ + public static boolean parseAscending(String path, JsonNode innerValue) { if (!innerValue.isInt()) { // Special checking for String and ArrayNode to give less confusing error messages if (innerValue.isArray()) { @@ -172,8 +187,7 @@ private SortExpression buildRegularSortExpression(String path, JsonNode innerVal .formatted(innerValue))); } - boolean ascending = innerValue.intValue() == 1; - return SortExpression.sort(path, ascending); + return innerValue.intValue() == 1; } private void validateSortExpressionPaths(ObjectNode sortNode) { @@ -183,7 +197,13 @@ private void validateSortExpressionPaths(ObjectNode sortNode) { } } - private void validateSortExpressionPath(String path) { + /** + * Validates the path of a "non-special" sort expression, such as address.city. + * + *

Public and static so the findAndRerank sort parser checks field sorts the same way, see + * {@link FindAndRerankSortParser}. + */ + public static void validateSortExpressionPath(String path) { if (!NamingRules.FIELD.apply(path)) { // This is only called after handling "special" sort expressions so simple validation if (path.isEmpty()) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java index a25765591d..76d04f67a4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.EJSONWrapper; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortClause; import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.schema.SchemaObject; @@ -62,7 +63,19 @@ public SortClause build(JsonNode node) { */ protected abstract SortClause buildClauseFromDefinition(ObjectNode sortNode); - protected float[] tryDecodeBinaryVector(String path, JsonNode innerValue) { + /** + * Decodes a $binary vector such as {"$binary": "P4AAAEAAAABAQAAA"}, throws {@link + * SortException.Code#SORT_CLAUSE_VALUE_INVALID} when the value is an object but not a valid + * $binary vector. + * + *

Public and static so the findAndRerank sort parser decodes a $binary vector the same way, + * see {@link FindAndRerankSortParser}. + * + * @param path The sort path the value is for, used in the error message. + * @param innerValue The value of the sort expression. + * @return The vector, or null when the value is not an object. + */ + public static float[] tryDecodeBinaryVector(String path, JsonNode innerValue) { if (innerValue instanceof ObjectNode innerObject) { var ejsonWrapped = EJSONWrapper.maybeFrom(innerObject); if (ejsonWrapped == null || ejsonWrapped.type() != EJSONWrapper.EJSONType.BINARY) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java index 2986063649..5ed4b42e74 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java @@ -1,161 +1,171 @@ package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; -import static io.stargate.sgv2.jsonapi.config.constants.DocumentConstants.Fields.*; - import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.FieldSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.LexicalSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorLegSortExpression; import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; import io.stargate.sgv2.jsonapi.util.recordable.Recordable; -import java.util.*; -import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; -import org.eclipse.microprofile.openapi.annotations.media.Schema; -import org.eclipse.microprofile.openapi.annotations.media.SchemaProperty; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; /** - * The sort clause of a findAndRerank command, see {@link FindAndRerankSortClauseDeserializer} for - * the forms it can take. + * The sort of a findAndRerank command after {@link FindAndRerankSortParser} parsed it, with one + * record for each form of sort. The planner works out the legs to read with from it. + * + *

* - * @param rerankLegMode {@link RerankLegMode#MULTI_LEG} when the sort clause uses $hybrid, otherwise - * {@link RerankLegMode#SINGLE_LEG}. - * @param hybridSort If null then the $hybrid field was not text, OR was blank text, - * otherwise it is the trimmed text. - * @param vectorizeSort If null then the $vectorize field was not present in the sort - * clause, OR was present but set to JSON null OR it was blank string, otherwise it is the value - * provided. - * @param lexicalSort If null then the $lexical field was not present in the sort - * clause, OR was present but set to JSON null OR it was blank string, otherwise it is the value - * provided. - * @param vectorSort If null then the vector field was not present in the sort clause, - * or present and JSON null, otherwise it is the value provided. + *

The command stores the sort as a {@link FindAndRerankSortDefinition}. This type has no form + * for a sort that is not parsed, so code that gets a FindAndRerankSort never sees the JSON the user + * sent. + * + *

NOTE: this is a sealed interface with a record for each form (a sum type), not one record with + * a leg mode and a nullable field for each value (a tagged class). With nullable fields, forms that + * mean different things look the same: with single leg sorts added, {"$lexical": null} + * and {} would both be all nulls. A form with no field of its own does not fit, e.g. a + * field sort. And every user of the sort must know which fields go with which mode. Here each + * record has only the values of its form, and a switch over the sort must handle every form or it + * does not compile. */ -@JsonDeserialize(using = FindAndRerankSortClauseDeserializer.class) -@Schema( - name = "FindAndRerankSort", - type = SchemaType.OBJECT, - implementation = Map.class, - nullable = true, - additionalProperties = Schema.False.class, - properties = - @SchemaProperty( - name = HYBRID_FIELD, - anyOf = {String.class, FindAndRerankSort.HybridSortSchema.class}), - description = - "Use $hybrid with a query string or an object containing $vectorize or $vector, and optionally $lexical." - + " $vector and $vectorize cannot be used together.", - examples = { - """ - {"$hybrid": "Same query for vectorize and BM25 sorting"} - """, - """ - {"$hybrid": {"$vectorize": "vectorize sort query", "$lexical": "lexical sort"}} - """, - """ - {"$hybrid": {"$vector": [1, 2, 3], "$lexical": "lexical sort"}} - """, - """ - {"$hybrid": {"$vector": {"$binary": "P4AAAEAAAABAQAAA"}, "$lexical": "lexical query"}} - """ - }) -public record FindAndRerankSort( - @JsonIgnore RerankLegMode rerankLegMode, - @JsonIgnore String hybridSort, - @JsonIgnore String vectorizeSort, - @JsonIgnore String lexicalSort, - @JsonIgnore float[] vectorSort, - @JsonIgnore CommandFeatures commandFeatures) - implements Recordable { - - // Schema-only classes. @SchemaProperty can point at a nested schema only through a class, so - // these empty classes carry the @Schema for each nested part of the sort. Nothing creates them. - - /** The object form of $hybrid, with $vector or $vectorize and an optional $lexical. */ - @Schema( - name = "HybridSort", - type = SchemaType.OBJECT, - implementation = Object.class, - additionalProperties = Schema.False.class, - properties = { - @SchemaProperty( - name = VECTOR_EMBEDDING_TEXT_FIELD, - type = SchemaType.STRING, - nullable = true), - @SchemaProperty(name = LEXICAL_CONTENT_FIELD, type = SchemaType.STRING, nullable = true), - @SchemaProperty( - name = VECTOR_EMBEDDING_FIELD, - nullable = true, - anyOf = {float[].class, BinaryVectorSchema.class}) - }, - not = ConflictingVectorSortSchema.class) - public static class HybridSortSchema {} - - /** A binary encoded vector such as {"$binary": "P4AAAEAAAABAQAAA"}. */ - @Schema( - name = "BinaryVector", - type = SchemaType.OBJECT, - implementation = Object.class, - nullable = true, - additionalProperties = Schema.False.class, - requiredProperties = BINARY_VECTOR_TEXT_FIELD, - properties = @SchemaProperty(name = BINARY_VECTOR_TEXT_FIELD, type = SchemaType.STRING)) - public static class BinaryVectorSchema {} +public sealed interface FindAndRerankSort extends Recordable + permits FindAndRerankSort.MultiLeg, FindAndRerankSort.SingleLeg { /** - * Matches an object that has both $vector and $vectorize. HybridSort uses it as its {@code not} - * schema, so a sort that sets both fails validation. + * A sort with $hybrid. The command may read with more than one leg, it does not have to: e.g. + * {"$hybrid": "cheese"} on a collection without a lexical index only reads with the + * vector leg. + * + *

TODO: {@link FindAndRerankSortClauseDeserializer} still parses $hybrid, so a MultiLeg is + * also the {@link FindAndRerankSortDefinition} the command stores, and carries the metrics + * features of the sort. When {@link FindAndRerankSortParser} parses $hybrid, a MultiLeg is only a + * FindAndRerankSort. */ - @Schema( - name = "ConflictingVectorSort", - type = SchemaType.OBJECT, - implementation = Object.class, - requiredProperties = {VECTOR_EMBEDDING_FIELD, VECTOR_EMBEDDING_TEXT_FIELD}) - public static class ConflictingVectorSortSchema {} - - public static final FindAndRerankSort NO_ARG_SORT = - new FindAndRerankSort( - RerankLegMode.SINGLE_LEG, null, null, null, null, CommandFeatures.EMPTY); - - public FindAndRerankSort { - Objects.requireNonNull(rerankLegMode, "rerankLegMode must not be null"); - Objects.requireNonNull(commandFeatures, "commandFeatures must not be null"); - } + sealed interface MultiLeg extends FindAndRerankSort, FindAndRerankSortDefinition + permits HybridText, HybridObject {} + + /** + * {"$hybrid": "cheese"}: the same text for the vector leg and the lexical leg. + * + * @param text The trimmed text, null when the user sent blank text. + */ + record HybridText(@JsonIgnore String text, @JsonIgnore CommandFeatures commandFeatures) + implements MultiLeg { + + public HybridText { + Objects.requireNonNull(commandFeatures, "commandFeatures must not be null"); + } - @Override - public DataRecorder recordTo(DataRecorder dataRecorder) { - return dataRecorder - .append("rerankLegMode", rerankLegMode) - .append("hybridSort", hybridSort) - .append("vectorizeSort", vectorizeSort) - .append("lexicalSort", lexicalSort) - .append("vectorSort", Arrays.toString(vectorSort)) - .append("commandFeatures", commandFeatures); + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("text", text).append("commandFeatures", commandFeatures); + } } /** - * Override to do a value equality check on the vector + * {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows"}}, or with $vector: a + * value for each leg. Which values must be set is checked when the command is planned. * - * @param obj the reference object with which to compare. - * @return + * @param vectorize null when $vectorize was left out, null or blank, otherwise the + * trimmed text. + * @param lexical null when $lexical was left out, null or blank, otherwise the + * trimmed text. + * @param vector null when $vector was left out or null, otherwise the vector. + */ + record HybridObject( + @JsonIgnore String vectorize, + @JsonIgnore String lexical, + @JsonIgnore float[] vector, + @JsonIgnore CommandFeatures commandFeatures) + implements MultiLeg { + + public HybridObject { + Objects.requireNonNull(commandFeatures, "commandFeatures must not be null"); + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder + .append("vectorize", vectorize) + .append("lexical", lexical) + .append("vector", Arrays.toString(vector)) + .append("commandFeatures", commandFeatures); + } + + /** Override to do a value equality check on the vector. */ + @Override + public boolean equals(Object obj) { + return (obj instanceof HybridObject other) + && Objects.equals(vectorize, other.vectorize) + && Objects.equals(lexical, other.lexical) + && Arrays.equals(vector, other.vector) + && Objects.equals(commandFeatures, other.commandFeatures); + } + + /** Override to do a value equality hash on the vector. */ + @Override + public int hashCode() { + return Objects.hash(vectorize, lexical, Arrays.hashCode(vector), commandFeatures); + } + } + + /** + * A sort without $hybrid, the command reads with one leg. It has the syntax of the find sort, see + * {@link FindAndRerankSortParser}. The planner rejects every single leg sort until GH #2602 and + * GH #2603. + */ + sealed interface SingleLeg extends FindAndRerankSort permits VectorLeg, LexicalLeg, FilterLeg {} + + /** + * {"$vectorize": "cheese"} or {"$vector": [0.1, 0.2]}: reads with the + * vector leg. */ - @Override - public boolean equals(Object obj) { - return (obj instanceof FindAndRerankSort other) - && rerankLegMode == other.rerankLegMode - && Objects.equals(hybridSort, other.hybridSort) - && Objects.equals(vectorizeSort, other.vectorizeSort) - && Objects.equals(lexicalSort, other.lexicalSort) - && Arrays.equals(vectorSort, other.vectorSort) - && Objects.equals(commandFeatures, other.commandFeatures); + record VectorLeg(VectorLegSortExpression sort) implements SingleLeg { + + public VectorLeg { + Objects.requireNonNull(sort, "sort must not be null"); + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("sort", sort); + } + } + + /** {"$lexical": "cows"}: reads with the lexical leg. */ + record LexicalLeg(LexicalSortExpression sort) implements SingleLeg { + + public LexicalLeg { + Objects.requireNonNull(sort, "sort must not be null"); + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("sort", sort); + } } - /** Override to do a value equality hash on the vector */ - @Override - public int hashCode() { - return Objects.hash( - rerankLegMode, - hybridSort, - vectorizeSort, - lexicalSort, - Arrays.hashCode(vectorSort), - commandFeatures); + /** + * {"age": -1, "name": 1}: reads the documents the filter matches with the filter + * leg, sorted by the fields. + * + * @param sorts The field sorts in the order the user sent them. Empty when the command has no + * sort, a null sort or an empty sort. + */ + record FilterLeg(List sorts) implements SingleLeg { + + public FilterLeg { + sorts = List.copyOf(Objects.requireNonNull(sorts, "sorts must not be null")); + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("sorts", Recordable.copyOf(sorts)); + } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java index a3294cf9a0..ce5cd05883 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java @@ -13,6 +13,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.TextNode; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.EJSONWrapper; +import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.metrics.CommandFeature; import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; import io.stargate.sgv2.jsonapi.util.JsonFieldMatcher; @@ -20,16 +21,26 @@ import java.util.*; /** - * {@link StdDeserializer} for the {@link FindAndRerankSort}. + * {@link StdDeserializer} for the {@link FindAndRerankSortDefinition}. + * + *

A sort with $hybrid is a multi leg sort, and its fields are read here. Any other sort is a + * single leg sort, e.g. {"$vectorize": "text"} or {"age": -1}. It is read + * by field name only and kept as the user sent it. * *

Note: This only checks the shape of the sort clause, the other validation is handled when * resolving the command to an operation. + * + *

NOTE: do not check a single leg sort here, {@link FindAndRerankSortParser} checks it when the + * command is resolved. An {@link APIException} thrown here does not reach MCP users: + * quarkus-mcp-server converts the tool arguments before it calls the tool, and reports any error + * other than a Jackson mapping error as "Internal error". */ -public class FindAndRerankSortClauseDeserializer extends StdDeserializer { +public class FindAndRerankSortClauseDeserializer + extends StdDeserializer { private static final String ERROR_CONTEXT = "`sort` clause"; - // the only field we accept at top level is $hybrid + // when the sort has $hybrid it must be the only field at top level // can be string or object // { "sort" : { "$hybrid" : "i like cheese" } } // { "sort" : { "$hybrid" : { "$vectorize" : "i like cheese", "$lexical" : "cows"} } } @@ -49,18 +60,26 @@ public class FindAndRerankSortClauseDeserializer extends StdDeserializer SINGLE_LEG_FEATURES = + Map.of( + VECTOR_EMBEDDING_TEXT_FIELD, CommandFeature.VECTORIZE, + VECTOR_EMBEDDING_FIELD, CommandFeature.VECTOR, + LEXICAL_CONTENT_FIELD, CommandFeature.LEXICAL); + public FindAndRerankSortClauseDeserializer() { - super(FindAndRerankSort.class); + super(FindAndRerankSortDefinition.class); } /** {@inheritDoc} */ @Override - public FindAndRerankSort deserialize( + public FindAndRerankSortDefinition deserialize( JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { return switch (deserializationContext.readTree(jsonParser)) { case NullNode ignored -> // this is {"sort" : null} - FindAndRerankSort.NO_ARG_SORT; + FindAndRerankSortDefinition.NO_ARG_SORT; case ObjectNode objectNode -> deserialize(jsonParser, objectNode); default -> throw new JsonMappingException( @@ -72,28 +91,33 @@ public FindAndRerankSort deserialize( * Deserialize the sort clause from an object node. * * @param sort The sort clause as an object node, no check for nulls. - * @return The {@link FindAndRerankSort} that reflects the request without validation, e.g. - * checking that a vectorize or vector sort is provided but not both. + * @return The {@link FindAndRerankSortDefinition} that reflects the request without validation, + * e.g. checking that a vectorize or vector sort is provided but not both. */ - private static FindAndRerankSort deserialize(JsonParser jsonParser, ObjectNode sort) + private static FindAndRerankSortDefinition deserialize(JsonParser jsonParser, ObjectNode sort) throws JsonMappingException { // { "sort" : { } } if (sort.isEmpty()) { - return FindAndRerankSort.NO_ARG_SORT; + return FindAndRerankSortDefinition.NO_ARG_SORT; + } + + // no $hybrid, so a single leg sort that is kept as sent, e.g. + // { "sort" : { "$lexical" : "cows" } } or { "sort" : { "age" : -1 } } + if (!sort.has(HYBRID_FIELD)) { + return new FindAndRerankSortDefinition.SingleLegDefinition( + sort, singleLegCommandFeatures(sort)); } + // TODO: $hybrid is parsed here, so MCP users get its errors as "unable to convert value" or + // "Internal error", without the error code. Parse it in FindAndRerankSortParser and keep only + // the sort as sent here, see FindAndRerankSortDefinition var hybridMatch = MATCH_HYBRID_FIELD.matchAndThrow(sort, jsonParser, ERROR_CONTEXT); return switch (hybridMatch.matched().get(HYBRID_FIELD)) { case TextNode textNode -> - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - normalizedText(textNode.asText()), - null, - null, - null, - CommandFeatures.of(CommandFeature.HYBRID)); + new FindAndRerankSort.HybridText( + normalizedText(textNode.asText()), CommandFeatures.of(CommandFeature.HYBRID)); case ObjectNode objectNode -> deserializeHybridObject(jsonParser, objectNode); case JsonNode node -> throw JsonFieldMatcher.errorForWrongType( @@ -101,7 +125,7 @@ private static FindAndRerankSort deserialize(JsonParser jsonParser, ObjectNode s }; } - private static FindAndRerankSort deserializeHybridObject( + private static FindAndRerankSort.HybridObject deserializeHybridObject( JsonParser jsonParser, ObjectNode hybridObject) throws JsonMappingException { var sortMatch = MATCH_SORT_FIELDS.matchAndThrow(hybridObject, jsonParser, ERROR_CONTEXT); @@ -213,8 +237,19 @@ private static FindAndRerankSort deserializeHybridObject( ObjectNode.class); }; - return new FindAndRerankSort( - RerankLegMode.MULTI_LEG, null, vectorizeText, lexicalText, vector, commandFeatures); + return new FindAndRerankSort.HybridObject(vectorizeText, lexicalText, vector, commandFeatures); + } + + /** The metrics features of a single leg sort, from its field names only. */ + private static CommandFeatures singleLegCommandFeatures(ObjectNode sort) { + var commandFeatures = CommandFeatures.create(); + SINGLE_LEG_FEATURES.forEach( + (field, feature) -> { + if (sort.has(field)) { + commandFeatures.addFeature(feature); + } + }); + return commandFeatures; } /** diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java new file mode 100644 index 0000000000..e4ac962122 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java @@ -0,0 +1,154 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; + +import static io.stargate.sgv2.jsonapi.config.constants.DocumentConstants.Fields.*; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.node.MissingNode; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; +import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; +import io.stargate.sgv2.jsonapi.util.recordable.Recordable; +import java.util.Map; +import java.util.Objects; +import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; +import org.eclipse.microprofile.openapi.annotations.media.Schema; +import org.eclipse.microprofile.openapi.annotations.media.SchemaProperty; + +/** + * The sort clause of a findAndRerank command as the command stores it, read by {@link + * FindAndRerankSortClauseDeserializer}. {@link FindAndRerankSortParser} parses it into a {@link + * FindAndRerankSort} when the command is resolved. + * + *

+ * + *

TODO: parse $hybrid in {@link FindAndRerankSortParser} as well, then this type only holds the + * sort as the user sent it, like {@link SortDefinition} does for find. Not done yet because moving + * $hybrid changes its error codes and the order of its errors, so it needs its own change. + * + *

NOTE: the {@link Schema} is named FindAndRerankSort, not after this type, so the + * OpenAPI and MCP schemas keep the name they had before the parsed sort got its own type. It only + * describes $hybrid. Single leg sorts are added to it when they are supported, see GH #2604. + */ +@JsonDeserialize(using = FindAndRerankSortClauseDeserializer.class) +@Schema( + name = "FindAndRerankSort", + type = SchemaType.OBJECT, + implementation = Map.class, + nullable = true, + additionalProperties = Schema.False.class, + properties = + @SchemaProperty( + name = HYBRID_FIELD, + anyOf = {String.class, FindAndRerankSortDefinition.HybridSortSchema.class}), + description = + "Use $hybrid with a query string or an object containing $vectorize or $vector, and optionally $lexical." + + " $vector and $vectorize cannot be used together.", + examples = { + """ + {"$hybrid": "Same query for vectorize and BM25 sorting"} + """, + """ + {"$hybrid": {"$vectorize": "vectorize sort query", "$lexical": "lexical sort"}} + """, + """ + {"$hybrid": {"$vector": [1, 2, 3], "$lexical": "lexical sort"}} + """, + """ + {"$hybrid": {"$vector": {"$binary": "P4AAAEAAAABAQAAA"}, "$lexical": "lexical query"}} + """ + }) +public sealed interface FindAndRerankSortDefinition extends Recordable + permits FindAndRerankSort.MultiLeg, FindAndRerankSortDefinition.SingleLegDefinition { + + /** + * A command with no sort, a null sort or an empty sort. + * + *

NOTE: uses {@link MissingNode} because it cannot be changed, and this instance is shared. + */ + SingleLegDefinition NO_ARG_SORT = + new SingleLegDefinition(MissingNode.getInstance(), CommandFeatures.EMPTY); + + /** + * The metrics features of the sort, recorded when the sort is read. The command adds them to its + * features before it is resolved, so before the sort is parsed. + */ + CommandFeatures commandFeatures(); + + /** + * Any sort without $hybrid, e.g. {"$vectorize": "cheese"}, {"$vector": [0.1, + * 0.2]}, {"$lexical": "cows"} or {"age": -1}. It is kept as the + * user sent it, {@link FindAndRerankSortParser} checks and parses it. + * + * @param sort The sort as the user sent it: an object, or {@link MissingNode} when the command + * has no sort, a null sort or an empty sort. + * @param commandFeatures The metrics features, from the field names of the sort only. + */ + record SingleLegDefinition(@JsonIgnore JsonNode sort, @JsonIgnore CommandFeatures commandFeatures) + implements FindAndRerankSortDefinition { + + public SingleLegDefinition { + Objects.requireNonNull(sort, "sort must not be null"); + Objects.requireNonNull(commandFeatures, "commandFeatures must not be null"); + // the parser reads the sort as an object, see FindAndRerankSortParser + if (!sort.isObject() && !sort.isMissingNode()) { + throw new IllegalArgumentException( + "sort must be an object or MissingNode, sort.getNodeType(): " + sort.getNodeType()); + } + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("sort", sort).append("commandFeatures", commandFeatures); + } + } + + // Schema-only classes. @SchemaProperty can point at a nested schema only through a class, so + // these empty classes carry the @Schema for each nested part of the sort. Nothing creates them. + + /** The object form of $hybrid, with $vector or $vectorize and an optional $lexical. */ + @Schema( + name = "HybridSort", + type = SchemaType.OBJECT, + implementation = Object.class, + additionalProperties = Schema.False.class, + properties = { + @SchemaProperty( + name = VECTOR_EMBEDDING_TEXT_FIELD, + type = SchemaType.STRING, + nullable = true), + @SchemaProperty(name = LEXICAL_CONTENT_FIELD, type = SchemaType.STRING, nullable = true), + @SchemaProperty( + name = VECTOR_EMBEDDING_FIELD, + nullable = true, + anyOf = {float[].class, BinaryVectorSchema.class}) + }, + not = ConflictingVectorSortSchema.class) + class HybridSortSchema {} + + /** A binary encoded vector such as {"$binary": "P4AAAEAAAABAQAAA"}. */ + @Schema( + name = "BinaryVector", + type = SchemaType.OBJECT, + implementation = Object.class, + nullable = true, + additionalProperties = Schema.False.class, + requiredProperties = BINARY_VECTOR_TEXT_FIELD, + properties = @SchemaProperty(name = BINARY_VECTOR_TEXT_FIELD, type = SchemaType.STRING)) + class BinaryVectorSchema {} + + /** + * Matches an object that has both $vector and $vectorize. HybridSort uses it as its {@code not} + * schema, so a sort that sets both fails validation. + */ + @Schema( + name = "ConflictingVectorSort", + type = SchemaType.OBJECT, + implementation = Object.class, + requiredProperties = {VECTOR_EMBEDDING_FIELD, VECTOR_EMBEDDING_TEXT_FIELD}) + class ConflictingVectorSortSchema {} +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java new file mode 100644 index 0000000000..cd39b86ab2 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java @@ -0,0 +1,152 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; + +import static io.stargate.sgv2.jsonapi.config.constants.DocumentConstants.Fields.*; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import io.stargate.sgv2.jsonapi.api.model.command.builders.CollectionSortClauseBuilder; +import io.stargate.sgv2.jsonapi.api.model.command.builders.SortClauseBuilder; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.FieldSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.LexicalSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorizeSortExpression; +import io.stargate.sgv2.jsonapi.exception.SortException; +import io.stargate.sgv2.jsonapi.util.JsonUtil; +import io.stargate.sgv2.jsonapi.util.StringUtil; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Parses the sort of a findAndRerank command, as the command stores it, into a {@link + * FindAndRerankSort}. + * + *

The parser checks only the syntax of the sort, it does not use the collection or the config. + * For example it checks that $lexical has text, not that the collection has a lexical index: the + * checks that need the collection are made when the command is planned. The command resolver calls + * the parser before the planner, so a syntax error in the sort is reported before the errors the + * planner finds, such as a missing rerankOn. + * + *

A single leg sort, one without $hybrid, has the syntax of the find sort: $lexical, $vector or + * $vectorize must be the only field in the sort, any other field is a field sort with the value + * 1 or -1. The errors have the same {@link SortException} codes as find. + * The checks for field sorts and for $binary vectors are the ones find uses, see {@link + * CollectionSortClauseBuilder} and {@link SortClauseBuilder}. + * + *

NOTE: find checks $lexical, $vector and $vectorize in {@link CollectionSortClauseBuilder}, in + * the same method as its lexical index check, which needs the collection, so they are checked again + * here. Two differences from find are on purpose: $lexical must not be blank, and a $lexical that + * is not text gets SORT_CLAUSE_VALUE_INVALID, the same as $vectorize, not SORT_CLAUSE_INVALID. + * TODO: when find moves to {@link SortExpression2}, find should parse its sort here and keep only + * its collection checks, then the copy of these checks in {@link CollectionSortClauseBuilder} goes. + */ +public final class FindAndRerankSortParser { + + private FindAndRerankSortParser() { + // static methods only + } + + /** + * Parses the sort, throws a {@link SortException} when its syntax is not valid. + * + * @param definition The sort as the command stores it. + * @return The parsed sort. + */ + public static FindAndRerankSort parse(FindAndRerankSortDefinition definition) { + Objects.requireNonNull(definition, "definition must not be null"); + + return switch (definition) { + // TODO: FindAndRerankSortClauseDeserializer still parses $hybrid, parse it here instead, + // see FindAndRerankSortDefinition + case FindAndRerankSort.MultiLeg multiLeg -> multiLeg; + case FindAndRerankSortDefinition.SingleLegDefinition singleLeg -> + parseSingleLeg(singleLeg.sort()); + }; + } + + private static FindAndRerankSort.SingleLeg parseSingleLeg(JsonNode sort) { + + // no sort, a null sort or an empty sort: the filter leg, without sorting + if (sort.isEmpty()) { + return new FindAndRerankSort.FilterLeg(List.of()); + } + + // $lexical, $vector and $vectorize are checked in the same order as find, so a sort with more + // than one of them gets the same error + + // { "sort" : { "$lexical" : "cows" } } + if (sort.has(LEXICAL_CONTENT_FIELD)) { + return new FindAndRerankSort.LexicalLeg( + new LexicalSortExpression(text(sort, LEXICAL_CONTENT_FIELD))); + } + + // { "sort" : { "$vector" : [0.1, 0.2, 0.3] } } + if (sort.has(VECTOR_EMBEDDING_FIELD)) { + return new FindAndRerankSort.VectorLeg(new VectorSortExpression(vector(sort))); + } + + // { "sort" : { "$vectorize" : "cheese" } } + if (sort.has(VECTOR_EMBEDDING_TEXT_FIELD)) { + return new FindAndRerankSort.VectorLeg( + new VectorizeSortExpression(text(sort, VECTOR_EMBEDDING_TEXT_FIELD))); + } + + // { "sort" : { "age" : -1, "name" : 1 } } + // every path is checked before any value, the same as find + sort.fieldNames().forEachRemaining(CollectionSortClauseBuilder::validateSortExpressionPath); + var sorts = new ArrayList(sort.size()); + for (var field : sort.properties()) { + sorts.add( + new FieldSortExpression( + field.getKey(), + CollectionSortClauseBuilder.parseAscending(field.getKey(), field.getValue()))); + } + return new FindAndRerankSort.FilterLeg(sorts); + } + + /** The value of $lexical, $vector or $vectorize, which must be the only field in the sort. */ + private static JsonNode onlyField(JsonNode sort, String field) { + if (sort.size() > 1) { + throw SortException.Code.SORT_CLAUSE_INVALID.get( + Map.of( + "problem", + "when sorting by field '%s' no other sort expressions allowed".formatted(field))); + } + return sort.get(field); + } + + /** The trimmed text of $lexical or $vectorize, which must not be blank. */ + private static String text(JsonNode sort, String field) { + var value = onlyField(sort, field); + // textValue() is null when the value is not text + var text = value.textValue(); + if (StringUtil.isNullOrBlank(text)) { + throw SortException.Code.SORT_CLAUSE_VALUE_INVALID.get( + Map.of( + "path", + field, + "problem", + "value must be a non-blank String, not %s".formatted(value))); + } + return text.trim(); + } + + /** The vector of $vector: an array of numbers, or a $binary vector. */ + private static float[] vector(JsonNode sort) { + return switch (onlyField(sort, VECTOR_EMBEDDING_FIELD)) { + case ArrayNode arrayNode -> JsonUtil.arrayNodeToVector(arrayNode); + case ObjectNode objectNode -> + SortClauseBuilder.tryDecodeBinaryVector(VECTOR_EMBEDDING_FIELD, objectNode); + case JsonNode node -> + throw SortException.Code.SORT_CLAUSE_VALUE_INVALID.get( + Map.of( + "path", + VECTOR_EMBEDDING_FIELD, + "problem", + "value must be an Array of numbers or a $binary vector, not %s" + .formatted(JsonUtil.nodeTypeAsString(node)))); + }; + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/RerankLegMode.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/RerankLegMode.java deleted file mode 100644 index fc2d0d7918..0000000000 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/RerankLegMode.java +++ /dev/null @@ -1,13 +0,0 @@ -package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; - -/** - * If a findAndRerank command reads with one leg or with more than one, decided by its sort clause: - * {@link #MULTI_LEG} when the sort clause uses $hybrid, otherwise {@link #SINGLE_LEG}. - * - *

A MULTI_LEG command may still read with only one leg, e.g. {"$hybrid": "cheese"} - * on a collection without a lexical index. - */ -public enum RerankLegMode { - SINGLE_LEG, - MULTI_LEG -} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2.java new file mode 100644 index 0000000000..0f3f605005 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2.java @@ -0,0 +1,142 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; + +import static io.stargate.sgv2.jsonapi.config.constants.DocumentConstants.Fields.*; + +import io.stargate.sgv2.jsonapi.util.StringUtil; +import io.stargate.sgv2.jsonapi.util.recordable.Recordable; +import java.util.Arrays; +import java.util.Objects; + +/** + * One expression in a sort clause, with one record for each kind of sort: by vector similarity, by + * lexical relevance, or by the value of a document field. {@link FindAndRerankSortParser} makes + * them for the single leg sorts of findAndRerank. + * + *

NOTE: this replaces {@link SortExpression}, one class with a nullable field for each kind of + * sort: users of it must check the fields in the right order to know what kind of sort they have. + * Here the kind of sort is the type, each record has only the values of its kind, and a switch over + * the expression must handle every kind or it does not compile. + * + *

NOTE: SortExpression2 is a temporary name while both types are in use: the + * findAndRerank single leg sorts use this type, find and the other commands still use {@link + * SortExpression}. When they move to this type, {@link SortExpression} is deleted and this type + * takes its name. + */ +public sealed interface SortExpression2 extends Recordable + permits SortExpression2.VectorLegSortExpression, + SortExpression2.LexicalSortExpression, + SortExpression2.FieldSortExpression { + + /** + * The path the expression sorts on: $vectorize, $vector or $lexical, or the path of a document + * field such as address.city. + */ + String path(); + + /** A sort by vector similarity: to the vector of a text, or to a vector. */ + sealed interface VectorLegSortExpression extends SortExpression2 + permits VectorizeSortExpression, VectorSortExpression {} + + /** + * {"$vectorize": "cheese"}: sort by similarity to the vector of the text, made by + * the vectorize service of the collection. + * + * @param text The text to vectorize, not blank. + */ + record VectorizeSortExpression(String text) implements VectorLegSortExpression { + + public VectorizeSortExpression { + StringUtil.requireNonBlank(text, "text"); + } + + @Override + public String path() { + return VECTOR_EMBEDDING_TEXT_FIELD; + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("text", text); + } + } + + /** + * {"$vector": [0.1, 0.2, 0.3]}: sort by similarity to the vector. + * + * @param vector The vector. Its dimension is not checked here, that needs the collection. + */ + record VectorSortExpression(float[] vector) implements VectorLegSortExpression { + + public VectorSortExpression { + Objects.requireNonNull(vector, "vector must not be null"); + } + + @Override + public String path() { + return VECTOR_EMBEDDING_FIELD; + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("vector", Arrays.toString(vector)); + } + + /** Override to do a value equality check on the vector. */ + @Override + public boolean equals(Object obj) { + return (obj instanceof VectorSortExpression other) && Arrays.equals(vector, other.vector); + } + + /** Override to do a value equality hash on the vector. */ + @Override + public int hashCode() { + return Arrays.hashCode(vector); + } + } + + /** + * {"$lexical": "cows"}: sort by the lexical (BM25) relevance of the documents to the + * text. + * + * @param text The text to match, not blank. + */ + record LexicalSortExpression(String text) implements SortExpression2 { + + public LexicalSortExpression { + StringUtil.requireNonBlank(text, "text"); + } + + @Override + public String path() { + return LEXICAL_CONTENT_FIELD; + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("text", text); + } + } + + /** + * {"age": -1}: sort by the value of a document field. + * + * @param path The path of the field, e.g. address.city. + * @param ascending true for ascending, from 1 in the sort clause, + * false for descending, from -1. + */ + record FieldSortExpression(String path, boolean ascending) implements SortExpression2 { + + public FieldSortExpression { + Objects.requireNonNull(path, "path must not be null"); + // NOTE: not isBlank(), a blank name such as " " is a valid field name, see FieldNamingRule + if (path.isEmpty()) { + throw new IllegalArgumentException("path must not be empty"); + } + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("path", path).append("ascending", ascending); + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/FindAndRerankCommand.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/FindAndRerankCommand.java index d5a8887fc6..26d9493aa2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/FindAndRerankCommand.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/FindAndRerankCommand.java @@ -20,7 +20,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import io.stargate.sgv2.jsonapi.api.model.command.*; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.FilterDefinition; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortDefinition; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.config.constants.RerankingConstants; import io.stargate.sgv2.jsonapi.metrics.CommandFeature; @@ -41,12 +41,12 @@ public record FindAndRerankCommand( @Valid @JsonProperty("filter") FilterDefinition filterDefinition, @JsonProperty("projection") JsonNode projectionDefinition, - @Valid @JsonProperty("sort") FindAndRerankSort sortClause, + @Valid @JsonProperty("sort") FindAndRerankSortDefinition sortClause, @Valid @Nullable Options options) implements ReadCommand, Filterable, Projectable, Windowable { public FindAndRerankCommand { - sortClause = (sortClause == null) ? FindAndRerankSort.NO_ARG_SORT : sortClause; + sortClause = (sortClause == null) ? FindAndRerankSortDefinition.NO_ARG_SORT : sortClause; } // NOTE: is not VectorSortable because it has its own sort clause. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java index efb89bf227..774b1d83fe 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java @@ -8,7 +8,7 @@ import io.smallrye.mutiny.Uni; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.FilterDefinition; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortDefinition; import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateClause; import io.stargate.sgv2.jsonapi.api.model.command.impl.*; import io.stargate.sgv2.jsonapi.api.model.command.table.definition.indexes.RegularIndexDefinitionDesc; @@ -160,7 +160,7 @@ public Uni findAndRerank( "Fields to include or exclude in the returned documents, for example {\"title\": 1}", required = false) JsonNode projection, - @ToolArg(required = false) FindAndRerankSort sort, + @ToolArg(required = false) FindAndRerankSortDefinition sort, @ToolArg( description = "Options for result limits, reranking, and what the response includes", required = false) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolver.java index 3fe52dd383..981d2c5d39 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolver.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.impl.FindAndRerankCommand; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonApiMetricsConfig; import io.stargate.sgv2.jsonapi.config.OperationsConfig; @@ -55,7 +56,10 @@ public Operation resolveCollectionCommand( throw SchemaException.Code.RERANKING_FEATURE_NOT_ENABLED.get(); } - var plan = FindAndRerankPlanner.plan(command, commandContext); + // NOTE: the sort is parsed before the planner runs, so a syntax error in the sort is reported + // before the errors the planner finds, such as a missing rerankOn + var sort = FindAndRerankSortParser.parse(command.sortClause()); + var plan = FindAndRerankPlanner.plan(command, sort, commandContext); return new FindAndRerankOperationBuilder(commandContext) .withCommand(command) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java index 2d02a5acbc..21a0a324cb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java @@ -6,6 +6,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.impl.FindAndRerankCommand; import io.stargate.sgv2.jsonapi.config.IntConfigWithBounds; import io.stargate.sgv2.jsonapi.config.OperationsConfig; @@ -24,7 +25,14 @@ /** * Validates a {@link FindAndRerankCommand} and translates the intention of the user, expressed in - * the command, into a {@link FindAndRerankPlan} that is used to build the operation that runs it. + * the command and its sort, into a {@link FindAndRerankPlan} that is used to build the operation + * that runs it. The sort comes to the planner already parsed by {@link FindAndRerankSortParser}. + * + *

TODO: move the checks that the command can run on the collection and with the config (the sort + * is supported, hybridLimits, the rerank service, rerankQuery, rerankOn, the projection, the + * $hybrid values) into a binder that runs between the parser and the planner, so the planner only + * works out how to run a command that can run. Not done in the change that added the parser, so the + * move is a change of its own that does not change behaviour. */ class FindAndRerankPlanner { @@ -36,16 +44,19 @@ private FindAndRerankPlanner() { * Validates the command and makes its plan. * * @param command The command to plan. + * @param sort The sort of the command, parsed by {@link FindAndRerankSortParser}. * @param commandContext Context for the command, it has the collection and the config. * @return The plan, it always has a VECTOR_LEG. */ static FindAndRerankPlan plan( - FindAndRerankCommand command, CommandContext commandContext) { + FindAndRerankCommand command, + FindAndRerankSort sort, + CommandContext commandContext) { Objects.requireNonNull(command, "command must not be null"); + Objects.requireNonNull(sort, "sort must not be null"); Objects.requireNonNull(commandContext, "commandContext must not be null"); - var sort = command.sortClause(); var schemaObject = commandContext.schemaObject(); var operationsConfig = commandContext.config().get(OperationsConfig.class); @@ -56,7 +67,7 @@ static FindAndRerankPlan plan( RerankingQuery.create( getOrDefault(command.options(), FindAndRerankCommand.Options::rerankQuery, null), vectorizeText(sort)); - var passageLocator = passageLocator(command); + var passageLocator = passageLocator(command, sort); var projector = command.buildProjector(); // NOTE: the legs are planned after the options and the projection are checked, to keep the @@ -66,11 +77,12 @@ static FindAndRerankPlan plan( // NOTE: a sort with no $vector or $vectorize fails here with a SORT error, not later with an // UNEXPECTED_SERVER_ERROR, see GH #2577 var legs = - switch (sort.rerankLegMode()) { - // TODO: GH #2602, GH #2603 - plan single leg sorts, until then only a command with no - // sort is SINGLE_LEG - case SINGLE_LEG -> throw SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT.get(); - case MULTI_LEG -> multiLegs(command, schemaObject); + switch (sort) { + case FindAndRerankSort.MultiLeg multiLeg -> multiLegs(command, multiLeg, schemaObject); + // TODO: GH #2602, GH #2603 - plan single leg sorts. Until then every sort without + // $hybrid is rejected here, after the checks above + case FindAndRerankSort.SingleLeg ignored -> + throw SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT.get(); }; var vectorizeStep = isVectorizeSort(sort) ? planVectorizeStep(sort, schemaObject) : null; @@ -166,11 +178,12 @@ private static void checkSortSupported( * vector for the vector leg. */ private static List multiLegs( - FindAndRerankCommand command, CollectionSchemaObject schemaObject) { + FindAndRerankCommand command, + FindAndRerankSort.MultiLeg multiLeg, + CollectionSchemaObject schemaObject) { - var sort = command.sortClause(); - if (!isVectorizeSort(sort) && !isVectorSort(sort)) { - throw isLexicalSort(sort) + if (!isVectorizeSort(multiLeg) && !isVectorSort(multiLeg)) { + throw isLexicalSort(multiLeg) ? SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT.get() : SortException.Code.MISSING_HYBRID_SORT_VALUE.get(); } @@ -189,18 +202,21 @@ private static List multiLegs( legs.add( new FindAndRerankPlan.RerankLeg( RerankLegType.VECTOR_LEG, - isVectorizeSort(sort) ? null : sort.vectorSort(), + isVectorizeSort(multiLeg) ? null : vector(multiLeg), null, hybridLimits.vectorLimit())); // the lexical leg is optional: if there is a lexical sort, but the user did not ask explicitly // for it then it is OK to skip when there is no lexical index. - if (isLexicalSort(sort)) { + if (isLexicalSort(multiLeg)) { if (schemaObject.lexicalDef().enabled()) { legs.add( new FindAndRerankPlan.RerankLeg( - RerankLegType.LEXICAL_LEG, null, lexicalText(sort), hybridLimits.lexicalLimit())); - } else if (isExplicitLexicalSort(sort)) { + RerankLegType.LEXICAL_LEG, + null, + lexicalText(multiLeg), + hybridLimits.lexicalLimit())); + } else if (isExplicitLexicalSort(multiLeg)) { // error should have been caught in checkSortSupported() safety here throwIfNoLexicalIndex(schemaObject); } @@ -258,14 +274,15 @@ private static CollectionRerankDef.RerankServiceDef resolveRerankServiceDef( return serviceDef; } - private static PathMatchLocator passageLocator(FindAndRerankCommand command) { + private static PathMatchLocator passageLocator( + FindAndRerankCommand command, FindAndRerankSort sort) { var rerankOn = getOrDefault(command.options(), FindAndRerankCommand.Options::rerankOn, null); var isRerankOn = rerankOn != null && !rerankOn.isBlank(); String finalRerankField; - if (isVectorizeSort(command.sortClause())) { + if (isVectorizeSort(sort)) { // use the vectorize field, unless the user has overridden finalRerankField = isRerankOn ? rerankOn : VECTOR_EMBEDDING_TEXT_FIELD; } else if (isRerankOn) { @@ -284,16 +301,34 @@ private static void throwIfNoLexicalIndex(CollectionSchemaObject schemaObject) { } } + // NOTE: the helpers below return null or false for a SingleLeg sort, because plan() rejects + // single leg sorts until GH #2602 and GH #2603 + /** The text to vectorize: the $hybrid text, or the $vectorize value, or null. */ private static String vectorizeText(FindAndRerankSort sort) { - return sort.hybridSort() != null ? sort.hybridSort() : sort.vectorizeSort(); + return switch (sort) { + case FindAndRerankSort.HybridText hybridText -> hybridText.text(); + case FindAndRerankSort.HybridObject hybridObject -> hybridObject.vectorize(); + case FindAndRerankSort.SingleLeg ignored -> null; + }; } /** * The text for the lexical sort: the $hybrid text, or the $lexical value, or null. */ private static String lexicalText(FindAndRerankSort sort) { - return sort.hybridSort() != null ? sort.hybridSort() : sort.lexicalSort(); + return switch (sort) { + case FindAndRerankSort.HybridText hybridText -> hybridText.text(); + case FindAndRerankSort.HybridObject hybridObject -> hybridObject.lexical(); + case FindAndRerankSort.SingleLeg ignored -> null; + }; + } + + /** The vector from the $hybrid object, or null. */ + private static float[] vector(FindAndRerankSort sort) { + return sort instanceof FindAndRerankSort.HybridObject hybridObject + ? hybridObject.vector() + : null; } private static boolean isLexicalSort(FindAndRerankSort sort) { @@ -302,7 +337,8 @@ private static boolean isLexicalSort(FindAndRerankSort sort) { /** The $hybrid object has $lexical text, so the collection must have a lexical index. */ private static boolean isExplicitLexicalSort(FindAndRerankSort sort) { - return sort.lexicalSort() != null; + return sort instanceof FindAndRerankSort.HybridObject hybridObject + && hybridObject.lexical() != null; } private static boolean isVectorizeSort(FindAndRerankSort sort) { @@ -310,6 +346,6 @@ private static boolean isVectorizeSort(FindAndRerankSort sort) { } private static boolean isVectorSort(FindAndRerankSort sort) { - return sort.vectorSort() != null; + return vector(sort) != null; } } diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index cf77441744..39907c0e30 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -2338,9 +2338,9 @@ request-errors: code: UNSUPPORTED_FIND_AND_RERANK_SORT title: Hybrid sort required by findAndRerank body: |- - The command attempted to run findAndRerank with no sort, or with an empty sort. + The command attempted to run findAndRerank without a `$hybrid` sort. - The findAndRerank command finds the documents to rerank with a `$hybrid` sort, e.g. `{"$hybrid": "query text"}` or `{"$hybrid": {"$vectorize": "query text", "$lexical": "query text"}}`. Reranking the documents matched by the filter alone is not supported. + The findAndRerank command finds the documents to rerank with a `$hybrid` sort, e.g. `{"$hybrid": "query text"}` or `{"$hybrid": {"$vectorize": "query text", "$lexical": "query text"}}`. Sorting by only one of `$vectorize`, `$vector` or `$lexical`, sorting by document fields, and reranking the documents matched by the filter alone are not supported. Resend the command with a `$hybrid` sort. diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java index e3a66280f2..a0ba56f5d8 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java @@ -6,12 +6,19 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; import static org.mockito.Mockito.when; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.TextNode; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridObject; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridText; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortDefinition.SingleLegDefinition; import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; import io.stargate.sgv2.jsonapi.util.recordable.PrettyPrintable; import java.util.stream.Stream; @@ -31,6 +38,8 @@ public class FindAndRerankSortClauseDeserializerTest { private static final Logger LOGGER = LoggerFactory.getLogger(FindAndRerankSortClauseDeserializerTest.class); + private static final ObjectMapper MAPPER = new ObjectMapper(); + private FindAndRerankSortClauseDeserializer deserializer; @Mock private JsonParser jsonParser; @@ -46,64 +55,73 @@ void setUp() { @Test public void testEqualsAndHash() { var value1 = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( + "vectorize sort", + "lexical sort", + new float[] {1.1f, 2.2f, 3.3f}, + CommandFeatures.EMPTY); + var sameValue = + new HybridObject( "vectorize sort", "lexical sort", new float[] {1.1f, 2.2f, 3.3f}, CommandFeatures.EMPTY); var diffVectorize = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize sort 2", "lexical sort", new float[] {1.1f, 2.2f, 3.3f}, CommandFeatures.EMPTY); var diffLexical = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize sort", "lexical sort 2", new float[] {1.1f, 2.2f, 3.3f}, CommandFeatures.EMPTY); var diffVector = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize sort", "lexical sort", new float[] {1.1f, 2.2f, 3.3f, 4.4f}, CommandFeatures.EMPTY); + var diffForm = new HybridText("vectorize sort", CommandFeatures.EMPTY); - var diffMode = - new FindAndRerankSort( - RerankLegMode.SINGLE_LEG, - null, - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, + var singleLeg = + new SingleLegDefinition( + sortJson( + """ + {"$lexical": "cows"} + """), CommandFeatures.EMPTY); - var diffHybridSort = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - "hybrid sort", - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, + var sameSingleLeg = + new SingleLegDefinition( + sortJson( + """ + {"$lexical": "cows"} + """), + CommandFeatures.EMPTY); + var diffSingleLeg = + new SingleLegDefinition( + sortJson( + """ + {"$lexical": "sheep"} + """), CommandFeatures.EMPTY); assertThat(value1).as("Object equals self").isEqualTo(value1); - assertThat(value1).as("different rerankLegMode").isNotEqualTo(diffMode); - assertThat(value1).as("different hybrid sort").isNotEqualTo(diffHybridSort); + assertThat(value1).as("same values, vector compared by value").isEqualTo(sameValue); + assertThat(value1).as("different form").isNotEqualTo(diffForm); assertThat(value1).as("different vectorize sort").isNotEqualTo(diffVectorize); assertThat(value1).as("different lexical sort").isNotEqualTo(diffLexical); assertThat(value1).as("different vector").isNotEqualTo(diffVector); + assertThat(singleLeg).as("same single leg sort").isEqualTo(sameSingleLeg); + assertThat(singleLeg).as("different single leg sort").isNotEqualTo(diffSingleLeg); assertThat(value1.hashCode()).as("hash code equals self").isEqualTo(value1.hashCode()); + assertThat(value1.hashCode()) + .as("hash code same values, vector compared by value") + .isEqualTo(sameValue.hashCode()); assertThat(value1.hashCode()) .as("hash code different vectorize sort") .isNotEqualTo(diffVectorize.hashCode()); @@ -113,23 +131,34 @@ public void testEqualsAndHash() { assertThat(value1.hashCode()) .as("hash code different vector") .isNotEqualTo(diffVector.hashCode()); - assertThat(value1.hashCode()) - .as("hash code different rerankLegMode") - .isNotEqualTo(diffMode.hashCode()); - assertThat(value1.hashCode()) - .as("hash code different hybrid sort") - .isNotEqualTo(diffHybridSort.hashCode()); + assertThat(singleLeg.hashCode()) + .as("hash code same single leg sort") + .isEqualTo(sameSingleLeg.hashCode()); + assertThat(singleLeg.hashCode()) + .as("hash code different single leg sort") + .isNotEqualTo(diffSingleLeg.hashCode()); + } + + /** The parser reads a single leg sort as an object, so other JSON is a bug in the caller. */ + @Test + public void singleLegDefinitionRejectsNonObject() { + var ex = + assertThrowsExactly( + IllegalArgumentException.class, + () -> new SingleLegDefinition(TextNode.valueOf("cows"), CommandFeatures.EMPTY)); + assertThat(ex).hasMessageContaining("sort must be an object or MissingNode"); } @ParameterizedTest @MethodSource("validSortsTestCases") - public void validSorts(String jsonString, FindAndRerankSort expected) throws Exception { + public void validSorts(String jsonString, FindAndRerankSortDefinition expected) throws Exception { LOGGER.debug("validSorts() - jsonString: {}", jsonString); ObjectMapper mapper = new ObjectMapper(); when(deserializationContext.readTree(jsonParser)).thenReturn(mapper.readTree(jsonString)); - FindAndRerankSort actual = deserializer.deserialize(jsonParser, deserializationContext); + FindAndRerankSortDefinition actual = + deserializer.deserialize(jsonParser, deserializationContext); LOGGER.debug("validSorts() - actual: {}", PrettyPrintable.pprint(actual)); assertEquals(expected, actual); @@ -147,57 +176,41 @@ private static Stream validSortsTestCases() { """ {} """, - FindAndRerankSort.NO_ARG_SORT), + FindAndRerankSortDefinition.NO_ARG_SORT), Arguments.of( """ null """, - FindAndRerankSort.NO_ARG_SORT), + FindAndRerankSortDefinition.NO_ARG_SORT), // ---- // $hybrid only Arguments.of( """ { "$hybrid" : "same for hybrid and lexical" } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - "same for hybrid and lexical", - null, - null, - null, - CommandFeatures.of(HYBRID))), + new HybridText("same for hybrid and lexical", CommandFeatures.of(HYBRID))), Arguments.of( """ { "$hybrid" : " padded text " } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - "padded text", - null, - null, - null, - CommandFeatures.of(HYBRID))), + new HybridText("padded text", CommandFeatures.of(HYBRID))), Arguments.of( """ { "$hybrid" : "" } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, null, null, null, null, CommandFeatures.of(HYBRID))), + new HybridText(null, CommandFeatures.of(HYBRID))), Arguments.of( """ { "$hybrid" : " " } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, null, null, null, null, CommandFeatures.of(HYBRID))), + new HybridText(null, CommandFeatures.of(HYBRID))), // ---- // maximum fields, resolver works out the valid combinations Arguments.of( """ { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : "lexical sort", "$vector" : [1.1, 2.2, 3.3]} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize sort", "lexical sort", new float[] {1.1f, 2.2f, 3.3f}, @@ -206,9 +219,7 @@ private static Stream validSortsTestCases() { """ { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : "lexical sort"} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize sort", "lexical sort", null, @@ -219,44 +230,26 @@ private static Stream validSortsTestCases() { """ { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : null} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort", - null, - null, - CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL))), + new HybridObject( + "vectorize sort", null, null, CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL))), Arguments.of( """ { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : ""} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort", - null, - null, - CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL))), + new HybridObject( + "vectorize sort", null, null, CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL))), Arguments.of( """ { "$hybrid" : { "$vectorize" : "vectorize sort"} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort", - null, - null, - CommandFeatures.of(HYBRID, VECTORIZE))), + new HybridObject("vectorize sort", null, null, CommandFeatures.of(HYBRID, VECTORIZE))), // ---- // $vectorize variations Arguments.of( """ { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : "lexical sort"} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize sort", "lexical sort", null, @@ -265,44 +258,26 @@ private static Stream validSortsTestCases() { """ { "$hybrid" : { "$vectorize" : null, "$lexical" : "lexical sort"} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - null, - "lexical sort", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), + new HybridObject( + null, "lexical sort", null, CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), Arguments.of( """ { "$hybrid" : { "$vectorize" : "", "$lexical" : "lexical sort"} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - null, - "lexical sort", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), + new HybridObject( + null, "lexical sort", null, CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), Arguments.of( """ { "$hybrid" : {"$lexical" : "lexical sort"} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - null, - "lexical sort", - null, - CommandFeatures.of(HYBRID, LEXICAL))), + new HybridObject(null, "lexical sort", null, CommandFeatures.of(HYBRID, LEXICAL))), // ---- // $vector variations Arguments.of( """ { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : null} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize", "lexical", null, @@ -311,9 +286,7 @@ private static Stream validSortsTestCases() { """ { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : [0.1, 0.2, 0.3]} } """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize", "lexical", new float[] {0.1f, 0.2f, 0.3f}, @@ -323,9 +296,7 @@ private static Stream validSortsTestCases() { { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } """ .formatted(emptyVectorBase64), - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize", "lexical", emptyVector, @@ -335,15 +306,82 @@ private static Stream validSortsTestCases() { { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } """ .formatted(vectorBase64), - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, + new HybridObject( "vectorize", "lexical", vector, CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR)))); } + /** + * A sort without $hybrid is read by field name only and kept as sent, {@link + * FindAndRerankSortParser} checks the values. + */ + @ParameterizedTest + @MethodSource("singleLegSortsTestCases") + public void singleLegSorts(String jsonString, CommandFeatures expectedFeatures) throws Exception { + + LOGGER.debug("singleLegSorts() - jsonString: {}", jsonString); + when(deserializationContext.readTree(jsonParser)).thenReturn(MAPPER.readTree(jsonString)); + + FindAndRerankSortDefinition actual = + deserializer.deserialize(jsonParser, deserializationContext); + LOGGER.debug("singleLegSorts() - actual: {}", PrettyPrintable.pprint(actual)); + + // the expected sort is a separate parse of the same JSON + assertEquals(new SingleLegDefinition(sortJson(jsonString), expectedFeatures), actual); + } + + private static Stream singleLegSortsTestCases() { + + String vectorBase64 = encodeAsMimeBase64(floatsToBytes(new float[] {1.1f, 2.2f, 3.3f})); + + return Stream.of( + Arguments.of( + """ + { "$vectorize" : "vectorize sort" } + """, + CommandFeatures.of(VECTORIZE)), + Arguments.of( + """ + { "$vector" : [1.1, 2.2, 3.3] } + """, + CommandFeatures.of(VECTOR)), + Arguments.of( + """ + { "$vector" : {"$binary": "%s"} } + """ + .formatted(vectorBase64), + CommandFeatures.of(VECTOR)), + Arguments.of( + """ + { "$lexical" : "lexical sort" } + """, + CommandFeatures.of(LEXICAL)), + Arguments.of( + """ + { "age" : -1, "name" : 1 } + """, + CommandFeatures.EMPTY), + // ---- + // not checked here, FindAndRerankSortParser checks the values + Arguments.of( + """ + { "$lexical" : null } + """, + CommandFeatures.of(LEXICAL)), + Arguments.of( + """ + { "$vector" : [1.1, 2.2, 3.3], "age" : 1 } + """, + CommandFeatures.of(VECTOR)), + Arguments.of( + """ + { "age" : 2 } + """, + CommandFeatures.EMPTY)); + } + @ParameterizedTest @MethodSource("invalidSortsTestCases") public void invalidSorts(String jsonString, String message) throws Exception { @@ -372,7 +410,7 @@ private static Stream invalidSortsTestCases() { "sort clause must be an object"), Arguments.of( """ - { "fake" : 1} + { "$hybrid" : "text", "fake" : 1} """, "Expected fields: $hybrid. Unexpected fields: fake"), Arguments.of( @@ -410,4 +448,13 @@ private static Stream invalidSortsTestCases() { """, "`sort` clause (binary vector) contained fields with the wrong type. Field $vector may only be of types NullNode, ArrayNode, ObjectNode, but got: ObjectNode")); } + + /** The sort JSON as the deserializer gets it. */ + private static JsonNode sortJson(String json) { + try { + return MAPPER.readTree(json); + } catch (JsonProcessingException e) { + throw new IllegalArgumentException("test JSON does not parse: " + json, e); + } + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java new file mode 100644 index 0000000000..120d639a59 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java @@ -0,0 +1,381 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; + +import static io.stargate.sgv2.jsonapi.metrics.CommandFeature.*; +import static io.stargate.sgv2.jsonapi.util.Base64Util.encodeAsMimeBase64; +import static io.stargate.sgv2.jsonapi.util.CqlVectorUtil.floatsToBytes; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.FilterLeg; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridObject; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridText; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.LexicalLeg; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.VectorLeg; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortDefinition.SingleLegDefinition; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.FieldSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.LexicalSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorizeSortExpression; +import io.stargate.sgv2.jsonapi.exception.APIException; +import io.stargate.sgv2.jsonapi.exception.DocumentException; +import io.stargate.sgv2.jsonapi.exception.ErrorCode; +import io.stargate.sgv2.jsonapi.exception.SortException; +import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Tests for {@link FindAndRerankSortParser}: the sort each form of single leg sort parses into, and + * the error for each syntax mistake. + */ +class FindAndRerankSortParserTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + /** The vector in the $vector cases, sent as an array or as $binary. */ + private static final float[] VECTOR = {1.1f, 2.2f, 3.3f}; + + // =========================================================================================== + // $hybrid + // =========================================================================================== + + /** The deserializer still parses $hybrid, the parser returns it unchanged. */ + @Test + void returnsHybridSortUnchanged() { + var hybridText = new HybridText("cheese", CommandFeatures.of(HYBRID)); + var hybridObject = + new HybridObject("cheese", "cows", null, CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL)); + + assertThat(FindAndRerankSortParser.parse(hybridText)).isSameAs(hybridText); + assertThat(FindAndRerankSortParser.parse(hybridObject)).isSameAs(hybridObject); + } + + // =========================================================================================== + // Single leg sorts + // =========================================================================================== + + /** No sort, a null sort and an empty sort are the filter leg, without sorting. */ + @Test + void parsesNoSortToFilterLegWithoutSorts() throws Exception { + var noSorts = new FilterLeg(List.of()); + + assertThat(FindAndRerankSortParser.parse(FindAndRerankSortDefinition.NO_ARG_SORT)) + .as("no sort, null sort and empty sort from the deserializer") + .isEqualTo(noSorts); + assertThat(parse("{}")).as("empty object").isEqualTo(noSorts); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("singleLegCases") + void parsesSingleLegSort(String sort, FindAndRerankSort expected) throws Exception { + assertThat(parse(sort)).isEqualTo(expected); + } + + static Stream singleLegCases() { + return Stream.of( + Arguments.of( + """ + {"$vectorize": "cheese"} + """, + new VectorLeg(new VectorizeSortExpression("cheese"))), + Arguments.of( + """ + {"$vectorize": " padded text "} + """, + new VectorLeg(new VectorizeSortExpression("padded text"))), + Arguments.of( + """ + {"$vector": [1.1, 2.2, 3.3]} + """, + new VectorLeg(new VectorSortExpression(VECTOR))), + Arguments.of( + """ + {"$vector": {"$binary": "%s"}} + """ + .formatted(encodeAsMimeBase64(floatsToBytes(VECTOR))), + new VectorLeg(new VectorSortExpression(VECTOR))), + Arguments.of( + """ + {"$lexical": "cows"} + """, + new LexicalLeg(new LexicalSortExpression("cows"))), + Arguments.of( + """ + {"$lexical": " padded text "} + """, + new LexicalLeg(new LexicalSortExpression("padded text"))), + // ---- + // field sorts, in the order sent + Arguments.of( + """ + {"age": -1, "name": 1} + """, + new FilterLeg( + List.of( + new FieldSortExpression("age", false), new FieldSortExpression("name", true)))), + Arguments.of( + """ + {"name": 1, "age": -1} + """, + new FilterLeg( + List.of( + new FieldSortExpression("name", true), new FieldSortExpression("age", false)))), + Arguments.of( + """ + {"address.city": 1} + """, + new FilterLeg(List.of(new FieldSortExpression("address.city", true)))), + Arguments.of( + """ + {" ": 1} + """, + new FilterLeg(List.of(new FieldSortExpression(" ", true))))); + } + + /** A single leg sort gets the same error codes as find for the same mistakes. */ + @ParameterizedTest(name = "{0}") + @MethodSource("invalidSingleLegCases") + void rejectsSingleLegSort( + String sort, + Class exceptionClass, + ErrorCode code, + String message) { + var ex = assertThrowsExactly(exceptionClass, () -> parse(sort)); + + assertThat(ex.code).isEqualTo(code.name()); + assertThat(ex).hasMessageContaining(message); + } + + static Stream invalidSingleLegCases() { + return Stream.of( + // ---- + // $lexical, $vector or $vectorize with another field, checked in the same order as find + Arguments.of( + """ + {"$lexical": "cows", "age": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_INVALID, + "when sorting by field '$lexical' no other sort expressions allowed"), + Arguments.of( + """ + {"age": 1, "$vector": [1.1, 2.2, 3.3]} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_INVALID, + "when sorting by field '$vector' no other sort expressions allowed"), + Arguments.of( + """ + {"$vectorize": "cheese", "age": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_INVALID, + "when sorting by field '$vectorize' no other sort expressions allowed"), + Arguments.of( + """ + {"$vectorize": "cheese", "$lexical": "cows"} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_INVALID, + "when sorting by field '$lexical' no other sort expressions allowed"), + Arguments.of( + """ + {"$vectorize": "cheese", "$vector": [1.1, 2.2, 3.3]} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_INVALID, + "when sorting by field '$vector' no other sort expressions allowed"), + // ---- + // $lexical and $vectorize values: non-blank text + Arguments.of( + """ + {"$lexical": null} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path '$lexical' not valid: value must be a non-blank String, not null"), + Arguments.of( + """ + {"$lexical": " "} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "value must be a non-blank String, not \" \""), + Arguments.of( + """ + {"$lexical": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "value must be a non-blank String, not 1"), + Arguments.of( + """ + {"$vectorize": null} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path '$vectorize' not valid: value must be a non-blank String, not null"), + Arguments.of( + """ + {"$vectorize": ""} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "value must be a non-blank String, not \"\""), + Arguments.of( + """ + {"$vectorize": ["cheese"]} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "value must be a non-blank String, not [\"cheese\"]"), + // ---- + // $vector values: an array of numbers or $binary + Arguments.of( + """ + {"$vector": null} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path '$vector' not valid: value must be an Array of numbers or a $binary vector, not Null"), + Arguments.of( + """ + {"$vector": "cheese"} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "value must be an Array of numbers or a $binary vector, not String"), + Arguments.of( + """ + {"$vector": {"$date": 1}} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "only binary vector object values are supported for sorting"), + Arguments.of( + """ + {"$vector": {"$binary": "not base64!"}} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "Invalid content in EJSON $binary wrapper"), + Arguments.of( + """ + {"$vector": []} + """, + DocumentException.class, + DocumentException.Code.SHRED_BAD_VECTOR_SIZE, + "cannot be empty Array"), + Arguments.of( + """ + {"$vector": [1.1, "cheese"]} + """, + DocumentException.class, + DocumentException.Code.SHRED_BAD_VECTOR_VALUE, + "has a String value (\"cheese\")"), + // ---- + // field sort paths, every path is checked before any value, the same as find + Arguments.of( + """ + {"$fake": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_PATH_INVALID, + "Path '$fake' used in sort clause not valid: path cannot start with '$'"), + Arguments.of( + """ + {"age": 1, "$fake": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_PATH_INVALID, + "Path '$fake' used in sort clause not valid"), + Arguments.of( + """ + {"age": 2, "$fake": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_PATH_INVALID, + "Path '$fake' used in sort clause not valid"), + Arguments.of( + """ + {"": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_PATH_INVALID, + "path must be represented as a non-empty string"), + Arguments.of( + """ + {"a&b": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_PATH_INVALID, + "The ampersand character '&' at position 1 must be followed by either '&' or '.'"), + // ---- + // field sort values, only 1 and -1, the same as find + Arguments.of( + """ + {"age": 0} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "sort ordering value can only be `1` for ascending or `-1` for descending (not `0`)"), + Arguments.of( + """ + {"age": 2} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "(not `2`)"), + Arguments.of( + """ + {"age": 1, "name": -2} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path 'name' not valid"), + Arguments.of( + """ + {"age": 1.0} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "sort ordering value should be integer `1` or `-1`; or Array (vector); or String (lexical or vectorize), was: Number"), + Arguments.of( + """ + {"age": null} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "was: Null"), + Arguments.of( + """ + {"age": "asc"} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "sort ordering value can be String only for lexical or vectorize search"), + Arguments.of( + """ + {"age": [1]} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "sort ordering value can be Array only for vector search")); + } + + // =========================================================================================== + // Helpers / no more tests below + // =========================================================================================== + + /** Parses the sort the way the deserializer keeps a sort without $hybrid. */ + private static FindAndRerankSort parse(String sort) throws JsonProcessingException { + return FindAndRerankSortParser.parse( + new SingleLegDefinition(MAPPER.readTree(sort), CommandFeatures.EMPTY)); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2Test.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2Test.java new file mode 100644 index 0000000000..15c4f6c5ed --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortExpression2Test.java @@ -0,0 +1,50 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; + +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.FieldSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.LexicalSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorSortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorizeSortExpression; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** Tests for the {@link SortExpression2} records. */ +class SortExpression2Test { + + @Test + void vectorSortComparesVectorByValue() { + var vectorSort = new VectorSortExpression(new float[] {1.1f, 2.2f, 3.3f}); + var sameVector = new VectorSortExpression(new float[] {1.1f, 2.2f, 3.3f}); + var diffVector = new VectorSortExpression(new float[] {1.1f, 2.2f, 4.4f}); + + assertThat(vectorSort).as("same vector").isEqualTo(sameVector); + assertThat(vectorSort.hashCode()).as("same vector hash").isEqualTo(sameVector.hashCode()); + assertThat(vectorSort).as("different vector").isNotEqualTo(diffVector); + } + + @Test + void pathIsTheSortField() { + assertThat(new VectorizeSortExpression("cheese").path()).isEqualTo("$vectorize"); + assertThat(new VectorSortExpression(new float[] {1.1f}).path()).isEqualTo("$vector"); + assertThat(new LexicalSortExpression("cows").path()).isEqualTo("$lexical"); + assertThat(new FieldSortExpression("address.city", true).path()).isEqualTo("address.city"); + } + + /** The parser rejects blank text with an API error, so blank text here is a bug. */ + @ParameterizedTest + @ValueSource(strings = {"", " "}) + void textSortsRejectBlankText(String text) { + assertThrowsExactly(IllegalArgumentException.class, () -> new VectorizeSortExpression(text)); + assertThrowsExactly(IllegalArgumentException.class, () -> new LexicalSortExpression(text)); + } + + @Test + void fieldSortRejectsEmptyPath() { + assertThrowsExactly(IllegalArgumentException.class, () -> new FieldSortExpression("", true)); + // a blank name is a valid field name, see FieldNamingRule + assertThat(new FieldSortExpression(" ", true).path()).isEqualTo(" "); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaContractTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaContractTest.java index 99f5e33de0..7a1c7b1a2e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaContractTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaContractTest.java @@ -21,7 +21,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.JsonDefinition; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.FilterDefinition; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortDefinition; import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; import io.stargate.sgv2.jsonapi.api.model.command.impl.FindAndRerankCommand; import io.stargate.sgv2.jsonapi.exception.RequestException; @@ -323,7 +323,7 @@ private JsonNode openApi() throws Exception { @Test void rerankToolUsesSortClassDescription() throws Exception { - var description = FindAndRerankSort.class.getAnnotation(Schema.class).description(); + var description = FindAndRerankSortDefinition.class.getAnnotation(Schema.class).description(); assertFalse(description.isBlank()); var document = inputSchema("findAndRerank"); assertEquals( @@ -536,8 +536,9 @@ void rerankToolAcceptsWirePayloads(String limits) throws Exception { }) void sortSchemaAcceptsNullableAndEmptyWireForms(String json) throws Exception { var sort = objectMapper.readTree(json); - assertSchemaAccepts(schemaFor(FindAndRerankSort.class), sort); - var clause = assertDoesNotThrow(() -> objectMapper.treeToValue(sort, FindAndRerankSort.class)); + assertSchemaAccepts(schemaFor(FindAndRerankSortDefinition.class), sort); + var clause = + assertDoesNotThrow(() -> objectMapper.treeToValue(sort, FindAndRerankSortDefinition.class)); if (!sort.isNull()) { assertNotNull(clause); } @@ -611,11 +612,12 @@ void schemaRejectsInvalidWireShapes(Class clauseType, String json) throws Exc static Stream invalidWireShapes() { return Stream.of( - Arguments.of(FindAndRerankSort.class, "{\"vectorizeSort\": \"query\"}"), - Arguments.of(FindAndRerankSort.class, "{\"$hybrid\": {\"$lexical\": 1}}"), - Arguments.of(FindAndRerankSort.class, "{\"$hybrid\": {\"$vector\": \"query\"}}"), + Arguments.of(FindAndRerankSortDefinition.class, "{\"vectorizeSort\": \"query\"}"), + Arguments.of(FindAndRerankSortDefinition.class, "{\"$hybrid\": {\"$lexical\": 1}}"), + Arguments.of(FindAndRerankSortDefinition.class, "{\"$hybrid\": {\"$vector\": \"query\"}}"), Arguments.of( - FindAndRerankSort.class, "{\"$hybrid\": {\"$vector\": null, \"$vectorize\": null}}"), + FindAndRerankSortDefinition.class, + "{\"$hybrid\": {\"$vector\": null, \"$vectorize\": null}}"), Arguments.of( FindAndRerankCommand.HybridLimits.class, "{\"vectorLimit\": 50, \"lexicalLimit\": 10}"), Arguments.of(FindAndRerankCommand.HybridLimits.class, "{\"$vector\": 50}"), diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java new file mode 100644 index 0000000000..66cd00fc55 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java @@ -0,0 +1,83 @@ +package io.stargate.sgv2.jsonapi.service.resolver; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.micrometer.core.instrument.MeterRegistry; +import io.stargate.sgv2.jsonapi.TestConstants; +import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; +import io.stargate.sgv2.jsonapi.api.model.command.CommandName; +import io.stargate.sgv2.jsonapi.api.model.command.impl.FindAndRerankCommand; +import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonApiMetricsConfig; +import io.stargate.sgv2.jsonapi.config.OperationsConfig; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; +import io.stargate.sgv2.jsonapi.exception.SchemaException; +import io.stargate.sgv2.jsonapi.exception.SortException; +import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** + * Tests for {@link FindAndRerankCommandResolver}: the order of the errors. The checks themselves + * are tested in FindAndRerankSortParserTest and FindAndRerankPlannerTest. + */ +class FindAndRerankCommandResolverTest { + + /** A sort with a syntax error and no options, so the planner would reject the options too. */ + private static final String BAD_SORT_NO_OPTIONS = + """ + {"findAndRerank": {"sort": {"age": 2}}} + """; + + private final TestConstants TEST_CONSTANTS = new TestConstants(); + + private final ObjectMapper objectMapper = new ObjectMapper(); + + private final FindAndRerankCommandResolver resolver = + new FindAndRerankCommandResolver( + mock(OperationsConfig.class), + objectMapper, + mock(MeterRegistry.class), + mock(JsonApiMetricsConfig.class), + mock(FindCommandResolver.class)); + + /** The sort is parsed before the planner checks the options, such as the missing rerankOn. */ + @Test + void reportsSortSyntaxErrorBeforeOptionErrors() throws Exception { + var command = objectMapper.readValue(BAD_SORT_NO_OPTIONS, FindAndRerankCommand.class); + var commandContext = commandContext(true); + + var ex = + assertThrowsExactly( + SortException.class, () -> resolver.resolveCollectionCommand(commandContext, command)); + assertThat(ex.code).isEqualTo(SortException.Code.SORT_CLAUSE_VALUE_INVALID.name()); + } + + /** The reranking feature is checked before the sort is parsed. */ + @Test + void reportsRerankingDisabledBeforeSortSyntaxError() throws Exception { + var command = objectMapper.readValue(BAD_SORT_NO_OPTIONS, FindAndRerankCommand.class); + var commandContext = commandContext(false); + + var ex = + assertThrowsExactly( + SchemaException.class, + () -> resolver.resolveCollectionCommand(commandContext, command)); + assertThat(ex.code).isEqualTo(SchemaException.Code.RERANKING_FEATURE_NOT_ENABLED.name()); + } + + private CommandContext commandContext(boolean rerankingEnabled) { + var commandContext = + TEST_CONSTANTS.collectionContext( + CommandName.FIND_AND_RERANK, TEST_CONSTANTS.COLLECTION_SCHEMA_OBJECT); + when(commandContext.requestContext().apiFeatures()) + .thenReturn( + ApiFeatures.fromConfigAndRequest( + Map.of(ApiFeature.RERANKING, String.valueOf(rerankingEnabled)), null)); + return commandContext; + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java index 9d078e36ac..dbb3c8ef3c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java @@ -13,6 +13,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandConfig; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.CommandName; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.impl.FindAndRerankCommand; import io.stargate.sgv2.jsonapi.config.IntConfigWithBounds; import io.stargate.sgv2.jsonapi.config.OperationsConfig; @@ -84,7 +85,7 @@ void plansLegs( String vectorizeText, List legs) throws Exception { - var plan = FindAndRerankPlanner.plan(command(sort, options), commandContext(collection)); + var plan = plan(command(sort, options), commandContext(collection)); assertThat(plan.vectorizeStep() == null ? null : plan.vectorizeStep().text()) .as("%s: text to vectorize", description) @@ -319,6 +320,50 @@ static Stream sortErrorCases() { SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT)); } + /** + * Until GH #2602 and GH #2603, a sort without $hybrid parses but cannot be planned. The error + * must not tell the user they sent no sort. + */ + @ParameterizedTest(name = "{0}") + @MethodSource("singleLegSorts") + void rejectsSingleLegSort(String description, String sort) throws Exception { + var ex = + assertPlanFails( + description, + command(sort, RERANK_OPTIONS), + Collection.VECTORIZE_LEXICAL, + SortException.class, + SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT); + + assertThat(ex) + .hasMessageContaining( + "The command attempted to run findAndRerank without a `$hybrid` sort."); + } + + static Stream singleLegSorts() { + return Stream.of( + Arguments.of( + "vector leg, $vectorize", + """ + {"$vectorize": "cheese"} + """), + Arguments.of( + "vector leg, $vector", + """ + {"$vector": [0.1, 0.2, 0.3]} + """), + Arguments.of( + "lexical leg", + """ + {"$lexical": "cows"} + """), + Arguments.of( + "filter leg", + """ + {"age": -1} + """)); + } + // =========================================================================================== // Sorts the collection cannot run // =========================================================================================== @@ -449,9 +494,7 @@ void usesRerankOptionWhenCollectionHasNoReranking() throws Exception { """ .formatted(TestConstants.RERANK_MODEL); - var plan = - FindAndRerankPlanner.plan( - command(VECTOR_SORT, options), commandContext(Collection.VECTOR_NO_RERANK)); + var plan = plan(command(VECTOR_SORT, options), commandContext(Collection.VECTOR_NO_RERANK)); assertThat(plan.rerankStep().rerankServiceDef().provider()).isEqualTo("nvidia"); assertThat(plan.rerankStep().rerankServiceDef().modelName()) @@ -569,7 +612,7 @@ static Stream hybridLimitsErrorCases() { @Test void usesVectorizeTextWhenRerankOptionsAreBlank() throws Exception { var plan = - FindAndRerankPlanner.plan( + plan( command( """ {"$hybrid": {"$vectorize": "cheese"}} @@ -610,6 +653,26 @@ void reportsMissingRerankQueryBeforeHybridSortWithNoValue() throws Exception { RequestException.Code.MISSING_RERANK_QUERY_TEXT); } + /** + * A single leg sort is rejected after the options are checked, the same as a $hybrid sort is + * planned after them. This command has no rerank query, it fails for the rerank query. + */ + @Test + void reportsMissingRerankQueryBeforeSingleLegSort() throws Exception { + assertPlanFails( + "no rerank query and a single leg sort", + command( + """ + {"$lexical": "cows"} + """, + """ + {"rerankOn": "body"} + """), + Collection.VECTORIZE_LEXICAL, + RequestException.class, + RequestException.Code.MISSING_RERANK_QUERY_TEXT); + } + // =========================================================================================== // Whole plan // =========================================================================================== @@ -617,7 +680,7 @@ void reportsMissingRerankQueryBeforeHybridSortWithNoValue() throws Exception { @Test void plansHybridTextWithDefaults() throws Exception { var plan = - FindAndRerankPlanner.plan( + plan( command( """ {"$hybrid": " cheese "} @@ -655,7 +718,7 @@ void plansHybridTextWithDefaults() throws Exception { @Test void plansVectorWithOptions() throws Exception { var plan = - FindAndRerankPlanner.plan( + plan( command( """ {"$hybrid": {"$vector": [0.1, 0.2, 0.3], "$lexical": "cows"}} @@ -692,13 +755,18 @@ private APIException assertPlanFails( ErrorCode code) { var commandContext = commandContext(collection); - var ex = - assertThrowsExactly( - exceptionClass, () -> FindAndRerankPlanner.plan(command, commandContext), description); + var ex = assertThrowsExactly(exceptionClass, () -> plan(command, commandContext), description); assertThat(ex.code).as(description).isEqualTo(code.name()); return ex; } + /** Parses the sort of the command, then plans the command, the same as the command resolver. */ + private static FindAndRerankPlan plan( + FindAndRerankCommand command, CommandContext commandContext) { + return FindAndRerankPlanner.plan( + command, FindAndRerankSortParser.parse(command.sortClause()), commandContext); + } + private FindAndRerankCommand command(String sort, String options) throws Exception { return command(sort, options, null); } From b0e941c43b75d240786141c211790aaa60d27d7f Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 9 Nov 2026 14:44:23 -0800 Subject: [PATCH 2/7] remove java doc --- .../builders/CollectionSortClauseBuilder.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java index 316397352e..bab99b25e6 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortClause; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; @@ -140,16 +139,6 @@ private SortExpression buildRegularSortExpression(String path, JsonNode innerVal return SortExpression.sort(path, parseAscending(path, innerValue)); } - /** - * Validates the value of a "non-special" sort expression, which must be 1 or -1. - * - *

Public and static so the findAndRerank sort parser checks field sorts the same way, see - * {@link FindAndRerankSortParser}. - * - * @param path Path to the field to sort by, used in the error message - * @param innerValue JSON value of the sort expression - * @return true for 1 (ascending), false for -1 (descending) - */ public static boolean parseAscending(String path, JsonNode innerValue) { if (!innerValue.isInt()) { // Special checking for String and ArrayNode to give less confusing error messages @@ -197,12 +186,6 @@ private void validateSortExpressionPaths(ObjectNode sortNode) { } } - /** - * Validates the path of a "non-special" sort expression, such as address.city. - * - *

Public and static so the findAndRerank sort parser checks field sorts the same way, see - * {@link FindAndRerankSortParser}. - */ public static void validateSortExpressionPath(String path) { if (!NamingRules.FIELD.apply(path)) { // This is only called after handling "special" sort expressions so simple validation From f8868d2feaa6b6c18c97edfe085d99b2f9d18ec7 Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 9 Nov 2026 14:46:05 -0800 Subject: [PATCH 3/7] remove java doc --- .../model/command/builders/SortClauseBuilder.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java index 76d04f67a4..4645762fbb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java @@ -4,7 +4,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.EJSONWrapper; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortClause; import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.schema.SchemaObject; @@ -63,18 +62,6 @@ public SortClause build(JsonNode node) { */ protected abstract SortClause buildClauseFromDefinition(ObjectNode sortNode); - /** - * Decodes a $binary vector such as {"$binary": "P4AAAEAAAABAQAAA"}, throws {@link - * SortException.Code#SORT_CLAUSE_VALUE_INVALID} when the value is an object but not a valid - * $binary vector. - * - *

Public and static so the findAndRerank sort parser decodes a $binary vector the same way, - * see {@link FindAndRerankSortParser}. - * - * @param path The sort path the value is for, used in the error message. - * @param innerValue The value of the sort expression. - * @return The vector, or null when the value is not an object. - */ public static float[] tryDecodeBinaryVector(String path, JsonNode innerValue) { if (innerValue instanceof ObjectNode innerObject) { var ejsonWrapped = EJSONWrapper.maybeFrom(innerObject); From 100818cfb81da30e2f2d12d3eb6c696a56158396 Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 9 Nov 2026 15:06:36 -0800 Subject: [PATCH 4/7] remove java doc --- .../clause/sort/FindAndRerankSort.java | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java index 5ed4b42e74..04af068bdb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java @@ -12,7 +12,7 @@ /** * The sort of a findAndRerank command after {@link FindAndRerankSortParser} parsed it, with one - * record for each form of sort. The planner works out the legs to read with from it. + * record for each form of sort. * *

- * - *

The command stores the sort as a {@link FindAndRerankSortDefinition}. This type has no form - * for a sort that is not parsed, so code that gets a FindAndRerankSort never sees the JSON the user - * sent. - * - *

NOTE: this is a sealed interface with a record for each form (a sum type), not one record with - * a leg mode and a nullable field for each value (a tagged class). With nullable fields, forms that - * mean different things look the same: with single leg sorts added, {"$lexical": null} - * and {} would both be all nulls. A form with no field of its own does not fit, e.g. a - * field sort. And every user of the sort must know which fields go with which mode. Here each - * record has only the values of its form, and a switch over the sort must handle every form or it - * does not compile. */ public sealed interface FindAndRerankSort extends Recordable permits FindAndRerankSort.MultiLeg, FindAndRerankSort.SingleLeg { @@ -40,11 +28,6 @@ public sealed interface FindAndRerankSort extends Recordable * A sort with $hybrid. The command may read with more than one leg, it does not have to: e.g. * {"$hybrid": "cheese"} on a collection without a lexical index only reads with the * vector leg. - * - *

TODO: {@link FindAndRerankSortClauseDeserializer} still parses $hybrid, so a MultiLeg is - * also the {@link FindAndRerankSortDefinition} the command stores, and carries the metrics - * features of the sort. When {@link FindAndRerankSortParser} parses $hybrid, a MultiLeg is only a - * FindAndRerankSort. */ sealed interface MultiLeg extends FindAndRerankSort, FindAndRerankSortDefinition permits HybridText, HybridObject {} From 4865d03ba0b395d19a6b735574c9a924c2489dcd Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 9 Nov 2026 15:24:32 -0800 Subject: [PATCH 5/7] remove java doc --- .../clause/sort/FindAndRerankSort.java | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java index 04af068bdb..a30940eda1 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java @@ -51,14 +51,11 @@ public DataRecorder recordTo(DataRecorder dataRecorder) { } /** - * {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows"}}, or with $vector: a - * value for each leg. Which values must be set is checked when the command is planned. + * {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows"}}: a separate value for + * the vector leg and the lexical leg. The vector leg can use $vector instead of $vectorize. * - * @param vectorize null when $vectorize was left out, null or blank, otherwise the - * trimmed text. - * @param lexical null when $lexical was left out, null or blank, otherwise the - * trimmed text. - * @param vector null when $vector was left out or null, otherwise the vector. + *

Each value can be null. The planner checks that the sort has $vectorize or + * $vector. */ record HybridObject( @JsonIgnore String vectorize, @@ -80,7 +77,6 @@ public DataRecorder recordTo(DataRecorder dataRecorder) { .append("commandFeatures", commandFeatures); } - /** Override to do a value equality check on the vector. */ @Override public boolean equals(Object obj) { return (obj instanceof HybridObject other) @@ -90,18 +86,13 @@ public boolean equals(Object obj) { && Objects.equals(commandFeatures, other.commandFeatures); } - /** Override to do a value equality hash on the vector. */ @Override public int hashCode() { return Objects.hash(vectorize, lexical, Arrays.hashCode(vector), commandFeatures); } } - /** - * A sort without $hybrid, the command reads with one leg. It has the syntax of the find sort, see - * {@link FindAndRerankSortParser}. The planner rejects every single leg sort until GH #2602 and - * GH #2603. - */ + /** A sort without $hybrid, the command reads with one leg. */ sealed interface SingleLeg extends FindAndRerankSort permits VectorLeg, LexicalLeg, FilterLeg {} /** @@ -136,9 +127,6 @@ public DataRecorder recordTo(DataRecorder dataRecorder) { /** * {"age": -1, "name": 1}: reads the documents the filter matches with the filter * leg, sorted by the fields. - * - * @param sorts The field sorts in the order the user sent them. Empty when the command has no - * sort, a null sort or an empty sort. */ record FilterLeg(List sorts) implements SingleLeg { From c9b356d9e6ae8457ec3123b1813199beddd95f67 Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 9 Nov 2026 16:01:33 -0800 Subject: [PATCH 6/7] update java doc --- .../sort/FindAndRerankSortClauseDeserializer.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java index ce5cd05883..a81cc6f232 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java @@ -13,7 +13,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.TextNode; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.EJSONWrapper; -import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.metrics.CommandFeature; import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; import io.stargate.sgv2.jsonapi.util.JsonFieldMatcher; @@ -23,17 +22,8 @@ /** * {@link StdDeserializer} for the {@link FindAndRerankSortDefinition}. * - *

A sort with $hybrid is a multi leg sort, and its fields are read here. Any other sort is a - * single leg sort, e.g. {"$vectorize": "text"} or {"age": -1}. It is read - * by field name only and kept as the user sent it. - * - *

Note: This only checks the shape of the sort clause, the other validation is handled when - * resolving the command to an operation. - * - *

NOTE: do not check a single leg sort here, {@link FindAndRerankSortParser} checks it when the - * command is resolved. An {@link APIException} thrown here does not reach MCP users: - * quarkus-mcp-server converts the tool arguments before it calls the tool, and reports any error - * other than a Jackson mapping error as "Internal error". + *

It keeps the sort as the user sent it. It only checks that the sort is an object or null. + * There is no validation. */ public class FindAndRerankSortClauseDeserializer extends StdDeserializer { From 54fef2abde06be441192713fb840e079ddc57816 Mon Sep 17 00:00:00 2001 From: Hazel Date: Mon, 9 Nov 2026 17:50:40 -0800 Subject: [PATCH 7/7] parse $hybrid in the findAndRerank sort parser The command keeps the sort as the user sent it. FindAndRerankSortDefinition only wraps the JSON, so reading a request never fails because of the sort. The custom deserializer is removed. FindAndRerankSortParser parses the whole sort, $hybrid included. It also checks that a $hybrid sort has a value for the vector leg, which the planner did before. The metrics features come from the field names of the sort. Behaviour changes: - A $hybrid sort with a syntax error gets a SortException code, not REQUEST_STRUCTURE_MISMATCH. MCP now gets the full error too. - A sort that is not an object gets SORT_CLAUSE_INVALID. - A $hybrid sort with no value fails before the option errors. --- .../clause/sort/FindAndRerankSort.java | 56 +-- .../FindAndRerankSortClauseDeserializer.java | 258 ---------- .../sort/FindAndRerankSortDefinition.java | 108 ++-- .../clause/sort/FindAndRerankSortParser.java | 187 +++++-- .../resolver/FindAndRerankPlanner.java | 29 +- ...ndAndRerankSortClauseDeserializerTest.java | 460 ------------------ .../sort/FindAndRerankSortDefinitionTest.java | 136 ++++++ .../sort/FindAndRerankSortParserTest.java | 294 +++++++++-- .../FindAndRerankCommandResolverTest.java | 34 ++ .../resolver/FindAndRerankPlannerTest.java | 87 ---- 10 files changed, 664 insertions(+), 985 deletions(-) delete mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java delete mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinitionTest.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java index a30940eda1..dbeba4df4f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSort.java @@ -1,52 +1,44 @@ package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; -import com.fasterxml.jackson.annotation.JsonIgnore; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.FieldSortExpression; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.LexicalSortExpression; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorLegSortExpression; -import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; import io.stargate.sgv2.jsonapi.util.recordable.Recordable; import java.util.Arrays; import java.util.List; import java.util.Objects; /** - * The sort of a findAndRerank command after {@link FindAndRerankSortParser} parsed it, with one - * record for each form of sort. + * The parsed sort of a findAndRerank command, with one record for each form of sort. * *

*/ public sealed interface FindAndRerankSort extends Recordable permits FindAndRerankSort.MultiLeg, FindAndRerankSort.SingleLeg { - /** - * A sort with $hybrid. The command may read with more than one leg, it does not have to: e.g. - * {"$hybrid": "cheese"} on a collection without a lexical index only reads with the - * vector leg. - */ - sealed interface MultiLeg extends FindAndRerankSort, FindAndRerankSortDefinition - permits HybridText, HybridObject {} + /** A sort with $hybrid. It always has a value for the vector leg. */ + sealed interface MultiLeg extends FindAndRerankSort permits HybridText, HybridObject {} /** * {"$hybrid": "cheese"}: the same text for the vector leg and the lexical leg. * - * @param text The trimmed text, null when the user sent blank text. + * @param text The trimmed text. It is never blank. */ - record HybridText(@JsonIgnore String text, @JsonIgnore CommandFeatures commandFeatures) - implements MultiLeg { + record HybridText(String text) implements MultiLeg { public HybridText { - Objects.requireNonNull(commandFeatures, "commandFeatures must not be null"); + Objects.requireNonNull(text, "text must not be null"); + if (text.isBlank()) { + throw new IllegalArgumentException("text must not be blank"); + } } @Override public DataRecorder recordTo(DataRecorder dataRecorder) { - return dataRecorder.append("text", text).append("commandFeatures", commandFeatures); + return dataRecorder.append("text", text); } } @@ -54,18 +46,14 @@ public DataRecorder recordTo(DataRecorder dataRecorder) { * {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows"}}: a separate value for * the vector leg and the lexical leg. The vector leg can use $vector instead of $vectorize. * - *

Each value can be null. The planner checks that the sort has $vectorize or - * $vector. + *

At least one of $vectorize and $vector is set. $lexical can be null. */ - record HybridObject( - @JsonIgnore String vectorize, - @JsonIgnore String lexical, - @JsonIgnore float[] vector, - @JsonIgnore CommandFeatures commandFeatures) - implements MultiLeg { + record HybridObject(String vectorize, String lexical, float[] vector) implements MultiLeg { public HybridObject { - Objects.requireNonNull(commandFeatures, "commandFeatures must not be null"); + if (vectorize == null && vector == null) { + throw new IllegalArgumentException("vectorize or vector must not be null"); + } } @Override @@ -73,22 +61,22 @@ public DataRecorder recordTo(DataRecorder dataRecorder) { return dataRecorder .append("vectorize", vectorize) .append("lexical", lexical) - .append("vector", Arrays.toString(vector)) - .append("commandFeatures", commandFeatures); + .append("vector", Arrays.toString(vector)); } + /** Override to do a value equality check on the vector. */ @Override public boolean equals(Object obj) { return (obj instanceof HybridObject other) && Objects.equals(vectorize, other.vectorize) && Objects.equals(lexical, other.lexical) - && Arrays.equals(vector, other.vector) - && Objects.equals(commandFeatures, other.commandFeatures); + && Arrays.equals(vector, other.vector); } + /** Override to do a value equality hash on the vector. */ @Override public int hashCode() { - return Objects.hash(vectorize, lexical, Arrays.hashCode(vector), commandFeatures); + return Objects.hash(vectorize, lexical, Arrays.hashCode(vector)); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java deleted file mode 100644 index a81cc6f232..0000000000 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java +++ /dev/null @@ -1,258 +0,0 @@ -package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; - -import static io.stargate.sgv2.jsonapi.config.constants.DocumentConstants.Fields.*; -import static io.stargate.sgv2.jsonapi.util.JsonUtil.arrayNodeToVector; - -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JsonMappingException; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.deser.std.StdDeserializer; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.NullNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.fasterxml.jackson.databind.node.TextNode; -import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.EJSONWrapper; -import io.stargate.sgv2.jsonapi.metrics.CommandFeature; -import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; -import io.stargate.sgv2.jsonapi.util.JsonFieldMatcher; -import java.io.IOException; -import java.util.*; - -/** - * {@link StdDeserializer} for the {@link FindAndRerankSortDefinition}. - * - *

It keeps the sort as the user sent it. It only checks that the sort is an object or null. - * There is no validation. - */ -public class FindAndRerankSortClauseDeserializer - extends StdDeserializer { - - private static final String ERROR_CONTEXT = "`sort` clause"; - - // when the sort has $hybrid it must be the only field at top level - // can be string or object - // { "sort" : { "$hybrid" : "i like cheese" } } - // { "sort" : { "$hybrid" : { "$vectorize" : "i like cheese", "$lexical" : "cows"} } } - // { "sort" : { "$hybrid" : { "$vector" : [1, 2, 3], "$lexical" : "cows"} } } - private static final JsonFieldMatcher MATCH_HYBRID_FIELD = - new JsonFieldMatcher<>(JsonNode.class, List.of(HYBRID_FIELD), List.of()); - - // user is specifying the sorts individually - // can be - // { "sort" : { "$hybrid" : { "$vectorize" : "i like cheese", "$lexical" : "cows"} } } - // { "sort" : { "$hybrid" : { "$vector" : [1, 2, 3], "$lexical" : "cows"} } } - // they could also set something to null to skip that read, that is handled by the resolver - // later. - private static final JsonFieldMatcher MATCH_SORT_FIELDS = - new JsonFieldMatcher<>( - JsonNode.class, - List.of(), - List.of(LEXICAL_CONTENT_FIELD, VECTOR_EMBEDDING_TEXT_FIELD, VECTOR_EMBEDDING_FIELD)); - - // the metrics feature for the fields of a single leg sort that have one, the same features the - // $hybrid object records for these fields - private static final Map SINGLE_LEG_FEATURES = - Map.of( - VECTOR_EMBEDDING_TEXT_FIELD, CommandFeature.VECTORIZE, - VECTOR_EMBEDDING_FIELD, CommandFeature.VECTOR, - LEXICAL_CONTENT_FIELD, CommandFeature.LEXICAL); - - public FindAndRerankSortClauseDeserializer() { - super(FindAndRerankSortDefinition.class); - } - - /** {@inheritDoc} */ - @Override - public FindAndRerankSortDefinition deserialize( - JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { - - return switch (deserializationContext.readTree(jsonParser)) { - case NullNode ignored -> // this is {"sort" : null} - FindAndRerankSortDefinition.NO_ARG_SORT; - case ObjectNode objectNode -> deserialize(jsonParser, objectNode); - default -> - throw new JsonMappingException( - jsonParser, "sort clause must be an object or null", jsonParser.currentLocation()); - }; - } - - /** - * Deserialize the sort clause from an object node. - * - * @param sort The sort clause as an object node, no check for nulls. - * @return The {@link FindAndRerankSortDefinition} that reflects the request without validation, - * e.g. checking that a vectorize or vector sort is provided but not both. - */ - private static FindAndRerankSortDefinition deserialize(JsonParser jsonParser, ObjectNode sort) - throws JsonMappingException { - - // { "sort" : { } } - if (sort.isEmpty()) { - return FindAndRerankSortDefinition.NO_ARG_SORT; - } - - // no $hybrid, so a single leg sort that is kept as sent, e.g. - // { "sort" : { "$lexical" : "cows" } } or { "sort" : { "age" : -1 } } - if (!sort.has(HYBRID_FIELD)) { - return new FindAndRerankSortDefinition.SingleLegDefinition( - sort, singleLegCommandFeatures(sort)); - } - - // TODO: $hybrid is parsed here, so MCP users get its errors as "unable to convert value" or - // "Internal error", without the error code. Parse it in FindAndRerankSortParser and keep only - // the sort as sent here, see FindAndRerankSortDefinition - var hybridMatch = MATCH_HYBRID_FIELD.matchAndThrow(sort, jsonParser, ERROR_CONTEXT); - - return switch (hybridMatch.matched().get(HYBRID_FIELD)) { - case TextNode textNode -> - new FindAndRerankSort.HybridText( - normalizedText(textNode.asText()), CommandFeatures.of(CommandFeature.HYBRID)); - case ObjectNode objectNode -> deserializeHybridObject(jsonParser, objectNode); - case JsonNode node -> - throw JsonFieldMatcher.errorForWrongType( - jsonParser, ERROR_CONTEXT, HYBRID_FIELD, node, TextNode.class, ObjectNode.class); - }; - } - - private static FindAndRerankSort.HybridObject deserializeHybridObject( - JsonParser jsonParser, ObjectNode hybridObject) throws JsonMappingException { - - var sortMatch = MATCH_SORT_FIELDS.matchAndThrow(hybridObject, jsonParser, ERROR_CONTEXT); - CommandFeatures commandFeatures = CommandFeatures.of(CommandFeature.HYBRID); - - var vectorizeText = - switch (sortMatch.matched().get(VECTOR_EMBEDDING_TEXT_FIELD)) { - case null -> { - // undefined in the object, up to the resolver to check if both vectorize and vector are - // not set - // { "sort" : { "$hybrid" : { }} - yield null; - } - case NullNode ignored -> { - // explict setting to null is allowed - // { "sort" : { "$hybrid" : { "$vectorize" : null, - commandFeatures.addFeature(CommandFeature.VECTORIZE); - yield null; - } - case TextNode textNode -> { - // { "sort" : { "$hybrid" : { "$vectorize" : "I like cheese", - commandFeatures.addFeature(CommandFeature.VECTORIZE); - yield normalizedText(textNode.asText()); - } - case JsonNode node -> - throw JsonFieldMatcher.errorForWrongType( - jsonParser, - ERROR_CONTEXT, - VECTOR_EMBEDDING_TEXT_FIELD, - node, - NullNode.class, - TextNode.class); - }; - - var lexicalText = - switch (sortMatch.matched().get(LEXICAL_CONTENT_FIELD)) { - case null -> { - // undefined in the object, up to the resolver to check if valid - // { "sort" : { "$hybrid" : { }} - yield null; - } - case NullNode ignored -> { - // explict setting to null is allowed - // { "sort" : { "$hybrid" : { "$lexical" : null, - commandFeatures.addFeature(CommandFeature.LEXICAL); - yield null; - } - case TextNode textNode -> { - // { "sort" : { "$hybrid" : { "$lexical" : "cheese", - commandFeatures.addFeature(CommandFeature.LEXICAL); - yield normalizedText(textNode.asText()); - } - case JsonNode node -> - throw JsonFieldMatcher.errorForWrongType( - jsonParser, - ERROR_CONTEXT, - LEXICAL_CONTENT_FIELD, - node, - NullNode.class, - TextNode.class); - }; - - var vector = - switch (sortMatch.matched().get(VECTOR_EMBEDDING_FIELD)) { - case null -> { - // undefined in the object, up to the resolver to check if valid - // { "sort" : { "$hybrid" : { }} - yield null; - } - case NullNode ignored -> { - // explict setting to null is allowed - // { "sort" : { "$hybrid" : { "$vector" : null, - commandFeatures.addFeature(CommandFeature.VECTOR); - yield null; - } - case ArrayNode arrayNode -> { - // { "sort" : { "$hybrid" : { "vector" : [1,2,3], - commandFeatures.addFeature(CommandFeature.VECTOR); - yield arrayNodeToVector(arrayNode); - } - case ObjectNode objectNode -> { - // binary vector - // { "sort" : { "$hybrid" : { "$vector" : {"$binary": "c3VyZS4="}, - var ejson = EJSONWrapper.maybeFrom(objectNode); - if (ejson == null || ejson.type() != EJSONWrapper.EJSONType.BINARY) { - // not the best error, wont tell people how to do a binary vector - // and it will say it got an object node and that is not supported - // TODO: better error message - throw JsonFieldMatcher.errorForWrongType( - jsonParser, - ERROR_CONTEXT + " (binary vector)", - VECTOR_EMBEDDING_FIELD, - objectNode, - NullNode.class, - ArrayNode.class, - ObjectNode.class); - } - commandFeatures.addFeature(CommandFeature.VECTOR); - yield ejson.getVectorValueForBinary(); - } - case JsonNode node -> - throw JsonFieldMatcher.errorForWrongType( - jsonParser, - ERROR_CONTEXT, - VECTOR_EMBEDDING_FIELD, - node, - NullNode.class, - ArrayNode.class, - ObjectNode.class); - }; - - return new FindAndRerankSort.HybridObject(vectorizeText, lexicalText, vector, commandFeatures); - } - - /** The metrics features of a single leg sort, from its field names only. */ - private static CommandFeatures singleLegCommandFeatures(ObjectNode sort) { - var commandFeatures = CommandFeatures.create(); - SINGLE_LEG_FEATURES.forEach( - (field, feature) -> { - if (sort.has(field)) { - commandFeatures.addFeature(feature); - } - }); - return commandFeatures; - } - - /** - * Normalise the sort string from the user. - * - * @param value Nullable string value from the user. - * @return null, if the user provided null, or a blank string, otherwise the trimmed value. - */ - private static String normalizedText(String value) { - return switch (value) { - case null -> null; - case String s when s.isBlank() -> null; - default -> value.trim(); - }; - } -} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java index e4ac962122..f22a54a4ed 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java @@ -2,11 +2,11 @@ import static io.stargate.sgv2.jsonapi.config.constants.DocumentConstants.Fields.*; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.node.MissingNode; -import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; +import io.stargate.sgv2.jsonapi.metrics.CommandFeature; import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; import io.stargate.sgv2.jsonapi.util.recordable.Recordable; import java.util.Map; @@ -16,25 +16,14 @@ import org.eclipse.microprofile.openapi.annotations.media.SchemaProperty; /** - * The sort clause of a findAndRerank command as the command stores it, read by {@link - * FindAndRerankSortClauseDeserializer}. {@link FindAndRerankSortParser} parses it into a {@link - * FindAndRerankSort} when the command is resolved. - * - *

- * - *

TODO: parse $hybrid in {@link FindAndRerankSortParser} as well, then this type only holds the - * sort as the user sent it, like {@link SortDefinition} does for find. Not done yet because moving - * $hybrid changes its error codes and the order of its errors, so it needs its own change. + * The sort of a findAndRerank command, as the user sent it. It can be any JSON value. This class + * does not check it. * *

NOTE: the {@link Schema} is named FindAndRerankSort, not after this type, so the - * OpenAPI and MCP schemas keep the name they had before the parsed sort got its own type. It only - * describes $hybrid. Single leg sorts are added to it when they are supported, see GH #2604. + * OpenAPI and MCP schemas keep their name. + * + * @param json The sort as the user sent it. {@link MissingNode} when the command has no sort. */ -@JsonDeserialize(using = FindAndRerankSortClauseDeserializer.class) @Schema( name = "FindAndRerankSort", type = SchemaType.OBJECT, @@ -62,49 +51,54 @@ {"$hybrid": {"$vector": {"$binary": "P4AAAEAAAABAQAAA"}, "$lexical": "lexical query"}} """ }) -public sealed interface FindAndRerankSortDefinition extends Recordable - permits FindAndRerankSort.MultiLeg, FindAndRerankSortDefinition.SingleLegDefinition { +public record FindAndRerankSortDefinition(@JsonIgnore JsonNode json) implements Recordable { - /** - * A command with no sort, a null sort or an empty sort. - * - *

NOTE: uses {@link MissingNode} because it cannot be changed, and this instance is shared. - */ - SingleLegDefinition NO_ARG_SORT = - new SingleLegDefinition(MissingNode.getInstance(), CommandFeatures.EMPTY); + /** A command with no sort. */ + public static final FindAndRerankSortDefinition NO_ARG_SORT = + new FindAndRerankSortDefinition(MissingNode.getInstance()); - /** - * The metrics features of the sort, recorded when the sort is read. The command adds them to its - * features before it is resolved, so before the sort is parsed. - */ - CommandFeatures commandFeatures(); + /** The metrics feature of each sort field that has one. */ + private static final Map FIELD_FEATURES = + Map.of( + VECTOR_EMBEDDING_TEXT_FIELD, CommandFeature.VECTORIZE, + VECTOR_EMBEDDING_FIELD, CommandFeature.VECTOR, + LEXICAL_CONTENT_FIELD, CommandFeature.LEXICAL); + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public FindAndRerankSortDefinition { + Objects.requireNonNull(json, "json must not be null"); + } /** - * Any sort without $hybrid, e.g. {"$vectorize": "cheese"}, {"$vector": [0.1, - * 0.2]}, {"$lexical": "cows"} or {"age": -1}. It is kept as the - * user sent it, {@link FindAndRerankSortParser} checks and parses it. - * - * @param sort The sort as the user sent it: an object, or {@link MissingNode} when the command - * has no sort, a null sort or an empty sort. - * @param commandFeatures The metrics features, from the field names of the sort only. + * The metrics features of the sort, from its field names only. A field set to null + * still counts. */ - record SingleLegDefinition(@JsonIgnore JsonNode sort, @JsonIgnore CommandFeatures commandFeatures) - implements FindAndRerankSortDefinition { - - public SingleLegDefinition { - Objects.requireNonNull(sort, "sort must not be null"); - Objects.requireNonNull(commandFeatures, "commandFeatures must not be null"); - // the parser reads the sort as an object, see FindAndRerankSortParser - if (!sort.isObject() && !sort.isMissingNode()) { - throw new IllegalArgumentException( - "sort must be an object or MissingNode, sort.getNodeType(): " + sort.getNodeType()); - } + public CommandFeatures commandFeatures() { + var commandFeatures = CommandFeatures.create(); + var hybrid = json.get(HYBRID_FIELD); + if (hybrid == null) { + // a single leg sort, e.g. {"$vectorize": "cheese"} + addFieldFeatures(json, commandFeatures); + } else { + // e.g. {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows"}} + commandFeatures.addFeature(CommandFeature.HYBRID); + addFieldFeatures(hybrid, commandFeatures); } + return commandFeatures; + } - @Override - public DataRecorder recordTo(DataRecorder dataRecorder) { - return dataRecorder.append("sort", sort).append("commandFeatures", commandFeatures); - } + private static void addFieldFeatures(JsonNode node, CommandFeatures commandFeatures) { + FIELD_FEATURES.forEach( + (field, feature) -> { + if (node.has(field)) { + commandFeatures.addFeature(feature); + } + }); + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder.append("json", json); } // Schema-only classes. @SchemaProperty can point at a nested schema only through a class, so @@ -128,7 +122,7 @@ public DataRecorder recordTo(DataRecorder dataRecorder) { anyOf = {float[].class, BinaryVectorSchema.class}) }, not = ConflictingVectorSortSchema.class) - class HybridSortSchema {} + public static class HybridSortSchema {} /** A binary encoded vector such as {"$binary": "P4AAAEAAAABAQAAA"}. */ @Schema( @@ -139,7 +133,7 @@ class HybridSortSchema {} additionalProperties = Schema.False.class, requiredProperties = BINARY_VECTOR_TEXT_FIELD, properties = @SchemaProperty(name = BINARY_VECTOR_TEXT_FIELD, type = SchemaType.STRING)) - class BinaryVectorSchema {} + public static class BinaryVectorSchema {} /** * Matches an object that has both $vector and $vectorize. HybridSort uses it as its {@code not} @@ -150,5 +144,5 @@ class BinaryVectorSchema {} type = SchemaType.OBJECT, implementation = Object.class, requiredProperties = {VECTOR_EMBEDDING_FIELD, VECTOR_EMBEDDING_TEXT_FIELD}) - class ConflictingVectorSortSchema {} + public static class ConflictingVectorSortSchema {} } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java index cd39b86ab2..ace1805c0a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.node.TextNode; import io.stargate.sgv2.jsonapi.api.model.command.builders.CollectionSortClauseBuilder; import io.stargate.sgv2.jsonapi.api.model.command.builders.SortClauseBuilder; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.FieldSortExpression; @@ -18,32 +19,30 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; /** - * Parses the sort of a findAndRerank command, as the command stores it, into a {@link - * FindAndRerankSort}. + * Parses the sort of a findAndRerank command into a {@link FindAndRerankSort}. * - *

The parser checks only the syntax of the sort, it does not use the collection or the config. - * For example it checks that $lexical has text, not that the collection has a lexical index: the - * checks that need the collection are made when the command is planned. The command resolver calls - * the parser before the planner, so a syntax error in the sort is reported before the errors the - * planner finds, such as a missing rerankOn. + *

It checks only the syntax of the sort, so it does not need the collection or the config. * - *

A single leg sort, one without $hybrid, has the syntax of the find sort: $lexical, $vector or - * $vectorize must be the only field in the sort, any other field is a field sort with the value - * 1 or -1. The errors have the same {@link SortException} codes as find. - * The checks for field sorts and for $binary vectors are the ones find uses, see {@link - * CollectionSortClauseBuilder} and {@link SortClauseBuilder}. + *

A sort with $hybrid must have a value for the vector leg: the text, $vectorize or $vector. * - *

NOTE: find checks $lexical, $vector and $vectorize in {@link CollectionSortClauseBuilder}, in - * the same method as its lexical index check, which needs the collection, so they are checked again - * here. Two differences from find are on purpose: $lexical must not be blank, and a $lexical that - * is not text gets SORT_CLAUSE_VALUE_INVALID, the same as $vectorize, not SORT_CLAUSE_INVALID. - * TODO: when find moves to {@link SortExpression2}, find should parse its sort here and keep only - * its collection checks, then the copy of these checks in {@link CollectionSortClauseBuilder} goes. + *

A sort without $hybrid uses the find sort syntax. For field sorts and $binary vectors it calls + * the checks find uses, in {@link CollectionSortClauseBuilder} and {@link SortClauseBuilder}. + * + *

NOTE: find checks $lexical, $vector and $vectorize in the same method as its lexical index + * check, so this class has its own copy of those checks. The copy differs from find in three ways: + * $lexical must not be blank, a $lexical that is not text gets SORT_CLAUSE_VALUE_INVALID, and the + * text of $lexical and $vectorize is trimmed. TODO: when find moves to {@link SortExpression2}, + * find should parse its sort here and keep only its collection checks. Then the copy is not needed. */ public final class FindAndRerankSortParser { + /** The fields a $hybrid object can have. */ + private static final Set HYBRID_OBJECT_FIELDS = + Set.of(VECTOR_EMBEDDING_TEXT_FIELD, VECTOR_EMBEDDING_FIELD, LEXICAL_CONTENT_FIELD); + private FindAndRerankSortParser() { // static methods only } @@ -51,24 +50,137 @@ private FindAndRerankSortParser() { /** * Parses the sort, throws a {@link SortException} when its syntax is not valid. * - * @param definition The sort as the command stores it. + * @param definition The sort as the user sent it. * @return The parsed sort. */ public static FindAndRerankSort parse(FindAndRerankSortDefinition definition) { Objects.requireNonNull(definition, "definition must not be null"); + var sort = definition.json(); + + // no sort or a null sort: the filter leg, without sorting + if (sort.isMissingNode() || sort.isNull()) { + return new FindAndRerankSort.FilterLeg(List.of()); + } + if (!(sort instanceof ObjectNode sortObject)) { + throw SortException.Code.SORT_CLAUSE_INVALID.get( + Map.of( + "problem", + "sort clause must be submitted as JSON Object, not %s" + .formatted(JsonUtil.nodeTypeAsString(sort)))); + } + return sortObject.has(HYBRID_FIELD) ? parseHybrid(sortObject) : parseSingleLeg(sortObject); + } + + // =========================================================================================== + // $hybrid + // =========================================================================================== - return switch (definition) { - // TODO: FindAndRerankSortClauseDeserializer still parses $hybrid, parse it here instead, - // see FindAndRerankSortDefinition - case FindAndRerankSort.MultiLeg multiLeg -> multiLeg; - case FindAndRerankSortDefinition.SingleLegDefinition singleLeg -> - parseSingleLeg(singleLeg.sort()); + /** {"$hybrid": "cheese"}, or a $hybrid object with a value for each leg. */ + private static FindAndRerankSort.MultiLeg parseHybrid(ObjectNode sort) { + return switch (onlyField(sort, HYBRID_FIELD)) { + case TextNode textNode -> hybridText(textNode.textValue()); + case ObjectNode objectNode -> hybridObject(objectNode); + case JsonNode node -> + throw SortException.Code.SORT_CLAUSE_VALUE_INVALID.get( + Map.of( + "path", + HYBRID_FIELD, + "problem", + "value must be a String or an Object, not %s" + .formatted(JsonUtil.nodeTypeAsString(node)))); }; } - private static FindAndRerankSort.SingleLeg parseSingleLeg(JsonNode sort) { + /** The same text for both legs. It must not be blank. */ + private static FindAndRerankSort.HybridText hybridText(String text) { + if (text.isBlank()) { + throw SortException.Code.MISSING_HYBRID_SORT_VALUE.get(); + } + return new FindAndRerankSort.HybridText(text.trim()); + } + + /** + * {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows"}}, or with $vector. It + * must have $vectorize or $vector. + */ + private static FindAndRerankSort.HybridObject hybridObject(ObjectNode hybrid) { + hybrid + .fieldNames() + .forEachRemaining( + field -> { + if (!HYBRID_OBJECT_FIELDS.contains(field)) { + throw SortException.Code.SORT_CLAUSE_INVALID.get( + Map.of( + "problem", + "a %s object can only have %s, %s and %s, not '%s'" + .formatted( + HYBRID_FIELD, + VECTOR_EMBEDDING_TEXT_FIELD, + VECTOR_EMBEDDING_FIELD, + LEXICAL_CONTENT_FIELD, + field))); + } + }); + + var vectorize = optionalText(hybrid, VECTOR_EMBEDDING_TEXT_FIELD); + var lexical = optionalText(hybrid, LEXICAL_CONTENT_FIELD); + var vector = optionalVector(hybrid); - // no sort, a null sort or an empty sort: the filter leg, without sorting + // without $vectorize or $vector there is nothing to sort the vector leg by, see GH #2577 + if (vectorize == null && vector == null) { + throw lexical == null + ? SortException.Code.MISSING_HYBRID_SORT_VALUE.get() + : SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT.get(); + } + return new FindAndRerankSort.HybridObject(vectorize, lexical, vector); + } + + /** + * The trimmed text of $vectorize or $lexical in a $hybrid object. It is null when + * the field is missing, null or blank. + */ + private static String optionalText(ObjectNode hybrid, String field) { + var value = hybrid.get(field); + if (value == null || value.isNull()) { + return null; + } + if (!value.isTextual()) { + throw SortException.Code.SORT_CLAUSE_VALUE_INVALID.get( + Map.of( + "path", + hybridPath(field), + "problem", + "value must be a String or null, not %s" + .formatted(JsonUtil.nodeTypeAsString(value)))); + } + var text = value.textValue(); + return text.isBlank() ? null : text.trim(); + } + + /** + * The vector of $vector in a $hybrid object. It is null when the field is missing or + * null. + */ + private static float[] optionalVector(ObjectNode hybrid) { + var value = hybrid.get(VECTOR_EMBEDDING_FIELD); + if (value == null || value.isNull()) { + return null; + } + return vector(hybridPath(VECTOR_EMBEDDING_FIELD), value); + } + + /** The path of a field in a $hybrid object, for error messages, e.g. $hybrid.$vectorize. */ + private static String hybridPath(String field) { + return HYBRID_FIELD + "." + field; + } + + // =========================================================================================== + // Single leg sorts + // =========================================================================================== + + private static FindAndRerankSort.SingleLeg parseSingleLeg(ObjectNode sort) { + + // an empty sort: the filter leg, without sorting if (sort.isEmpty()) { return new FindAndRerankSort.FilterLeg(List.of()); } @@ -84,7 +196,9 @@ private static FindAndRerankSort.SingleLeg parseSingleLeg(JsonNode sort) { // { "sort" : { "$vector" : [0.1, 0.2, 0.3] } } if (sort.has(VECTOR_EMBEDDING_FIELD)) { - return new FindAndRerankSort.VectorLeg(new VectorSortExpression(vector(sort))); + return new FindAndRerankSort.VectorLeg( + new VectorSortExpression( + vector(VECTOR_EMBEDDING_FIELD, onlyField(sort, VECTOR_EMBEDDING_FIELD)))); } // { "sort" : { "$vectorize" : "cheese" } } @@ -106,8 +220,8 @@ private static FindAndRerankSort.SingleLeg parseSingleLeg(JsonNode sort) { return new FindAndRerankSort.FilterLeg(sorts); } - /** The value of $lexical, $vector or $vectorize, which must be the only field in the sort. */ - private static JsonNode onlyField(JsonNode sort, String field) { + /** The value of the field, which must be the only field in the sort. */ + private static JsonNode onlyField(ObjectNode sort, String field) { if (sort.size() > 1) { throw SortException.Code.SORT_CLAUSE_INVALID.get( Map.of( @@ -118,7 +232,7 @@ private static JsonNode onlyField(JsonNode sort, String field) { } /** The trimmed text of $lexical or $vectorize, which must not be blank. */ - private static String text(JsonNode sort, String field) { + private static String text(ObjectNode sort, String field) { var value = onlyField(sort, field); // textValue() is null when the value is not text var text = value.textValue(); @@ -133,17 +247,16 @@ private static String text(JsonNode sort, String field) { return text.trim(); } - /** The vector of $vector: an array of numbers, or a $binary vector. */ - private static float[] vector(JsonNode sort) { - return switch (onlyField(sort, VECTOR_EMBEDDING_FIELD)) { + /** A vector: an array of numbers, or a $binary vector. */ + private static float[] vector(String path, JsonNode value) { + return switch (value) { case ArrayNode arrayNode -> JsonUtil.arrayNodeToVector(arrayNode); - case ObjectNode objectNode -> - SortClauseBuilder.tryDecodeBinaryVector(VECTOR_EMBEDDING_FIELD, objectNode); + case ObjectNode objectNode -> SortClauseBuilder.tryDecodeBinaryVector(path, objectNode); case JsonNode node -> throw SortException.Code.SORT_CLAUSE_VALUE_INVALID.get( Map.of( "path", - VECTOR_EMBEDDING_FIELD, + path, "problem", "value must be an Array of numbers or a $binary vector, not %s" .formatted(JsonUtil.nodeTypeAsString(node)))); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java index 21a0a324cb..73c8c2dc5c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlanner.java @@ -6,7 +6,6 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortParser; import io.stargate.sgv2.jsonapi.api.model.command.impl.FindAndRerankCommand; import io.stargate.sgv2.jsonapi.config.IntConfigWithBounds; import io.stargate.sgv2.jsonapi.config.OperationsConfig; @@ -26,13 +25,12 @@ /** * Validates a {@link FindAndRerankCommand} and translates the intention of the user, expressed in * the command and its sort, into a {@link FindAndRerankPlan} that is used to build the operation - * that runs it. The sort comes to the planner already parsed by {@link FindAndRerankSortParser}. + * that runs it. * - *

TODO: move the checks that the command can run on the collection and with the config (the sort - * is supported, hybridLimits, the rerank service, rerankQuery, rerankOn, the projection, the - * $hybrid values) into a binder that runs between the parser and the planner, so the planner only - * works out how to run a command that can run. Not done in the change that added the parser, so the - * move is a change of its own that does not change behaviour. + *

TODO: move the checks that the command can run on the collection and with the config into a + * binder: the sort is supported, hybridLimits, the rerank service, rerankQuery, rerankOn and the + * projection. Then {@link #plan} takes only the bound command, not the command and the sort. The + * move is its own change and does not change behaviour. */ class FindAndRerankPlanner { @@ -44,7 +42,7 @@ private FindAndRerankPlanner() { * Validates the command and makes its plan. * * @param command The command to plan. - * @param sort The sort of the command, parsed by {@link FindAndRerankSortParser}. + * @param sort The parsed sort of the command. * @param commandContext Context for the command, it has the collection and the config. * @return The plan, it always has a VECTOR_LEG. */ @@ -70,12 +68,6 @@ static FindAndRerankPlan plan( var passageLocator = passageLocator(command, sort); var projector = command.buildProjector(); - // NOTE: the legs are planned after the options and the projection are checked, to keep the - // error order of the command: e.g. {"$hybrid": ""} with no rerankQuery fails with - // MISSING_RERANK_QUERY_TEXT, not MISSING_HYBRID_SORT_VALUE - - // NOTE: a sort with no $vector or $vectorize fails here with a SORT error, not later with an - // UNEXPECTED_SERVER_ERROR, see GH #2577 var legs = switch (sort) { case FindAndRerankSort.MultiLeg multiLeg -> multiLegs(command, multiLeg, schemaObject); @@ -174,20 +166,13 @@ private static void checkSortSupported( /** * The legs of a $hybrid sort: always a vector leg, and a lexical leg when the sort has lexical * text and the collection has a lexical index. Without a lexical index the $hybrid text skips the - * lexical leg, and an explicit $lexical throws. Throws when the sort has no vectorize text and no - * vector for the vector leg. + * lexical leg, and an explicit $lexical throws. */ private static List multiLegs( FindAndRerankCommand command, FindAndRerankSort.MultiLeg multiLeg, CollectionSchemaObject schemaObject) { - if (!isVectorizeSort(multiLeg) && !isVectorSort(multiLeg)) { - throw isLexicalSort(multiLeg) - ? SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT.get() - : SortException.Code.MISSING_HYBRID_SORT_VALUE.get(); - } - var hybridLimits = getOrDefault( command.options(), diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java deleted file mode 100644 index a0ba56f5d8..0000000000 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java +++ /dev/null @@ -1,460 +0,0 @@ -package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; - -import static io.stargate.sgv2.jsonapi.metrics.CommandFeature.*; -import static io.stargate.sgv2.jsonapi.util.Base64Util.encodeAsMimeBase64; -import static io.stargate.sgv2.jsonapi.util.CqlVectorUtil.floatsToBytes; -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertThrowsExactly; -import static org.mockito.Mockito.when; - -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JsonMappingException; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.TextNode; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridObject; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridText; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortDefinition.SingleLegDefinition; -import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; -import io.stargate.sgv2.jsonapi.util.recordable.PrettyPrintable; -import java.util.stream.Stream; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** Tests for the {@link FindAndRerankSortClauseDeserializer}. */ -public class FindAndRerankSortClauseDeserializerTest { - - private static final Logger LOGGER = - LoggerFactory.getLogger(FindAndRerankSortClauseDeserializerTest.class); - - private static final ObjectMapper MAPPER = new ObjectMapper(); - - private FindAndRerankSortClauseDeserializer deserializer; - - @Mock private JsonParser jsonParser; - - @Mock private DeserializationContext deserializationContext; - - @BeforeEach - void setUp() { - MockitoAnnotations.openMocks(this); - deserializer = new FindAndRerankSortClauseDeserializer(); - } - - @Test - public void testEqualsAndHash() { - var value1 = - new HybridObject( - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - var sameValue = - new HybridObject( - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - - var diffVectorize = - new HybridObject( - "vectorize sort 2", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - var diffLexical = - new HybridObject( - "vectorize sort", - "lexical sort 2", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - var diffVector = - new HybridObject( - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f, 4.4f}, - CommandFeatures.EMPTY); - var diffForm = new HybridText("vectorize sort", CommandFeatures.EMPTY); - - var singleLeg = - new SingleLegDefinition( - sortJson( - """ - {"$lexical": "cows"} - """), - CommandFeatures.EMPTY); - var sameSingleLeg = - new SingleLegDefinition( - sortJson( - """ - {"$lexical": "cows"} - """), - CommandFeatures.EMPTY); - var diffSingleLeg = - new SingleLegDefinition( - sortJson( - """ - {"$lexical": "sheep"} - """), - CommandFeatures.EMPTY); - - assertThat(value1).as("Object equals self").isEqualTo(value1); - assertThat(value1).as("same values, vector compared by value").isEqualTo(sameValue); - assertThat(value1).as("different form").isNotEqualTo(diffForm); - assertThat(value1).as("different vectorize sort").isNotEqualTo(diffVectorize); - assertThat(value1).as("different lexical sort").isNotEqualTo(diffLexical); - assertThat(value1).as("different vector").isNotEqualTo(diffVector); - assertThat(singleLeg).as("same single leg sort").isEqualTo(sameSingleLeg); - assertThat(singleLeg).as("different single leg sort").isNotEqualTo(diffSingleLeg); - - assertThat(value1.hashCode()).as("hash code equals self").isEqualTo(value1.hashCode()); - assertThat(value1.hashCode()) - .as("hash code same values, vector compared by value") - .isEqualTo(sameValue.hashCode()); - assertThat(value1.hashCode()) - .as("hash code different vectorize sort") - .isNotEqualTo(diffVectorize.hashCode()); - assertThat(value1.hashCode()) - .as("hash code different lexical sort") - .isNotEqualTo(diffLexical.hashCode()); - assertThat(value1.hashCode()) - .as("hash code different vector") - .isNotEqualTo(diffVector.hashCode()); - assertThat(singleLeg.hashCode()) - .as("hash code same single leg sort") - .isEqualTo(sameSingleLeg.hashCode()); - assertThat(singleLeg.hashCode()) - .as("hash code different single leg sort") - .isNotEqualTo(diffSingleLeg.hashCode()); - } - - /** The parser reads a single leg sort as an object, so other JSON is a bug in the caller. */ - @Test - public void singleLegDefinitionRejectsNonObject() { - var ex = - assertThrowsExactly( - IllegalArgumentException.class, - () -> new SingleLegDefinition(TextNode.valueOf("cows"), CommandFeatures.EMPTY)); - assertThat(ex).hasMessageContaining("sort must be an object or MissingNode"); - } - - @ParameterizedTest - @MethodSource("validSortsTestCases") - public void validSorts(String jsonString, FindAndRerankSortDefinition expected) throws Exception { - - LOGGER.debug("validSorts() - jsonString: {}", jsonString); - ObjectMapper mapper = new ObjectMapper(); - when(deserializationContext.readTree(jsonParser)).thenReturn(mapper.readTree(jsonString)); - - FindAndRerankSortDefinition actual = - deserializer.deserialize(jsonParser, deserializationContext); - LOGGER.debug("validSorts() - actual: {}", PrettyPrintable.pprint(actual)); - - assertEquals(expected, actual); - } - - private static Stream validSortsTestCases() { - - float[] emptyVector = new float[] {}; - String emptyVectorBase64 = encodeAsMimeBase64(floatsToBytes(emptyVector)); - float[] vector = new float[] {1.1f, 2.2f, 3.3f}; - String vectorBase64 = encodeAsMimeBase64(floatsToBytes(vector)); - - return Stream.of( - Arguments.of( - """ - {} - """, - FindAndRerankSortDefinition.NO_ARG_SORT), - Arguments.of( - """ - null - """, - FindAndRerankSortDefinition.NO_ARG_SORT), - // ---- - // $hybrid only - Arguments.of( - """ - { "$hybrid" : "same for hybrid and lexical" } - """, - new HybridText("same for hybrid and lexical", CommandFeatures.of(HYBRID))), - Arguments.of( - """ - { "$hybrid" : " padded text " } - """, - new HybridText("padded text", CommandFeatures.of(HYBRID))), - Arguments.of( - """ - { "$hybrid" : "" } - """, - new HybridText(null, CommandFeatures.of(HYBRID))), - Arguments.of( - """ - { "$hybrid" : " " } - """, - new HybridText(null, CommandFeatures.of(HYBRID))), - // ---- - // maximum fields, resolver works out the valid combinations - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : "lexical sort", "$vector" : [1.1, 2.2, 3.3]} } - """, - new HybridObject( - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.of(HYBRID, LEXICAL, VECTOR, VECTORIZE))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : "lexical sort"} } - """, - new HybridObject( - "vectorize sort", - "lexical sort", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), - // ---- - // $lexical variations - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : null} } - """, - new HybridObject( - "vectorize sort", null, null, CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : ""} } - """, - new HybridObject( - "vectorize sort", null, null, CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort"} } - """, - new HybridObject("vectorize sort", null, null, CommandFeatures.of(HYBRID, VECTORIZE))), - // ---- - // $vectorize variations - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : "lexical sort"} } - """, - new HybridObject( - "vectorize sort", - "lexical sort", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : null, "$lexical" : "lexical sort"} } - """, - new HybridObject( - null, "lexical sort", null, CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "", "$lexical" : "lexical sort"} } - """, - new HybridObject( - null, "lexical sort", null, CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), - Arguments.of( - """ - { "$hybrid" : {"$lexical" : "lexical sort"} } - """, - new HybridObject(null, "lexical sort", null, CommandFeatures.of(HYBRID, LEXICAL))), - // ---- - // $vector variations - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : null} } - """, - new HybridObject( - "vectorize", - "lexical", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : [0.1, 0.2, 0.3]} } - """, - new HybridObject( - "vectorize", - "lexical", - new float[] {0.1f, 0.2f, 0.3f}, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } - """ - .formatted(emptyVectorBase64), - new HybridObject( - "vectorize", - "lexical", - emptyVector, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } - """ - .formatted(vectorBase64), - new HybridObject( - "vectorize", - "lexical", - vector, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR)))); - } - - /** - * A sort without $hybrid is read by field name only and kept as sent, {@link - * FindAndRerankSortParser} checks the values. - */ - @ParameterizedTest - @MethodSource("singleLegSortsTestCases") - public void singleLegSorts(String jsonString, CommandFeatures expectedFeatures) throws Exception { - - LOGGER.debug("singleLegSorts() - jsonString: {}", jsonString); - when(deserializationContext.readTree(jsonParser)).thenReturn(MAPPER.readTree(jsonString)); - - FindAndRerankSortDefinition actual = - deserializer.deserialize(jsonParser, deserializationContext); - LOGGER.debug("singleLegSorts() - actual: {}", PrettyPrintable.pprint(actual)); - - // the expected sort is a separate parse of the same JSON - assertEquals(new SingleLegDefinition(sortJson(jsonString), expectedFeatures), actual); - } - - private static Stream singleLegSortsTestCases() { - - String vectorBase64 = encodeAsMimeBase64(floatsToBytes(new float[] {1.1f, 2.2f, 3.3f})); - - return Stream.of( - Arguments.of( - """ - { "$vectorize" : "vectorize sort" } - """, - CommandFeatures.of(VECTORIZE)), - Arguments.of( - """ - { "$vector" : [1.1, 2.2, 3.3] } - """, - CommandFeatures.of(VECTOR)), - Arguments.of( - """ - { "$vector" : {"$binary": "%s"} } - """ - .formatted(vectorBase64), - CommandFeatures.of(VECTOR)), - Arguments.of( - """ - { "$lexical" : "lexical sort" } - """, - CommandFeatures.of(LEXICAL)), - Arguments.of( - """ - { "age" : -1, "name" : 1 } - """, - CommandFeatures.EMPTY), - // ---- - // not checked here, FindAndRerankSortParser checks the values - Arguments.of( - """ - { "$lexical" : null } - """, - CommandFeatures.of(LEXICAL)), - Arguments.of( - """ - { "$vector" : [1.1, 2.2, 3.3], "age" : 1 } - """, - CommandFeatures.of(VECTOR)), - Arguments.of( - """ - { "age" : 2 } - """, - CommandFeatures.EMPTY)); - } - - @ParameterizedTest - @MethodSource("invalidSortsTestCases") - public void invalidSorts(String jsonString, String message) throws Exception { - - LOGGER.debug("invalidSorts() - jsonString: {}", jsonString); - ObjectMapper mapper = new ObjectMapper(); - when(deserializationContext.readTree(jsonParser)).thenReturn(mapper.readTree(jsonString)); - - var error = - assertThrows( - JsonMappingException.class, - () -> deserializer.deserialize(jsonParser, deserializationContext)); - LOGGER.debug("validSorts() - error.getMessage(): {}", error.getMessage()); - - assertThat(error).isInstanceOf(JsonMappingException.class).hasMessageContaining(message); - } - - private static Stream invalidSortsTestCases() { - return Stream.of( - // ---- - // unexpected fields - Arguments.of( - """ - 1 - """, - "sort clause must be an object"), - Arguments.of( - """ - { "$hybrid" : "text", "fake" : 1} - """, - "Expected fields: $hybrid. Unexpected fields: fake"), - Arguments.of( - """ - { "$hybrid" : {"fake" : 1} } - """, - "Expected fields: $lexical, $vectorize, $vector. Unexpected fields: fake"), - // ---- - // $hybrid only - Arguments.of( - """ - { "$hybrid" : 1} - """, - "Field $hybrid may only be of types TextNode, ObjectNode, but got: IntNode"), - // ---- - // Combinations - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : 1, "$lexical" : "", "$vector" : [1.1, 2.2, 3.3]} } - """, - "Field $vectorize may only be of types NullNode, TextNode, but got: IntNode"), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "", "$lexical" : 1, "$vector" : [1.1, 2.2, 3.3]} } - """, - "Field $lexical may only be of types NullNode, TextNode, but got: IntNode"), - Arguments.of( - """ - { "$hybrid" : { "$vector" : "", "$lexical" : "", "$vector" : 1} } - """, - "Field $vector may only be of types NullNode, ArrayNode, ObjectNode, but got: IntNode"), - Arguments.of( - """ - { "$hybrid" : { "$vector" : "", "$lexical" : "", "$vector" : {"$date": "2023-01-01"}} } - """, - "`sort` clause (binary vector) contained fields with the wrong type. Field $vector may only be of types NullNode, ArrayNode, ObjectNode, but got: ObjectNode")); - } - - /** The sort JSON as the deserializer gets it. */ - private static JsonNode sortJson(String json) { - try { - return MAPPER.readTree(json); - } catch (JsonProcessingException e) { - throw new IllegalArgumentException("test JSON does not parse: " + json, e); - } - } -} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinitionTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinitionTest.java new file mode 100644 index 0000000000..254d82d469 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinitionTest.java @@ -0,0 +1,136 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; + +import static io.stargate.sgv2.jsonapi.metrics.CommandFeature.*; +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.stargate.sgv2.jsonapi.api.model.command.impl.FindAndRerankCommand; +import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Tests for {@link FindAndRerankSortDefinition}: Jackson keeps any sort as sent, and the metrics + * features come from the field names. + */ +class FindAndRerankSortDefinitionTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + /** Reading the command does not check the sort, so these sorts are kept as sent. */ + @ParameterizedTest(name = "{0}") + @ValueSource( + strings = { + "{\"$hybrid\": \"cheese\"}", + "{\"$hybrid\": 1}", + "{\"$hybrid\": {\"fake\": 1}}", + "{\"age\": 2}", + "{}", + "\"cheese\"", + "1", + "[1, 2]" + }) + void keepsAnySortAsSent(String sort) throws Exception { + var command = + MAPPER.readValue( + "{\"findAndRerank\": {\"sort\": " + sort + "}}", FindAndRerankCommand.class); + + assertThat(command.sortClause().json()).isEqualTo(MAPPER.readTree(sort)); + } + + /** A command with no sort or a null sort gets {@link FindAndRerankSortDefinition#NO_ARG_SORT}. */ + @ParameterizedTest(name = "{0}") + @ValueSource(strings = {"{\"findAndRerank\": {}}", "{\"findAndRerank\": {\"sort\": null}}"}) + void usesNoArgSortWhenThereIsNoSort(String json) throws Exception { + var command = MAPPER.readValue(json, FindAndRerankCommand.class); + + assertThat(command.sortClause()).isSameAs(FindAndRerankSortDefinition.NO_ARG_SORT); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("commandFeaturesCases") + void recordsCommandFeaturesFromFieldNames(String sort, CommandFeatures expected) + throws Exception { + var definition = new FindAndRerankSortDefinition(MAPPER.readTree(sort)); + + assertThat(definition.commandFeatures()).isEqualTo(expected); + } + + static Stream commandFeaturesCases() { + return Stream.of( + // ---- + // $hybrid + Arguments.of( + """ + {"$hybrid": "cheese"} + """, + CommandFeatures.of(HYBRID)), + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows", "$vector": [1.1, 2.2]}} + """, + CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL, VECTOR)), + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "$lexical": null}} + """, + CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL)), + Arguments.of( + """ + {"$hybrid": {"$lexical": "cows"}} + """, + CommandFeatures.of(HYBRID, LEXICAL)), + Arguments.of( + """ + {"$hybrid": 1} + """, + CommandFeatures.of(HYBRID)), + // ---- + // single leg sorts + Arguments.of( + """ + {"$vectorize": "cheese"} + """, + CommandFeatures.of(VECTORIZE)), + Arguments.of( + """ + {"$vector": [1.1, 2.2]} + """, + CommandFeatures.of(VECTOR)), + Arguments.of( + """ + {"$lexical": null} + """, + CommandFeatures.of(LEXICAL)), + Arguments.of( + """ + {"$vector": [1.1, 2.2], "age": 1} + """, + CommandFeatures.of(VECTOR)), + Arguments.of( + """ + {"age": -1, "name": 1} + """, + CommandFeatures.EMPTY), + Arguments.of( + """ + {} + """, + CommandFeatures.EMPTY), + Arguments.of( + """ + "cheese" + """, + CommandFeatures.EMPTY)); + } + + @Test + void noArgSortHasNoCommandFeatures() { + assertThat(FindAndRerankSortDefinition.NO_ARG_SORT.commandFeatures()) + .isEqualTo(CommandFeatures.EMPTY); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java index 120d639a59..f4689f986f 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java @@ -1,6 +1,5 @@ package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; -import static io.stargate.sgv2.jsonapi.metrics.CommandFeature.*; import static io.stargate.sgv2.jsonapi.util.Base64Util.encodeAsMimeBase64; import static io.stargate.sgv2.jsonapi.util.CqlVectorUtil.floatsToBytes; import static org.assertj.core.api.Assertions.assertThat; @@ -8,12 +7,12 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.NullNode; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.FilterLeg; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridObject; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.HybridText; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.LexicalLeg; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort.VectorLeg; -import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSortDefinition.SingleLegDefinition; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.FieldSortExpression; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.LexicalSortExpression; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression2.VectorSortExpression; @@ -22,7 +21,6 @@ import io.stargate.sgv2.jsonapi.exception.DocumentException; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.SortException; -import io.stargate.sgv2.jsonapi.metrics.CommandFeatures; import java.util.List; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -31,8 +29,8 @@ import org.junit.jupiter.params.provider.MethodSource; /** - * Tests for {@link FindAndRerankSortParser}: the sort each form of single leg sort parses into, and - * the error for each syntax mistake. + * Tests for {@link FindAndRerankSortParser}: the sort each form of sort parses into, and the error + * for each syntax mistake. */ class FindAndRerankSortParserTest { @@ -41,23 +39,10 @@ class FindAndRerankSortParserTest { /** The vector in the $vector cases, sent as an array or as $binary. */ private static final float[] VECTOR = {1.1f, 2.2f, 3.3f}; - // =========================================================================================== - // $hybrid - // =========================================================================================== - - /** The deserializer still parses $hybrid, the parser returns it unchanged. */ - @Test - void returnsHybridSortUnchanged() { - var hybridText = new HybridText("cheese", CommandFeatures.of(HYBRID)); - var hybridObject = - new HybridObject("cheese", "cows", null, CommandFeatures.of(HYBRID, VECTORIZE, LEXICAL)); - - assertThat(FindAndRerankSortParser.parse(hybridText)).isSameAs(hybridText); - assertThat(FindAndRerankSortParser.parse(hybridObject)).isSameAs(hybridObject); - } + private static final String VECTOR_BASE64 = encodeAsMimeBase64(floatsToBytes(VECTOR)); // =========================================================================================== - // Single leg sorts + // No sort, and a sort that is not an object // =========================================================================================== /** No sort, a null sort and an empty sort are the filter leg, without sorting. */ @@ -66,11 +51,252 @@ void parsesNoSortToFilterLegWithoutSorts() throws Exception { var noSorts = new FilterLeg(List.of()); assertThat(FindAndRerankSortParser.parse(FindAndRerankSortDefinition.NO_ARG_SORT)) - .as("no sort, null sort and empty sort from the deserializer") + .as("no sort") + .isEqualTo(noSorts); + assertThat( + FindAndRerankSortParser.parse(new FindAndRerankSortDefinition(NullNode.getInstance()))) + .as("null sort") .isEqualTo(noSorts); - assertThat(parse("{}")).as("empty object").isEqualTo(noSorts); + assertThat(parse("{}")).as("empty sort").isEqualTo(noSorts); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("notObjectCases") + void rejectsSortThatIsNotAnObject(String sort, String nodeType) { + var ex = assertThrowsExactly(SortException.class, () -> parse(sort)); + + assertThat(ex.code).isEqualTo(SortException.Code.SORT_CLAUSE_INVALID.name()); + assertThat(ex) + .hasMessageContaining("sort clause must be submitted as JSON Object, not " + nodeType); + } + + static Stream notObjectCases() { + return Stream.of( + Arguments.of("\"cheese\"", "String"), + Arguments.of("1", "Number"), + Arguments.of("[1, 2]", "Array")); + } + + // =========================================================================================== + // $hybrid + // =========================================================================================== + + @ParameterizedTest(name = "{0}") + @MethodSource("hybridCases") + void parsesHybridSort(String sort, FindAndRerankSort expected) throws Exception { + assertThat(parse(sort)).isEqualTo(expected); + } + + static Stream hybridCases() { + return Stream.of( + Arguments.of( + """ + {"$hybrid": "cheese"} + """, + new HybridText("cheese")), + Arguments.of( + """ + {"$hybrid": " padded text "} + """, + new HybridText("padded text")), + // ---- + // $hybrid object, values that are null or blank are left out + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows", "$vector": [1.1, 2.2, 3.3]}} + """, + new HybridObject("cheese", "cows", VECTOR)), + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "$lexical": "cows"}} + """, + new HybridObject("cheese", "cows", null)), + Arguments.of( + """ + {"$hybrid": {"$vectorize": " padded text "}} + """, + new HybridObject("padded text", null, null)), + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "$lexical": null, "$vector": null}} + """, + new HybridObject("cheese", null, null)), + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "$lexical": " "}} + """, + new HybridObject("cheese", null, null)), + Arguments.of( + """ + {"$hybrid": {"$vector": [1.1, 2.2, 3.3], "$lexical": "cows"}} + """, + new HybridObject(null, "cows", VECTOR)), + Arguments.of( + """ + {"$hybrid": {"$vectorize": null, "$vector": [1.1, 2.2, 3.3]}} + """, + new HybridObject(null, null, VECTOR)), + Arguments.of( + """ + {"$hybrid": {"$vector": {"$binary": "%s"}}} + """ + .formatted(VECTOR_BASE64), + new HybridObject(null, null, VECTOR))); } + @ParameterizedTest(name = "{0}") + @MethodSource("invalidHybridCases") + void rejectsHybridSort( + String sort, + Class exceptionClass, + ErrorCode code, + String message) { + assertRejected(sort, exceptionClass, code, message); + } + + static Stream invalidHybridCases() { + return Stream.of( + // ---- + // structure + Arguments.of( + """ + {"$hybrid": "cheese", "age": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_INVALID, + "when sorting by field '$hybrid' no other sort expressions allowed"), + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "fake": 1}} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_INVALID, + "a $hybrid object can only have $vectorize, $vector and $lexical, not 'fake'"), + // ---- + // value types + Arguments.of( + """ + {"$hybrid": 1} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path '$hybrid' not valid: value must be a String or an Object, not Number"), + Arguments.of( + """ + {"$hybrid": null} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "value must be a String or an Object, not Null"), + Arguments.of( + """ + {"$hybrid": {"$vectorize": 1}} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path '$hybrid.$vectorize' not valid: value must be a String or null, not Number"), + Arguments.of( + """ + {"$hybrid": {"$vectorize": "cheese", "$lexical": ["cows"]}} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path '$hybrid.$lexical' not valid: value must be a String or null, not Array"), + Arguments.of( + """ + {"$hybrid": {"$vector": "cheese"}} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "on path '$hybrid.$vector' not valid: value must be an Array of numbers or a $binary vector, not String"), + Arguments.of( + """ + {"$hybrid": {"$vector": {"$date": 1}}} + """, + SortException.class, + SortException.Code.SORT_CLAUSE_VALUE_INVALID, + "only binary vector object values are supported for sorting"), + Arguments.of( + """ + {"$hybrid": {"$vector": []}} + """, + DocumentException.class, + DocumentException.Code.SHRED_BAD_VECTOR_SIZE, + "cannot be empty Array")); + } + + /** A $hybrid sort must have a value for the vector leg. */ + @ParameterizedTest(name = "{0}") + @MethodSource("hybridWithNoValueCases") + void rejectsHybridSortWithNoValue(String sort, SortException.Code code) { + var ex = assertThrowsExactly(SortException.class, () -> parse(sort)); + + assertThat(ex.code).isEqualTo(code.name()); + } + + static Stream hybridWithNoValueCases() { + return Stream.of( + Arguments.of( + """ + {"$hybrid": ""} + """, + SortException.Code.MISSING_HYBRID_SORT_VALUE), + Arguments.of( + """ + {"$hybrid": " "} + """, + SortException.Code.MISSING_HYBRID_SORT_VALUE), + Arguments.of( + """ + {"$hybrid": {}} + """, + SortException.Code.MISSING_HYBRID_SORT_VALUE), + Arguments.of( + """ + {"$hybrid": {"$vectorize": null}} + """, + SortException.Code.MISSING_HYBRID_SORT_VALUE), + Arguments.of( + """ + {"$hybrid": {"$vector": null}} + """, + SortException.Code.MISSING_HYBRID_SORT_VALUE), + Arguments.of( + """ + {"$hybrid": {"$lexical": null}} + """, + SortException.Code.MISSING_HYBRID_SORT_VALUE), + Arguments.of( + """ + {"$hybrid": {"$lexical": ""}} + """, + SortException.Code.MISSING_HYBRID_SORT_VALUE), + Arguments.of( + """ + {"$hybrid": {"$lexical": "cows"}} + """, + SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), + Arguments.of( + """ + {"$hybrid": {"$lexical": "cows", "$vectorize": null}} + """, + SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), + Arguments.of( + """ + {"$hybrid": {"$lexical": "cows", "$vectorize": " "}} + """, + SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), + Arguments.of( + """ + {"$hybrid": {"$lexical": "cows", "$vector": null}} + """, + SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT)); + } + + // =========================================================================================== + // Single leg sorts + // =========================================================================================== + @ParameterizedTest(name = "{0}") @MethodSource("singleLegCases") void parsesSingleLegSort(String sort, FindAndRerankSort expected) throws Exception { @@ -98,7 +324,7 @@ static Stream singleLegCases() { """ {"$vector": {"$binary": "%s"}} """ - .formatted(encodeAsMimeBase64(floatsToBytes(VECTOR))), + .formatted(VECTOR_BASE64), new VectorLeg(new VectorSortExpression(VECTOR))), Arguments.of( """ @@ -146,10 +372,7 @@ void rejectsSingleLegSort( Class exceptionClass, ErrorCode code, String message) { - var ex = assertThrowsExactly(exceptionClass, () -> parse(sort)); - - assertThat(ex.code).isEqualTo(code.name()); - assertThat(ex).hasMessageContaining(message); + assertRejected(sort, exceptionClass, code, message); } static Stream invalidSingleLegCases() { @@ -373,9 +596,20 @@ static Stream invalidSingleLegCases() { // Helpers / no more tests below // =========================================================================================== - /** Parses the sort the way the deserializer keeps a sort without $hybrid. */ + /** Parses the sort JSON, as the command keeps it. */ private static FindAndRerankSort parse(String sort) throws JsonProcessingException { - return FindAndRerankSortParser.parse( - new SingleLegDefinition(MAPPER.readTree(sort), CommandFeatures.EMPTY)); + return FindAndRerankSortParser.parse(new FindAndRerankSortDefinition(MAPPER.readTree(sort))); + } + + /** Asserts parsing fails with the exception, the code and a message that has the text. */ + private static void assertRejected( + String sort, + Class exceptionClass, + ErrorCode code, + String message) { + var ex = assertThrowsExactly(exceptionClass, () -> parse(sort)); + + assertThat(ex.code).isEqualTo(code.name()); + assertThat(ex).hasMessageContaining(message); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java index 66cd00fc55..44ce56b4e9 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java @@ -57,6 +57,40 @@ void reportsSortSyntaxErrorBeforeOptionErrors() throws Exception { assertThat(ex.code).isEqualTo(SortException.Code.SORT_CLAUSE_VALUE_INVALID.name()); } + /** A $hybrid sort with no value is a sort error, so it is reported before the missing options. */ + @Test + void reportsHybridSortWithNoValueBeforeOptionErrors() throws Exception { + var command = + objectMapper.readValue( + """ + {"findAndRerank": {"sort": {"$hybrid": ""}}} + """, + FindAndRerankCommand.class); + var commandContext = commandContext(true); + + var ex = + assertThrowsExactly( + SortException.class, () -> resolver.resolveCollectionCommand(commandContext, command)); + assertThat(ex.code).isEqualTo(SortException.Code.MISSING_HYBRID_SORT_VALUE.name()); + } + + /** Reading the command does not check the sort, so a sort that is not an object fails here. */ + @Test + void reportsSortThatIsNotAnObject() throws Exception { + var command = + objectMapper.readValue( + """ + {"findAndRerank": {"sort": "cheese"}} + """, + FindAndRerankCommand.class); + var commandContext = commandContext(true); + + var ex = + assertThrowsExactly( + SortException.class, () -> resolver.resolveCollectionCommand(commandContext, command)); + assertThat(ex.code).isEqualTo(SortException.Code.SORT_CLAUSE_INVALID.name()); + } + /** The reranking feature is checked before the sort is parsed. */ @Test void reportsRerankingDisabledBeforeSortSyntaxError() throws Exception { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java index dbb3c8ef3c..74c57ff7c3 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankPlannerTest.java @@ -239,72 +239,6 @@ void rejectsSort(String description, String sort, SortException.Code code) throw static Stream sortErrorCases() { return Stream.of( - Arguments.of( - "$hybrid text empty", - """ - {"$hybrid": ""} - """, - SortException.Code.MISSING_HYBRID_SORT_VALUE), - Arguments.of( - "$hybrid text blank", - """ - {"$hybrid": " "} - """, - SortException.Code.MISSING_HYBRID_SORT_VALUE), - Arguments.of( - "$hybrid object empty", - """ - {"$hybrid": {}} - """, - SortException.Code.MISSING_HYBRID_SORT_VALUE), - Arguments.of( - "$vectorize null", - """ - {"$hybrid": {"$vectorize": null}} - """, - SortException.Code.MISSING_HYBRID_SORT_VALUE), - Arguments.of( - "$vector null", - """ - {"$hybrid": {"$vector": null}} - """, - SortException.Code.MISSING_HYBRID_SORT_VALUE), - Arguments.of( - "$lexical null", - """ - {"$hybrid": {"$lexical": null}} - """, - SortException.Code.MISSING_HYBRID_SORT_VALUE), - Arguments.of( - "$lexical empty", - """ - {"$hybrid": {"$lexical": ""}} - """, - SortException.Code.MISSING_HYBRID_SORT_VALUE), - Arguments.of( - "$lexical only", - """ - {"$hybrid": {"$lexical": "cows"}} - """, - SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), - Arguments.of( - "$lexical with $vectorize null", - """ - {"$hybrid": {"$lexical": "cows", "$vectorize": null}} - """, - SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), - Arguments.of( - "$lexical with $vectorize blank", - """ - {"$hybrid": {"$lexical": "cows", "$vectorize": " "}} - """, - SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), - Arguments.of( - "$lexical with $vector null", - """ - {"$hybrid": {"$lexical": "cows", "$vector": null}} - """, - SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), Arguments.of("no sort", null, SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT), Arguments.of( "sort empty", @@ -632,27 +566,6 @@ void usesVectorizeTextWhenRerankOptionsAreBlank() throws Exception { // Error order // =========================================================================================== - /** - * The one error order the planner keeps on purpose, see the NOTE in {@link - * FindAndRerankPlanner#plan}: the options are checked before the legs are planned. This command - * has no rerank query and a $hybrid sort with no value, it fails for the rerank query. - */ - @Test - void reportsMissingRerankQueryBeforeHybridSortWithNoValue() throws Exception { - assertPlanFails( - "no rerank query and an empty $hybrid sort", - command( - """ - {"$hybrid": ""} - """, - """ - {"rerankOn": "body"} - """), - Collection.VECTOR_LEXICAL, - RequestException.class, - RequestException.Code.MISSING_RERANK_QUERY_TEXT); - } - /** * A single leg sort is rejected after the options are checked, the same as a $hybrid sort is * planned after them. This command has no rerank query, it fails for the rerank query.