diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java index a3294cf9a0..73536cfa57 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializer.java @@ -22,8 +22,8 @@ /** * {@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. + *

Checks the JSON structure and mutually exclusive vector keys. Semantic validation is handled + * when resolving the command to an operation. */ public class FindAndRerankSortClauseDeserializer extends StdDeserializer { @@ -72,8 +72,7 @@ public FindAndRerankSort deserialize( * Deserialize the sort clause from an object node. * * @param sort The sort clause as an object node, no check for nulls. - * @return The {@link FindAndRerankSort} that reflects the request without validation, e.g. - * checking that a vectorize or vector sort is provided but not both. + * @return The {@link FindAndRerankSort} that reflects the request after structural validation. */ private static FindAndRerankSort deserialize(JsonParser jsonParser, ObjectNode sort) throws JsonMappingException { @@ -105,6 +104,12 @@ private static FindAndRerankSort deserializeHybridObject( JsonParser jsonParser, ObjectNode hybridObject) throws JsonMappingException { var sortMatch = MATCH_SORT_FIELDS.matchAndThrow(hybridObject, jsonParser, ERROR_CONTEXT); + if (hybridObject.has(VECTOR_EMBEDDING_FIELD) && hybridObject.has(VECTOR_EMBEDDING_TEXT_FIELD)) { + throw new JsonMappingException( + jsonParser, + "$vector and $vectorize cannot be used together in a $hybrid sort", + jsonParser.currentLocation()); + } CommandFeatures commandFeatures = CommandFeatures.of(CommandFeature.HYBRID); var vectorizeText = 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..f4c2fb617d 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 @@ -184,8 +184,6 @@ private static List multiLegs( // the vector leg reads with the vector from the vectorize step when there is text to vectorize, // otherwise with the vector from the command - // NOTE: when the sort has both, $vectorize wins and $vector is ignored - // TODO: GH #2616 - reject a sort that has both $vectorize and $vector legs.add( new FindAndRerankPlan.RerankLeg( RerankLegType.VECTOR_LEG, diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java index e3a66280f2..5dc4e47344 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/FindAndRerankSortClauseDeserializerTest.java @@ -190,18 +190,6 @@ private static Stream validSortsTestCases() { 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"} } @@ -298,50 +286,73 @@ private static Stream validSortsTestCases() { // $vector variations Arguments.of( """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : null} } + { "$hybrid" : { "$lexical" : "lexical", "$vector" : null} } """, new FindAndRerankSort( RerankLegMode.MULTI_LEG, null, - "vectorize", + null, "lexical", null, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), + CommandFeatures.of(HYBRID, LEXICAL, VECTOR))), Arguments.of( """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : [0.1, 0.2, 0.3]} } + { "$hybrid" : { "$lexical" : "lexical", "$vector" : [0.1, 0.2, 0.3]} } """, new FindAndRerankSort( RerankLegMode.MULTI_LEG, null, - "vectorize", + null, "lexical", new float[] {0.1f, 0.2f, 0.3f}, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), + CommandFeatures.of(HYBRID, LEXICAL, VECTOR))), Arguments.of( """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } + { "$hybrid" : { "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } """ .formatted(emptyVectorBase64), new FindAndRerankSort( RerankLegMode.MULTI_LEG, null, - "vectorize", + null, "lexical", emptyVector, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR))), + CommandFeatures.of(HYBRID, LEXICAL, VECTOR))), Arguments.of( """ - { "$hybrid" : { "$vectorize" : "vectorize", "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } + { "$hybrid" : { "$lexical" : "lexical", "$vector" : {"$binary": "%s"}} } """ .formatted(vectorBase64), new FindAndRerankSort( RerankLegMode.MULTI_LEG, null, - "vectorize", + null, "lexical", vector, - CommandFeatures.of(HYBRID, LEXICAL, VECTORIZE, VECTOR)))); + CommandFeatures.of(HYBRID, LEXICAL, VECTOR)))); + } + + @ParameterizedTest + @MethodSource("conflictingVectorSorts") + void rejectsBothVectorKeysRegardlessOfTheirValues(String vectorize, String vector) + throws Exception { + String sort = + "{\"$hybrid\": {\"$vectorize\": %s, \"$vector\": %s}}".formatted(vectorize, vector); + + var error = + assertThrows( + JsonMappingException.class, + () -> new ObjectMapper().readValue(sort, FindAndRerankSort.class)); + + assertThat(error).hasMessageContaining("$vector and $vectorize cannot be used together"); + } + + private static Stream conflictingVectorSorts() { + return Stream.of("\"search text\"", "null", "\"\"", "\" \"") + .flatMap( + vectorize -> + Stream.of("[1, 2, 3]", "[]", "null", "{\"$binary\": \"P4AAAEAAAABAQAAA\"}") + .map(vector -> Arguments.of(vectorize, vector))); } @ParameterizedTest @@ -391,12 +402,12 @@ private static Stream invalidSortsTestCases() { // Combinations Arguments.of( """ - { "$hybrid" : { "$vectorize" : 1, "$lexical" : "", "$vector" : [1.1, 2.2, 3.3]} } + { "$hybrid" : { "$vectorize" : 1, "$lexical" : ""} } """, "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]} } + { "$hybrid" : { "$vectorize" : "", "$lexical" : 1} } """, "Field $lexical may only be of types NullNode, TextNode, but got: IntNode"), Arguments.of( 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..d12956cff2 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 @@ -128,15 +128,6 @@ static Stream legCases() { Collection.VECTOR_LEXICAL, null, List.of(vectorLeg(DEFAULT_READ_LIMIT))), - Arguments.of( - "$vectorize and $vector, $vectorize is used", - """ - {"$hybrid": {"$vectorize": "cheese", "$vector": [0.1, 0.2, 0.3]}} - """, - RERANK_OPTIONS, - Collection.VECTORIZE_NO_LEXICAL, - "cheese", - List.of(vectorizedLeg(DEFAULT_READ_LIMIT))), Arguments.of( "$vectorize and $lexical", """