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 6767f47f98..bc43ddad9c 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 @@ -88,6 +88,8 @@ private static FindAndRerankSort deserialize(JsonParser jsonParser, ObjectNode s case TextNode textNode -> { // using the same text for vectorize and for lexical, no vector var normalizedText = normalizedText(textNode.asText().trim()); + // NOTE: commandFeatures flags are used to determine user intent, changing + // impacts FindAndRerankOperationBuilder yield new FindAndRerankSort( normalizedText, normalizedText, null, CommandFeatures.of(CommandFeature.HYBRID)); } @@ -143,11 +145,15 @@ private static FindAndRerankSort deserializeHybridObject( case NullNode ignored -> { // explict setting to null is allowed // { "sort" : { "$hybrid" : { "$lexical" : null, + // NOTE: commandFeatures flags are used to determine user intent, changing + // impacts FindAndRerankOperationBuilder commandFeatures.addFeature(CommandFeature.LEXICAL); yield null; } case TextNode textNode -> { // { "sort" : { "$hybrid" : { "$lexical" : "cheese", + // NOTE: commandFeatures flags are used to determine user intent, changing + // impacts FindAndRerankOperationBuilder commandFeatures.addFeature(CommandFeature.LEXICAL); yield normalizedText(textNode.asText().trim()); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/metrics/CommandFeatures.java b/src/main/java/io/stargate/sgv2/jsonapi/metrics/CommandFeatures.java index 067e681428..f185a3f83b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/metrics/CommandFeatures.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/metrics/CommandFeatures.java @@ -56,6 +56,12 @@ public void addFeature(CommandFeature commandFeature) { commandFeatures.add(commandFeature); } + /** Test if the supplied feature is included in this set */ + public boolean contains(CommandFeature commandFeature) { + Objects.requireNonNull(commandFeature, "commandFeature cannot be null"); + return commandFeatures.contains(commandFeature); + } + /** * Adds all features from another {@code CommandFeatures} instance to this instance. Mutates the * current object. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilder.java index 7c511fb0b8..dad4f0a6a5 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilder.java @@ -17,6 +17,7 @@ import io.stargate.sgv2.jsonapi.exception.RequestException; import io.stargate.sgv2.jsonapi.exception.SchemaException; import io.stargate.sgv2.jsonapi.exception.SortException; +import io.stargate.sgv2.jsonapi.metrics.CommandFeature; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorColumnDefinition; import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProvider; import io.stargate.sgv2.jsonapi.service.operation.Operation; @@ -193,11 +194,10 @@ private void checkSortSupported() { } } - if (isLexicalSort()) { - if (!commandContext.schemaObject().lexicalDef().enabled()) { - throw SchemaException.Code.LEXICAL_NOT_ENABLED_FOR_COLLECTION.get( - errVars(commandContext.schemaObject())); - } + // The index is only required if the user explicitly asked for lexical, + // they could also have used $hybrid and it expanded into the lexical sort. + if (isLexicalSort() && isExplicitLexicalSort()) { + throwIfNoLexicalIndex(); } } @@ -345,6 +345,19 @@ private IntermediateCollectionReadTask buildBm25Read(DeferredCommandResultAction return null; } + // if there is a lexical sort, but the user did not ask explicitly for it + // then it is OK to skip. If the user explicitly asked for it and the index did not + // exist then we should have caught in checkSortSUpport() but also safety throw here. + if (!commandContext.schemaObject().lexicalDef().enabled()) { + if (isExplicitLexicalSort()) { + // error should have been caught in checkSortSUpport() safety here + throwIfNoLexicalIndex(); + } + // ok user only getting lexical because of $hybrid + deferredAction.setEmptyMultiDocumentResponse(); + return null; + } + var bm25SortTerm = command.sortClause().lexicalSort(); var bm25SortClause = new SortClause(List.of(SortExpression.collectionLexicalSort(bm25SortTerm))); @@ -457,10 +470,21 @@ private PathMatchLocator passageLocator() { return PathMatchLocator.forPath(finalRerankField); } + private void throwIfNoLexicalIndex() { + if (!commandContext.schemaObject().lexicalDef().enabled()) { + throw SchemaException.Code.LEXICAL_NOT_ENABLED_FOR_COLLECTION.get( + errVars(commandContext.schemaObject())); + } + } + private boolean isLexicalSort() { return command.sortClause().lexicalSort() != null; } + private boolean isExplicitLexicalSort() { + return command.sortClause().commandFeatures().contains(CommandFeature.LEXICAL); + } + private boolean isVectorizeSort() { return command.sortClause().vectorizeSort() != null; } diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index da315a0ac4..05c1023788 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -1421,7 +1421,10 @@ request-errors: title: Lexical search is not enabled for the collection body: |- Lexical content can only be added and filtering and sort only be used on collections for which Lexical feature is enabled. - The collection ${keyspace}.${table} does not have Lexical feature enabled. + + The collection without a lexical index: ${keyspace}.${table}. + + Resend the command without explicitly requesting a $lexical sort. - scope: SCHEMA code: MISSING_PARTITION_COLUMNS diff --git a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java index 8dd3949a92..d23c073916 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java @@ -122,6 +122,7 @@ public class TestConstants { public final CollectionSchemaObject COLLECTION_SCHEMA_OBJECT; public final CollectionSchemaObject COLLECTION_SCHEMA_OBJECT_LEGACY; public final CollectionSchemaObject VECTOR_COLLECTION_SCHEMA_OBJECT; + public final CollectionSchemaObject VECTORIZE_RERANK_COLLECTION_SCHEMA_OBJECT; public final CollectionSchemaObject VECTOR_LEXICAL_RERANK_COLLECTION_SCHEMA_OBJECT; public final TableSchemaObject TABLE_SCHEMA_OBJECT; public final KeyspaceSchemaObject KEYSPACE_SCHEMA_OBJECT; @@ -228,6 +229,27 @@ public TestConstants() { CollectionLexicalDefSchemaFactory.FOR_TESTING_DISABLED.currentVersion(null), CollectionRerankDefSchemaFactory.FOR_TESTING_DISABLED.currentVersion(null)); + // No Lexical + VECTORIZE_RERANK_COLLECTION_SCHEMA_OBJECT = + new CollectionSchemaObject( + COLLECTION_IDENTIFIER, + IdConfig.defaultIdConfig(), + VectorConfig.fromColumnDefinitions( + List.of( + new VectorColumnDefinition( + DocumentConstants.Fields.VECTOR_EMBEDDING_TEXT_FIELD, + -1, + SimilarityFunction.COSINE, + EmbeddingSourceModel.OTHER, + new VectorizeDefinition("custom", "custom", null, null)))), + null, + CollectionLexicalDefSchemaFactory.FOR_TESTING_DISABLED.currentVersion(null), + CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion( + new CollectionRerankDef( + true, + new CollectionRerankDef.RerankServiceDef( + "nvidia", "nvidia/llama-3.2-nv-rerankqa-1b-v2", null, null)))); + VECTOR_LEXICAL_RERANK_COLLECTION_SCHEMA_OBJECT = new CollectionSchemaObject( COLLECTION_IDENTIFIER, diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionIntegrationTest.java index f9d216d9fc..44dca4cbaf 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionIntegrationTest.java @@ -74,8 +74,59 @@ void failOnVectorizeDisabled() { "The collection %s.%s does not have vectorize enabled."); } + @Test + void successOnLexicalDisabledHybrid() { + // lexical disabled, and using $hybrid + // will get EmbeddingProviderException.Code.EMBEDDING_PROVIDER_CLIENT_ERROR.name() because + // using open AI with no token + errorOnNotEnabled( + "lexical_not_enabled", + """ + { + "name" : "%s", + "options": { + "vector": { + "metric": "cosine", + "dimension": 1024, + "service": { + "provider": "openai", + "modelName": "text-embedding-3-small" + } + }, + "lexical": { + "enabled": false + } + } + } + """, + EmbeddingProviderException.Code.EMBEDDING_PROVIDER_CLIENT_ERROR.name(), + "Incorrect API key provided:"); + } + @Test void failOnLexicalDisabled() { + // lexical disabled, and using $lexical + var rerank = + """ + {"findAndRerank": { + "filter": {}, + "projection": {}, + "sort": { + "$hybrid": { + "$vectorize" : "hello", + "$lexical" : "hello" + } + }, + "options": { + "limit" : 10, + "hybridLimits" : 10, + "includeScores": true, + "includeSortVector": false + } + } + } + """; + errorOnNotEnabled( "lexical_not_enabled", """ @@ -96,8 +147,9 @@ void failOnLexicalDisabled() { } } """, - "LEXICAL_NOT_ENABLED_FOR_COLLECTION", - "only be used on collections for which Lexical feature is enabled"); + SchemaException.Code.LEXICAL_NOT_ENABLED_FOR_COLLECTION.name(), + "The collection without a lexical index: %s.%s.", + rerank); } @Test @@ -194,27 +246,36 @@ void failOnEmptyRequest() { private void errorOnNotEnabled( String collectionName, String collectionSpec, String errorCode, String errorMessageContains) { - createCollectionWithCleanup(collectionName, collectionSpec); var rerank = """ - {"findAndRerank": { - "filter": {}, - "projection": {}, - "sort": { - "$hybrid": "hybrid sort" - }, - "options": { - "limit" : 10, - "hybridLimits" : 10, - "includeScores": true, - "includeSortVector": false - } - } - } - """; + {"findAndRerank": { + "filter": {}, + "projection": {}, + "sort": { + "$hybrid": "hybrid sort" + }, + "options": { + "limit" : 10, + "hybridLimits" : 10, + "includeScores": true, + "includeSortVector": false + } + } + } + """; + errorOnNotEnabled(collectionName, collectionSpec, errorCode, errorMessageContains, rerank); + } - givenHeadersPostJsonThen(keyspaceName, collectionName, rerank) + private void errorOnNotEnabled( + String collectionName, + String collectionSpec, + String errorCode, + String errorMessageContains, + String rerankCommand) { + createCollectionWithCleanup(collectionName, collectionSpec); + + givenHeadersPostJsonThen(keyspaceName, collectionName, rerankCommand) .body("$", responseIsError()) .body("errors[0].errorCode", is(errorCode)) .body( diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilderTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilderTest.java index ad732fc3bb..034a5827c9 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilderTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindAndRerankOperationBuilderTest.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrowsExactly; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -20,6 +21,7 @@ import io.stargate.sgv2.jsonapi.config.constants.RerankingConstants; import io.stargate.sgv2.jsonapi.exception.RequestException; import io.stargate.sgv2.jsonapi.exception.SchemaException; +import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProvider; import io.stargate.sgv2.jsonapi.service.provider.ApiModelSupport; import io.stargate.sgv2.jsonapi.service.reranking.configuration.RerankingProvidersConfig; import io.stargate.sgv2.jsonapi.service.reranking.configuration.RerankingProvidersConfigImpl; @@ -169,6 +171,61 @@ void failsWhenLexicalLimitAboveConfiguredMax() throws Exception { .hasMessageContaining("must be between 1 and 100"); } + @Test + public void failsWhenExplicitLexicalSortWhenLexicalDisabled() throws Exception { + var commandContext = commandContext(false); + var command = + command( + """ + { + "findAndRerank": { + "sort": { "$hybrid": { "$vector": [0.1, 0.2, 0.3], "$lexical": "text" } }, + "options": { + "rerankOn": "body", + "rerankQuery": "text", + "hybridLimits": { "$vector": 50, "$lexical": 10 } + } + } + } + """); + + assertThatThrownBy( + () -> + new FindAndRerankOperationBuilder(commandContext) + .withCommand(command) + .withFindCommandResolver(findCommandResolver) + .build()) + .isInstanceOf(RequestException.class) + .hasMessageContaining( + "The collection without a lexical index: %s.%s." + .formatted(TEST_CONSTANTS.KEYSPACE_NAME, TEST_CONSTANTS.COLLECTION_NAME)); + } + + @Test + public void acceptsImplicitLexicalWhenLexcialDisabled() throws Exception { + var commandContext = commandContext(false); + var command = + command( + """ + { + "findAndRerank": { + "sort": { "$hybrid": "cheese" }, + "options": { + "rerankOn": "body", + "rerankQuery": "text", + "hybridLimits": { "$vector": 50, "$lexical": 10 } + } + } + } + """); + + var operation = + new FindAndRerankOperationBuilder(commandContext) + .withCommand(command) + .withFindCommandResolver(findCommandResolver) + .build(); + } + @Test void failsWhenLimitBelowConfiguredMin() throws Exception { var commandContext = commandContext(); @@ -305,10 +362,17 @@ private FindAndRerankCommand command(String json) throws Exception { } private CommandContext commandContext() { + return commandContext(true); + } + + private CommandContext commandContext(boolean withLexical) { + + var schemaObject = + withLexical + ? TEST_CONSTANTS.VECTOR_LEXICAL_RERANK_COLLECTION_SCHEMA_OBJECT + : TEST_CONSTANTS.VECTORIZE_RERANK_COLLECTION_SCHEMA_OBJECT; var commandContext = - TEST_CONSTANTS.collectionContext( - CommandName.FIND_AND_RERANK, - TEST_CONSTANTS.VECTOR_LEXICAL_RERANK_COLLECTION_SCHEMA_OBJECT); + TEST_CONSTANTS.collectionContext(CommandName.FIND_AND_RERANK, schemaObject); var rerankingProvidersConfig = mock(RerankingProvidersConfig.class); var modelConfig = mock(RerankingProvidersConfig.RerankingProviderConfig.ModelConfig.class); @@ -322,6 +386,15 @@ private CommandContext commandContext() { when(commandContext.rerankingProviderFactory().create(any(), any(), any(), any(), any(), any())) .thenReturn(mock(RerankingProvider.class)); + if (schemaObject.vectorConfig().getFirstVectorColumnWithVectorizeDefinition().isPresent()) { + var embeddingProvider = mock(EmbeddingProvider.class); + + when(commandContext + .embeddingProviderFactory() + .create(any(), any(), any(), any(), anyInt(), any(), any(), any())) + .thenReturn(embeddingProvider); + } + return commandContext; }