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..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 @@ -136,6 +136,10 @@ 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)); + } + + 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 +176,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 +186,7 @@ private void validateSortExpressionPaths(ObjectNode sortNode) { } } - private void validateSortExpressionPath(String path) { + 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..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 @@ -62,7 +62,7 @@ public SortClause build(JsonNode node) { */ protected abstract SortClause buildClauseFromDefinition(ObjectNode sortNode); - protected float[] tryDecodeBinaryVector(String path, JsonNode innerValue) { + 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..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,161 +1,130 @@ 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.metrics.CommandFeatures; +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.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 parsed sort of a findAndRerank command, with one record for each form of sort. * - * @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. + * */ -@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 { + + /** A sort with $hybrid. It always has a value for the vector leg. */ + sealed interface MultiLeg extends FindAndRerankSort permits HybridText, HybridObject {} /** - * 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. + * {"$hybrid": "cheese"}: the same text for the vector leg and the lexical leg. + * + * @param text The trimmed text. It is never blank. */ - @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"); - } + record HybridText(String text) implements MultiLeg { + + public HybridText { + 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("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); + } } /** - * Override to do a value equality check on the vector + * {"$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 obj the reference object with which to compare. - * @return + *

At least one of $vectorize and $vector is set. $lexical can be null. + */ + record HybridObject(String vectorize, String lexical, float[] vector) implements MultiLeg { + + public HybridObject { + if (vectorize == null && vector == null) { + throw new IllegalArgumentException("vectorize or vector must not be null"); + } + } + + @Override + public DataRecorder recordTo(DataRecorder dataRecorder) { + return dataRecorder + .append("vectorize", vectorize) + .append("lexical", lexical) + .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); + } + + /** Override to do a value equality hash on the vector. */ + @Override + public int hashCode() { + return Objects.hash(vectorize, lexical, Arrays.hashCode(vector)); + } + } + + /** A sort without $hybrid, the command reads with one leg. */ + 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. + */ + 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 deleted file mode 100644 index a3294cf9a0..0000000000 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java +++ /dev/null @@ -1,233 +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 FindAndRerankSort}. - * - *

Note: This only checks the shape of the sort clause, the other validation is handled when - * resolving the command to an operation. - */ -public class FindAndRerankSortClauseDeserializer extends StdDeserializer { - - private static final String ERROR_CONTEXT = "`sort` clause"; - - // the only field we accept at top level is $hybrid - // 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)); - - public FindAndRerankSortClauseDeserializer() { - super(FindAndRerankSort.class); - } - - /** {@inheritDoc} */ - @Override - public FindAndRerankSort deserialize( - JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException { - - return switch (deserializationContext.readTree(jsonParser)) { - case NullNode ignored -> // this is {"sort" : null} - FindAndRerankSort.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 FindAndRerankSort} 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) - throws JsonMappingException { - - // { "sort" : { } } - if (sort.isEmpty()) { - return FindAndRerankSort.NO_ARG_SORT; - } - - 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)); - 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 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( - RerankLegMode.MULTI_LEG, null, vectorizeText, lexicalText, vector, 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 new file mode 100644 index 0000000000..f22a54a4ed --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortDefinition.java @@ -0,0 +1,148 @@ +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.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.MissingNode; +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; +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 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 their name. + * + * @param json The sort as the user sent it. {@link MissingNode} when the command has no sort. + */ +@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 record FindAndRerankSortDefinition(@JsonIgnore JsonNode json) implements Recordable { + + /** A command with no sort. */ + public static final FindAndRerankSortDefinition NO_ARG_SORT = + new FindAndRerankSortDefinition(MissingNode.getInstance()); + + /** 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"); + } + + /** + * The metrics features of the sort, from its field names only. A field set to null + * still counts. + */ + 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; + } + + 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 + // 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 {} + + /** + * 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}) + 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 new file mode 100644 index 0000000000..ace1805c0a --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParser.java @@ -0,0 +1,265 @@ +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 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; +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; +import java.util.Set; + +/** + * Parses the sort of a findAndRerank command into a {@link FindAndRerankSort}. + * + *

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

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

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 + } + + /** + * Parses the sort, throws a {@link SortException} when its syntax is not valid. + * + * @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 + // =========================================================================================== + + /** {"$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)))); + }; + } + + /** 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); + + // 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()); + } + + // $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(VECTOR_EMBEDDING_FIELD, onlyField(sort, VECTOR_EMBEDDING_FIELD)))); + } + + // { "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 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( + "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(ObjectNode 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(); + } + + /** 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(path, objectNode); + case JsonNode node -> + throw SortException.Code.SORT_CLAUSE_VALUE_INVALID.get( + Map.of( + "path", + 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/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..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 @@ -24,7 +24,13 @@ /** * 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. + * + *

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 { @@ -36,16 +42,19 @@ private FindAndRerankPlanner() { * Validates the command and makes its plan. * * @param command The command to plan. + * @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. */ 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,21 +65,16 @@ 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 - // 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.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; @@ -162,18 +166,12 @@ 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, CollectionSchemaObject schemaObject) { - - var sort = command.sortClause(); - if (!isVectorizeSort(sort) && !isVectorSort(sort)) { - throw isLexicalSort(sort) - ? SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT.get() - : SortException.Code.MISSING_HYBRID_SORT_VALUE.get(); - } + FindAndRerankCommand command, + FindAndRerankSort.MultiLeg multiLeg, + CollectionSchemaObject schemaObject) { var hybridLimits = getOrDefault( @@ -189,18 +187,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 +259,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 +286,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 +322,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 +331,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 deleted file mode 100644 index e3a66280f2..0000000000 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java +++ /dev/null @@ -1,413 +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.mockito.Mockito.when; - -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JsonMappingException; -import com.fasterxml.jackson.databind.ObjectMapper; -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 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 FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - - var diffVectorize = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort 2", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - var diffLexical = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort", - "lexical sort 2", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - var diffVector = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f, 4.4f}, - CommandFeatures.EMPTY); - - var diffMode = - new FindAndRerankSort( - RerankLegMode.SINGLE_LEG, - null, - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - CommandFeatures.EMPTY); - var diffHybridSort = - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - "hybrid sort", - "vectorize sort", - "lexical sort", - new float[] {1.1f, 2.2f, 3.3f}, - 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("different vectorize sort").isNotEqualTo(diffVectorize); - assertThat(value1).as("different lexical sort").isNotEqualTo(diffLexical); - assertThat(value1).as("different vector").isNotEqualTo(diffVector); - - assertThat(value1.hashCode()).as("hash code equals self").isEqualTo(value1.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(value1.hashCode()) - .as("hash code different rerankLegMode") - .isNotEqualTo(diffMode.hashCode()); - assertThat(value1.hashCode()) - .as("hash code different hybrid sort") - .isNotEqualTo(diffHybridSort.hashCode()); - } - - @ParameterizedTest - @MethodSource("validSortsTestCases") - public void validSorts(String jsonString, FindAndRerankSort 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); - 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( - """ - {} - """, - FindAndRerankSort.NO_ARG_SORT), - Arguments.of( - """ - null - """, - FindAndRerankSort.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))), - Arguments.of( - """ - { "$hybrid" : " padded text " } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - "padded text", - null, - null, - null, - CommandFeatures.of(HYBRID))), - Arguments.of( - """ - { "$hybrid" : "" } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, null, null, null, null, CommandFeatures.of(HYBRID))), - Arguments.of( - """ - { "$hybrid" : " " } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, null, null, null, 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, - "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 FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize sort", - "lexical sort", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), - // ---- - // $lexical variations - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort", "$lexical" : null} } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "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))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize sort"} } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "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, - "vectorize sort", - "lexical sort", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : null, "$lexical" : "lexical sort"} } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - 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))), - Arguments.of( - """ - { "$hybrid" : {"$lexical" : "lexical sort"} } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - 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, - "vectorize", - "lexical", - null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : [0.1, 0.2, 0.3]} } - """, - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "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 FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize", - "lexical", - emptyVector, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), - Arguments.of( - """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } - """ - .formatted(vectorBase64), - new FindAndRerankSort( - RerankLegMode.MULTI_LEG, - null, - "vectorize", - "lexical", - vector, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR)))); - } - - @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( - """ - { "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")); - } -} 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 new file mode 100644 index 0000000000..f4689f986f --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortParserTest.java @@ -0,0 +1,615 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.sort; + +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 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.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 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 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}; + + private static final String VECTOR_BASE64 = encodeAsMimeBase64(floatsToBytes(VECTOR)); + + // =========================================================================================== + // 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. */ + @Test + void parsesNoSortToFilterLegWithoutSorts() throws Exception { + var noSorts = new FilterLeg(List.of()); + + assertThat(FindAndRerankSortParser.parse(FindAndRerankSortDefinition.NO_ARG_SORT)) + .as("no sort") + .isEqualTo(noSorts); + assertThat( + FindAndRerankSortParser.parse(new FindAndRerankSortDefinition(NullNode.getInstance()))) + .as("null sort") + .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 { + 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(VECTOR_BASE64), + 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) { + assertRejected(sort, exceptionClass, code, 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 JSON, as the command keeps it. */ + private static FindAndRerankSort parse(String sort) throws JsonProcessingException { + 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/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..44ce56b4e9 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankCommandResolverTest.java @@ -0,0 +1,117 @@ +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()); + } + + /** 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 { + 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..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 @@ -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) @@ -238,85 +239,63 @@ void rejectsSort(String description, String sort, SortException.Code code) throw static Stream sortErrorCases() { return Stream.of( + Arguments.of("no sort", null, SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT), 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", + "sort empty", """ - {"$hybrid": {"$lexical": "cows"}} + {} """, - SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), + SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT), Arguments.of( - "$lexical with $vectorize null", + "sort null", """ - {"$hybrid": {"$lexical": "cows", "$vectorize": null}} + null """, - SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), + 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( - "$lexical with $vectorize blank", + "vector leg, $vectorize", """ - {"$hybrid": {"$lexical": "cows", "$vectorize": " "}} - """, - SortException.Code.MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT), + {"$vectorize": "cheese"} + """), Arguments.of( - "$lexical with $vector null", + "vector leg, $vector", """ - {"$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), + {"$vector": [0.1, 0.2, 0.3]} + """), Arguments.of( - "sort empty", + "lexical leg", """ - {} - """, - SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT), + {"$lexical": "cows"} + """), Arguments.of( - "sort null", + "filter leg", """ - null - """, - SortException.Code.UNSUPPORTED_FIND_AND_RERANK_SORT)); + {"age": -1} + """)); } // =========================================================================================== @@ -449,9 +428,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 +546,7 @@ static Stream hybridLimitsErrorCases() { @Test void usesVectorizeTextWhenRerankOptionsAreBlank() throws Exception { var plan = - FindAndRerankPlanner.plan( + plan( command( """ {"$hybrid": {"$vectorize": "cheese"}} @@ -590,22 +567,21 @@ void usesVectorizeTextWhenRerankOptionsAreBlank() throws Exception { // =========================================================================================== /** - * 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. + * 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 reportsMissingRerankQueryBeforeHybridSortWithNoValue() throws Exception { + void reportsMissingRerankQueryBeforeSingleLegSort() throws Exception { assertPlanFails( - "no rerank query and an empty $hybrid sort", + "no rerank query and a single leg sort", command( """ - {"$hybrid": ""} + {"$lexical": "cows"} """, """ {"rerankOn": "body"} """), - Collection.VECTOR_LEXICAL, + Collection.VECTORIZE_LEXICAL, RequestException.class, RequestException.Code.MISSING_RERANK_QUERY_TEXT); } @@ -617,7 +593,7 @@ void reportsMissingRerankQueryBeforeHybridSortWithNoValue() throws Exception { @Test void plansHybridTextWithDefaults() throws Exception { var plan = - FindAndRerankPlanner.plan( + plan( command( """ {"$hybrid": " cheese "} @@ -655,7 +631,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 +668,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); }