From 756f6150ef1f6ed844bd6f788d60142806e92eef Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 9 Oct 2024 15:56:02 +1300 Subject: [PATCH 01/18] Use WarningException, expand CommandResultBuilder Changes the warnings associated commands to use the new ApiExcpetions, and `warnings` in the return status now returns a list of error object V2. To do that needed to improve the way CommandResult was built, so it always had a status map so the CommandProcessor could append a warning. To do that expanded the CommandResultBuilder added for the OperationAttempts, removed the many overloads used for CommandResult, and updated all creation of the CommandResult to use the builder. See also https://github.com/stargate/data-api/issues/1518 to continue this --- .../api/model/command/CommandError.java | 84 ++++++++++ .../api/model/command/CommandErrorV2.java | 147 ++++++++++++++++++ .../api/model/command/CommandResult.java | 140 +++++------------ .../model/command/CommandResultBuilder.java | 56 ++++--- .../api/model/command/DeprecatedCommand.java | 19 +-- .../api/model/command/ResponseData.java | 69 ++++++++ .../challenge/impl/ErrorChallengeSender.java | 4 +- .../APIExceptionCommandErrorBuilder.java | 48 +++++- .../jsonapi/exception/JsonApiException.java | 14 +- .../jsonapi/exception/WarningException.java | 3 +- .../ConstraintViolationExceptionMapper.java | 10 +- .../ThrowableCommandResultSupplier.java | 12 +- .../WebApplicationExceptionMapper.java | 6 +- .../service/operation/InsertAttemptPage.java | 11 +- .../operation/InsertOperationPage.java | 43 +++-- .../service/operation/OperationAttempt.java | 12 +- .../service/operation/ReadAttemptPage.java | 10 +- .../service/operation/ReadOperationPage.java | 29 ++-- .../service/operation/SchemaAttemptPage.java | 9 +- .../collections/CountOperationPage.java | 10 +- .../collections/DeleteOperationPage.java | 35 +++-- .../collections/EstimatedCountResult.java | 5 +- .../FindCollectionsCollectionOperation.java | 11 +- .../collections/SchemaChangeResult.java | 5 +- .../ServiceRegistrationResult.java | 3 +- .../UpdateCollectionOperationPage.java | 43 +++-- .../FindEmbeddingProvidersOperation.java | 7 +- .../keyspaces/FindKeyspacesOperation.java | 11 +- .../tables/DeleteTableOperation.java | 8 +- .../tables/TableReadAttemptBuilder.java | 4 +- .../tables/UpdateTableOperation.java | 10 +- .../service/processor/CommandProcessor.java | 17 +- .../processor/MeteredCommandProcessor.java | 10 +- src/main/resources/errors.yaml | 11 ++ .../CommandResultToRestResponseTest.java | 44 ++++-- .../APIExceptionCommandErrorBuilderTest.java | 55 ++++++- .../MeteredCommandProcessorTest.java | 11 +- 37 files changed, 697 insertions(+), 329 deletions(-) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java new file mode 100644 index 0000000000..5af72cadec --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java @@ -0,0 +1,84 @@ +package io.stargate.sgv2.jsonapi.api.model.command; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import jakarta.ws.rs.core.Response; +import java.util.*; +import org.eclipse.microprofile.openapi.annotations.media.Schema; + +/** + * Intended to replace the {@link CommandResult.Error} with something that has all the fields in an + * error. + * + *

This super class is the legacy V1 structure, and the subclass {@link CommandErrorV2} is the + * new V2 structure. Once we have fully removed the {@link CommandResult.Error} we can remove this + * class and just have the V2 structure. + * + *

This object is intended to be used in the CommandResult, and is serialised into the JSON + * response. + * + *

aaron - 9 -oct-2024 - This is initially only used with the Warnings in the CommandResult, + * further work needed to use it in all places. Warnings are in the status, and not described on the + * swagger for the CommandResult. This is why all the decorators from the original class are not + * here yet, but they have things to ignore flagged just incase + */ +public class CommandError { + + private final String errorCode; + private final String message; + private final Response.Status httpStatus; + private final String errorClass; + private final Map metricsTags; + + public CommandError( + String errorCode, + String message, + Response.Status httpStatus, + String errorClass, + Map metricsTags) { + + this.errorCode = requireNoNullOrBlank(errorCode, "errorCode cannot be null or blank"); + this.message = requireNoNullOrBlank(message, "message cannot be null or blank"); + this.httpStatus = Objects.requireNonNull(httpStatus, "httpStatus cannot be null"); + + this.metricsTags = metricsTags == null ? Collections.emptyMap() : Map.copyOf(metricsTags); + // errorClass is not required, it is only passed when we are in debug mode + // normalise to null if blank + this.errorClass = errorClass == null || errorClass.isBlank() ? null : errorClass; + } + + protected static String requireNoNullOrBlank(String value, String message) { + if (Objects.isNull(value) || value.isBlank()) { + throw new IllegalArgumentException(message); + } + return value; + } + + /** + * @return + */ + public String errorCode() { + return errorCode; + } + + public String message() { + return message; + } + + @JsonIgnore + public Response.Status httpStatus() { + return httpStatus; + } + + @JsonIgnore + @Schema(hidden = true) + public Map metricsTags() { + return metricsTags; + } + + @Schema(hidden = true) + @JsonInclude(JsonInclude.Include.NON_NULL) + public String exceptionClass() { + return errorClass; + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java new file mode 100644 index 0000000000..41e8b4dcfe --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java @@ -0,0 +1,147 @@ +package io.stargate.sgv2.jsonapi.api.model.command; + +import jakarta.ws.rs.core.Response; +import java.util.Map; +import java.util.Objects; +import java.util.UUID; + +/** + * See {@link CommandError} for why this class exists. + * + *

Use either {@link #builderV1()} or {@link #builderV2()} to get te builder to create an + * instance of this class. + */ +public class CommandErrorV2 extends CommandError { + + private final String family; + private final String scope; + private final String title; + private final UUID id; + + public CommandErrorV2( + String errorCode, + String message, + Response.Status httpStatus, + String errorClass, + Map metricsTags, + String family, + String scope, + String title, + UUID id) { + super(errorCode, message, httpStatus, errorClass, metricsTags); + this.family = requireNoNullOrBlank(family, "family cannot be null or blank"); + this.scope = scope == null ? "" : scope; + this.title = requireNoNullOrBlank(title, "title cannot be null or blank"); + this.id = Objects.requireNonNull(id, "id cannot be null"); + } + + /** Create a new builder for the {@link CommandError} that represents the V1 error object. */ + public static Builder builderV1() { + return new Builder<>(true); + } + + /** Create a new builder for the {@link CommandErrorV2} that represents the V2 error object. */ + public static Builder builderV2() { + return new Builder<>(false); + } + + public String family() { + return family; + } + + public String scope() { + return scope; + } + + public String title() { + return title; + } + + public UUID id() { + return id; + } + + public static class Builder { + private String errorCode; + private String message; + private Response.Status httpStatus; + private String errorClass; + private Map metricsTags; + private String family; + private String scope; + private String title; + private UUID id; + + private final boolean v1Error; + + Builder(boolean v1Error) { + this.v1Error = v1Error; + } + + public Builder errorCode(String errorCode) { + this.errorCode = errorCode; + return this; + } + + public Builder message(String message) { + this.message = message; + return this; + } + + public Builder httpStatus(Response.Status httpStatus) { + this.httpStatus = httpStatus; + return this; + } + + public Builder errorClass(String errorClass) { + this.errorClass = errorClass; + return this; + } + + public Builder metricsTags(Map metricsTags) { + this.metricsTags = metricsTags; + return this; + } + + public Builder family(String family) { + this.family = family; + return this; + } + + public Builder scope(String scope) { + this.scope = scope; + return this; + } + + public Builder title(String title) { + this.title = title; + return this; + } + + public Builder id(UUID id) { + this.id = id; + return this; + } + + public T build() { + return v1Error + ? unchecked(new CommandError(errorCode, message, httpStatus, errorClass, metricsTags)) + : unchecked( + new CommandErrorV2( + errorCode, + message, + httpStatus, + errorClass, + metricsTags, + family, + scope, + title, + id)); + } + + @SuppressWarnings("unchecked") + private T unchecked(CommandError error) { + return (T) error; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java index c1ead5541b..b910644afc 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java @@ -3,9 +3,7 @@ import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.databind.JsonNode; import io.stargate.sgv2.jsonapi.exception.ErrorCodeV1; -import jakarta.validation.constraints.NotNull; import jakarta.ws.rs.core.Response; import java.util.*; import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; @@ -29,7 +27,7 @@ public record CommandResult( description = "A response data holding documents that were returned as the result of a command.", nullable = true, - oneOf = {CommandResult.MultiResponseData.class, CommandResult.SingleResponseData.class}) + oneOf = {ResponseData.MultiResponseData.class, ResponseData.SingleResponseData.class}) ResponseData data, @Schema( description = @@ -44,105 +42,51 @@ public record CommandResult( implementation = String.class, nullable = true) }) + @JsonInclude(JsonInclude.Include.NON_EMPTY) Map status, @JsonInclude(JsonInclude.Include.NON_EMPTY) @Schema(nullable = true) List errors) { - /** - * Constructor for only specifying the {@link MultiResponseData}. - * - * @param responseData {@link MultiResponseData} - */ - public CommandResult(ResponseData responseData) { - this(responseData, null, null); - } - - /** - * Constructor for specifying the {@link MultiResponseData} and statuses. - * - * @param responseData {@link MultiResponseData} - * @param status Map of status information. - */ - public CommandResult(ResponseData responseData, Map status) { - this(responseData, status, null); - } + public CommandResult { - /** - * Constructor for only specifying the status. - * - * @param status Map of status information. - */ - public CommandResult(Map status) { - this(null, status, null); + // make both of these not null, so they can be mutated if needed + // the decorators on the record fields tell Jackson to exclude when they are null or empty + if (null == status) { + status = new HashMap<>(); + } + if (null == errors) { + errors = new ArrayList<>(); + } } /** - * Constructor for only specifying the errors. + * Get a builder for the {@link CommandResult} for a single document response, see {@link + * CommandResultBuilder} * - * @param errors List of errors. + *

NOTE: aaron 9-oct-2024 I kept the errorObjectV2 and debugMode params to make it clear + * how inconsistency we are configuring these settings. Ultimately useErrorObjectV2 will go away, + * but we will still have the debugMode setting. I will create ticket so that we create the + * builder in resolver or similar and then pass it around rather than creating in many places. + * Also the {@link io.stargate.sgv2.jsonapi.service.operation.OperationAttemptPageBuilder} is how + * things will turn out. */ - public CommandResult(List errors) { - this(null, null, errors); + public static CommandResultBuilder singleDocumentBuilder( + boolean useErrorObjectV2, boolean debugMode) { + return new CommandResultBuilder( + CommandResultBuilder.ResponseType.SINGLE_DOCUMENT, useErrorObjectV2, debugMode); } - public interface ResponseData { - - /** - * @return Simple shared method to get the response documents. Usually used only in tests, - * ignored in JSON response. - */ - @JsonIgnore - List getResponseDocuments(); - } - - /** - * Response data object that's included in the {@link CommandResult}, for a single document - * responses. - * - * @param document Document. - */ - @Schema(description = "Response data for a single document commands.") - public record SingleResponseData( - @NotNull - @Schema( - description = "Document that resulted from a command.", - type = SchemaType.OBJECT, - implementation = Object.class, - nullable = true) - JsonNode document) - implements ResponseData { - - /** {@inheritDoc} */ - @Override - public List getResponseDocuments() { - return List.of(document); - } + /** See {@link #singleDocumentBuilder(boolean, boolean)} */ + public static CommandResultBuilder multiDocumentBuilder( + boolean useErrorObjectV2, boolean debugMode) { + return new CommandResultBuilder( + CommandResultBuilder.ResponseType.MULTI_DOCUMENT, useErrorObjectV2, debugMode); } - /** - * Response data object that's included in the {@link CommandResult}, for multi document - * responses. - * - * @param documents Documents. - * @param nextPageState Optional next page state. - */ - @Schema(description = "Response data for multiple documents commands.") - public record MultiResponseData( - @NotNull - @Schema( - description = "Documents that resulted from a command.", - type = SchemaType.ARRAY, - implementation = Object.class, - minItems = 0) - List documents, - @Schema(description = "Next page state for pagination.", nullable = true) - String nextPageState) - implements ResponseData { - - /** {@inheritDoc} */ - @Override - public List getResponseDocuments() { - return documents; - } + /** See {@link #singleDocumentBuilder(boolean, boolean)} */ + public static CommandResultBuilder statusOnlyBuilder( + boolean useErrorObjectV2, boolean debugMode) { + return new CommandResultBuilder( + CommandResultBuilder.ResponseType.STATUS_ONLY, useErrorObjectV2, debugMode); } /** @@ -202,19 +146,11 @@ public RestResponse toRestResponse() { * returned a new CommandResult with warning message added in status map * * @param warning message - * @return CommandResult */ - public CommandResult withWarning(String warning) { - Map newStatus = new HashMap<>(); - if (status != null) { - newStatus.putAll(status); - } - List newWarnings = - newStatus.get(CommandStatus.WARNINGS) != null - ? new ArrayList<>((List) newStatus.get(CommandStatus.WARNINGS)) - : new ArrayList<>(); - newWarnings.add(warning); - newStatus.put(CommandStatus.WARNINGS, newWarnings); - return new CommandResult(this.data, newStatus, errors); + public void addWarning(CommandErrorV2 warning) { + List warnings = + (List) + status.computeIfAbsent(CommandStatus.WARNINGS, k -> new ArrayList<>()); + warnings.add(warning); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java index f06fc3555b..f37ba7cf23 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java @@ -4,12 +4,18 @@ import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.exception.APIExceptionCommandErrorBuilder; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableToErrorMapper; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.function.Function; - +import java.util.*; + +/** + * A basic builder pattern for creating a {@link CommandResult} object, so we create it easily and + * more reliably. + * + *

See https://github.com/stargate/data-api/issues/1518 for future changes. + * + *

This is trying to codify the different ways the CommandResult was created, and it still needs + * to handle the migration from the old error object to the new error object. It should be improved + * in the ticket above, and once we finish the migration, we can remove the old error object. + */ public class CommandResultBuilder { public enum ResponseType { @@ -21,7 +27,8 @@ public enum ResponseType { private final Map cmdStatus = new HashMap<>(); private final List cmdErrors = new ArrayList<>(); private final List documents = new ArrayList<>(); - private final List warnings = new ArrayList<>(); + // Warnings are always the V2 error structure + private final List warnings = new ArrayList<>(); private String nextPageState = null; @@ -34,10 +41,9 @@ public enum ResponseType { private final boolean useErrorObjectV2; // Created in the Ctor - private final Function apiExceptionToError; + private final APIExceptionCommandErrorBuilder apiExceptionToError; - public CommandResultBuilder( - ResponseType responseType, boolean useErrorObjectV2, boolean debugMode) { + CommandResultBuilder(ResponseType responseType, boolean useErrorObjectV2, boolean debugMode) { this.responseType = responseType; this.useErrorObjectV2 = useErrorObjectV2; this.debugMode = debugMode; @@ -50,12 +56,22 @@ public CommandResultBuilder addStatus(CommandStatus status, Object value) { return this; } - public CommandResultBuilder addThrowable(Throwable thorwable) { - var error = throwableToCommandError(thorwable); - return addCommandError(error); + public CommandResultBuilder addThrowable(List throwables) { + Objects.requireNonNull(throwables, "throwables cannot be null").forEach(this::addThrowable); + return this; + } + + public CommandResultBuilder addThrowable(Throwable throwable) { + var error = throwableToCommandError(throwable); + return addCommandResultError(error); + } + + public CommandResultBuilder addCommandResultError(List errors) { + Objects.requireNonNull(errors, "errors cannot be null").forEach(this::addCommandResultError); + return this; } - public CommandResultBuilder addCommandError(CommandResult.Error error) { + public CommandResultBuilder addCommandResultError(CommandResult.Error error) { cmdErrors.add(error); return this; } @@ -70,8 +86,10 @@ public CommandResultBuilder addDocuments(List documents) { return this; } - public CommandResultBuilder addWarning(String warning) { - warnings.add(warning); + public CommandResultBuilder addWarning(APIException warning) { + warnings.add( + apiExceptionToError.buildCommandErrorV2( + Objects.requireNonNull(warning, "warning cannot be null"))); return this; } @@ -89,7 +107,7 @@ public CommandResult.Error throwableToCommandError(Throwable throwable) { // new v2 error object, with family etc. // the builder will handle the debug mode and extended errors settings to return a V1 or V2 // error - return apiExceptionToError.apply(apiException); + return apiExceptionToError.buildLegacyCommandResultError(apiException); } // the mapper handles error object v2 part @@ -132,8 +150,8 @@ public CommandResult build() { case SINGLE_DOCUMENT -> documents.isEmpty() ? null - : new CommandResult.SingleResponseData(documents.getFirst()); - case MULTI_DOCUMENT -> new CommandResult.MultiResponseData(documents, nextPageState); + : new ResponseData.SingleResponseData(documents.getFirst()); + case MULTI_DOCUMENT -> new ResponseData.MultiResponseData(documents, nextPageState); case STATUS_ONLY -> null; }; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java index 01db3ceec5..c5121f3d0d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java @@ -1,6 +1,7 @@ package io.stargate.sgv2.jsonapi.api.model.command; -import io.stargate.sgv2.jsonapi.api.model.command.impl.*; +import io.stargate.sgv2.jsonapi.exception.APIException; +import io.stargate.sgv2.jsonapi.exception.WarningException; /** * All deprecated commands will implement this interface. It has default deprecation message @@ -16,16 +17,10 @@ public interface DeprecatedCommand extends Command { */ Command.CommandName useCommandName(); - /** - * A warning message will be added to commandResult when deprecated command is used. Template: - * This ${commandName} has been deprecated and will be removed in future releases, use - * ${useCommandName} instead. - * - * @return String - */ - default String getDeprecationMessage() { - return String.format( - "This %s has been deprecated and will be removed in future releases, use %s instead.", - this.commandName().getApiName(), useCommandName().getApiName()); + /** A warning message will be added to commandResult when deprecated command is used. */ + default APIException getDeprecationWarning() { + return WarningException.Code.DEPRECATED_COMMAND.get( + "deprecatedCommand", this.commandName().getApiName(), + "replacementCommand", useCommandName().getApiName()); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java new file mode 100644 index 0000000000..9c4ad07091 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java @@ -0,0 +1,69 @@ +package io.stargate.sgv2.jsonapi.api.model.command; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.databind.JsonNode; +import jakarta.validation.constraints.NotNull; +import java.util.List; +import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; +import org.eclipse.microprofile.openapi.annotations.media.Schema; + +public interface ResponseData { + + /** + * @return Simple shared method to get the response documents. Usually used only in tests, ignored + * in JSON response. + */ + @JsonIgnore + List getResponseDocuments(); + + /** + * Response data object that's included in the {@link CommandResult}, for a single document + * responses. + * + * @param document Document. + */ + @Schema(description = "Response data for a single document commands.") + record SingleResponseData( + @NotNull + @Schema( + description = "Document that resulted from a command.", + type = SchemaType.OBJECT, + implementation = Object.class, + nullable = true) + JsonNode document) + implements ResponseData { + + /** {@inheritDoc} */ + @Override + public List getResponseDocuments() { + return List.of(document); + } + } + + /** + * Response data object that's included in the {@link CommandResult}, for multi document + * responses. + * + * @param documents Documents. + * @param nextPageState Optional next page state. + */ + @Schema(description = "Response data for multiple documents commands.") + record MultiResponseData( + @NotNull + @Schema( + description = "Documents that resulted from a command.", + type = SchemaType.ARRAY, + implementation = Object.class, + minItems = 0) + List documents, + @Schema(description = "Next page state for pagination.", nullable = true) + String nextPageState) + implements ResponseData { + + /** {@inheritDoc} */ + @Override + public List getResponseDocuments() { + return documents; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java b/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java index c76abd7dc8..1a165b66c5 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java @@ -13,7 +13,6 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import java.util.Collections; -import java.util.List; import java.util.function.BiFunction; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,7 +44,8 @@ public ErrorChallengeSender(ObjectMapper objectMapper) { CommandResult.Error error = new CommandResult.Error( message, Collections.emptyMap(), Collections.emptyMap(), Response.Status.UNAUTHORIZED); - commandResult = new CommandResult(List.of(error)); + commandResult = + CommandResult.statusOnlyBuilder(false, false).addCommandResultError(error).build(); } /** {@inheritDoc} */ diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java index 91d2a20054..3fdf5026cc 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java @@ -1,20 +1,21 @@ package io.stargate.sgv2.jsonapi.exception; +import io.stargate.sgv2.jsonapi.api.model.command.CommandErrorV2; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.config.constants.ErrorObjectV2Constants; import jakarta.ws.rs.core.Response; import java.util.HashMap; import java.util.Map; -import java.util.function.Function; /** * Builder that creates a {@link CommandResult.Error} from an {@link APIException}. * *

This class encapsulates the mapping between the APIException and the API tier to keep it out - * of the core exception classes. + * of the core exception classes. NOTE: aaron 9-oct-2024 needed to tweak this class to work + * with the new CommandErrorV2, once we have rolled out the use of CommandErrorV2 everywhere we can + * remove the legacy CommandResult.Error */ -public class APIExceptionCommandErrorBuilder - implements Function { +public class APIExceptionCommandErrorBuilder { private final boolean debugEnabled; private final boolean returnErrorObjectV2; @@ -40,8 +41,7 @@ public APIExceptionCommandErrorBuilder(boolean debugEnabled, boolean returnError * @param apiException the exception that is going to be returned. * @return a {@link CommandResult.Error} that represents the apiException. */ - @Override - public CommandResult.Error apply(APIException apiException) { + public CommandResult.Error buildLegacyCommandResultError(APIException apiException) { // Note, in the old JsonApiException the code also traverses the cause, we do not want to do // that in // error objects V2 because the proper error is created by the template etc. @@ -75,6 +75,42 @@ public CommandResult.Error apply(APIException apiException) { Response.Status.fromStatusCode(apiException.httpStatus)); } + /** + * Create a new instance that will create a {@link CommandErrorV2} that represents the + * apiException. + * + * @param apiException the exception that is going to be returned. + * @return a {@link CommandErrorV2} that represents the apiException. + */ + public CommandErrorV2 buildCommandErrorV2(APIException apiException) { + if (!returnErrorObjectV2) { + // aaron - oct 9 - I know this seems silly, we are in the process on moving all the errors to + // the V2 + // i added this function to be used with WARNING errors, once we have rolled out the use of + // CommandErrorV2 + // everywhere we wont need this test and there will be one build function + throw new IllegalStateException( + "Cannot build a V2 error object when returnErrorObjectV2 is false"); + } + + var builder = CommandErrorV2.builderV2(); + + if (debugEnabled) { + builder.errorClass(apiException.getClass().getSimpleName()); + } + + return builder + .errorCode(apiException.code) + .message(apiException.body) + .httpStatus(Response.Status.fromStatusCode(apiException.httpStatus)) + .metricsTags(tagsForMetrics(apiException)) + .family(apiException.family.name()) + .scope(apiException.scope) + .title(apiException.title) + .id(apiException.errorId) + .build(); + } + private Map tagsForMetrics(APIException apiException) { // These tags must be backwards compatible with how we tracked before return Map.of( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java index 86d3b06b3e..9f273e6edc 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java @@ -134,17 +134,17 @@ public CommandResult get() { if (message == null) { message = errorCode.getMessage(); } - // construct and return - CommandResult.Error error = getCommandResultError(message, httpStatus); + var builder = CommandResult.statusOnlyBuilder(false, false); + + // construct and return + builder.addCommandResultError(getCommandResultError(message, httpStatus)); // handle cause as well Throwable cause = getCause(); - if (null == cause) { - return new CommandResult(List.of(error)); - } else { - CommandResult.Error causeError = ThrowableToErrorMapper.getMapperFunction().apply(cause); - return new CommandResult(List.of(error, causeError)); + if (null != cause) { + builder.addCommandResultError(ThrowableToErrorMapper.getMapperFunction().apply(cause)); } + return builder.build(); } public CommandResult.Error getCommandResultError(String message, Response.Status status) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java index 7daeb2b687..e19419bff9 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java @@ -17,7 +17,8 @@ public enum Code implements ErrorCode { MISSING_INDEX, NOT_EQUALS_UNSUPPORTED_BY_INDEXING, ZERO_FILTER_OPERATIONS, - INCOMPLETE_PRIMARY_KEY_FILTER; + INCOMPLETE_PRIMARY_KEY_FILTER, + DEPRECATED_COMMAND; private final ErrorTemplate template; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java index 2ee65ca570..29ca731beb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java @@ -7,7 +7,6 @@ import jakarta.validation.ConstraintViolation; import jakarta.validation.ConstraintViolationException; import jakarta.ws.rs.core.Response; -import java.util.List; import java.util.Map; import java.util.Set; import org.jboss.resteasy.reactive.RestResponse; @@ -35,11 +34,14 @@ public RestResponse constraintViolationException( ConstraintViolationException exception) { // map all violations to errors Set> violations = exception.getConstraintViolations(); - List errors = - violations.stream().map(ConstraintViolationExceptionMapper::getError).distinct().toList(); + var builder = CommandResult.statusOnlyBuilder(false, false); + violations.stream() + .map(ConstraintViolationExceptionMapper::getError) + .distinct() + .forEach(builder::addCommandResultError); // return result - return new CommandResult(errors).toRestResponse(); + return builder.build().toRestResponse(); } private static CommandResult.Error getError(ConstraintViolation violation) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java index c7d569b83e..090319f126 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java @@ -1,7 +1,6 @@ package io.stargate.sgv2.jsonapi.exception.mappers; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; -import java.util.List; import java.util.function.Supplier; /** @@ -14,13 +13,14 @@ public record ThrowableCommandResultSupplier(Throwable t) implements Supplier webApplicationExceptionMapper(WebApplicationE var errorBuilder = new APIExceptionCommandErrorBuilder( debugModeConfig.enabled(), operationsConfig.extendError()); - return RestResponse.ok(new CommandResult(List.of(errorBuilder.apply(ae)))); + return RestResponse.ok( + CommandResult.statusOnlyBuilder(false, false) + .addCommandResultError(errorBuilder.buildLegacyCommandResultError(ae)) + .build()); } CommandResult commandResult = new ThrowableCommandResultSupplier(toReport).get(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java index e68d9009a0..8c4094853f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java @@ -123,7 +123,7 @@ private void buildPerDocumentResult() { new InsertionResult(attempt.docRowID().orElseThrow(), InsertionStatus.SKIPPED, null); } - seenErrors.forEach(resultBuilder::addCommandError); + seenErrors.forEach(resultBuilder::addCommandResultError); resultBuilder.addStatus(CommandStatus.DOCUMENT_RESPONSES, Arrays.asList(results)); maybeAddSchema(); } @@ -192,11 +192,10 @@ public Builder returnDocumentResponses(boolean returnDocumentResponses) @Override public InsertAttemptPage getOperationPage() { - var resultBuilder = - new CommandResultBuilder( - CommandResultBuilder.ResponseType.STATUS_ONLY, useErrorObjectV2, debugMode); - - return new InsertAttemptPage<>(attempts, resultBuilder, returnDocumentResponses); + return new InsertAttemptPage<>( + attempts, + CommandResult.statusOnlyBuilder(useErrorObjectV2, debugMode), + returnDocumentResponses); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java index 47d25f0ec2..8524dd6ebf 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonPropertyOrder; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResultBuilder; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.exception.APIExceptionCommandErrorBuilder; @@ -11,7 +12,6 @@ import io.stargate.sgv2.jsonapi.service.shredding.DocRowIdentifer; import java.util.*; import java.util.function.BiConsumer; -import java.util.function.Function; import java.util.function.Supplier; /** @@ -45,7 +45,7 @@ public class InsertOperationPage private final boolean useErrorObjectV2; // Created in the Ctor - private final Function apiExceptionToError; + private final APIExceptionCommandErrorBuilder apiExceptionToError; /** Create an instance that has debug false and useErrorIbhectV2 false */ public InsertOperationPage( @@ -102,11 +102,8 @@ public CommandResult get() { // TODO AARON used to only sort the success list when not returning detailed responses, check OK Collections.sort(successfulInsertions); - if (!returnDocumentResponses) { - return nonPerDocumentResult(); - } - - return perDocumentResult(); + var builder = CommandResult.statusOnlyBuilder(useErrorObjectV2, debugMode); + return returnDocumentResponses ? perDocumentResult(builder) : nonPerDocumentResult(builder); } /** @@ -116,7 +113,7 @@ public CommandResult get() { * * @return Command result */ - private CommandResult perDocumentResult() { + private CommandResult perDocumentResult(CommandResultBuilder builder) { // New style output: detailed responses. InsertionResult[] results = new InsertionResult[allInsertions.size()]; List errors = new ArrayList<>(); @@ -126,6 +123,7 @@ private CommandResult perDocumentResult() { results[okInsertion.position()] = new InsertionResult(okInsertion.docRowID().orElseThrow(), InsertionStatus.OK, null); } + // Second: failed insertions; output in order of insertion for (var failedInsertion : failedInsertions) { // TODO AARON - confirm the null handling in the getError @@ -152,11 +150,10 @@ private CommandResult perDocumentResult() { allInsertions.get(i).docRowID().orElseThrow(), InsertionStatus.SKIPPED, null); } } - Map status = new HashMap<>(); - status.put(CommandStatus.DOCUMENT_RESPONSES, Arrays.asList(results)); - maybeAddSchema(status); + builder.addStatus(CommandStatus.DOCUMENT_RESPONSES, Arrays.asList(results)); + maybeAddSchema(builder); - return new CommandResult(null, status, errors); + return builder.build(); } /** @@ -166,12 +163,9 @@ private CommandResult perDocumentResult() { * * @return Command result */ - private CommandResult nonPerDocumentResult() { + private CommandResult nonPerDocumentResult(CommandResultBuilder builder) { - List errors = - failedInsertions.isEmpty() - ? null - : failedInsertions.stream().map(this::getErrorObject).toList(); + failedInsertions.stream().map(this::getErrorObject).forEach(builder::addCommandResultError); // Note: See DocRowIdentifer, it has an attribute that will be called for JSON serialization List insertedIds = @@ -180,11 +174,10 @@ private CommandResult nonPerDocumentResult() { .map(Optional::orElseThrow) .toList(); - Map status = new HashMap<>(); - status.put(CommandStatus.INSERTED_IDS, insertedIds); - maybeAddSchema(status); + builder.addStatus(CommandStatus.INSERTED_IDS, insertedIds); + maybeAddSchema(builder); - return new CommandResult(null, status, errors); + return builder.build(); } /** @@ -194,16 +187,16 @@ private CommandResult nonPerDocumentResult() { *

Uses the first, not the first successful, because we may fail to do an insert but will still * have the _id or PK to report. * - * @param status Map to add the status to + * @param builder CommandResult builder to add the status to */ - private void maybeAddSchema(Map status) { + private void maybeAddSchema(CommandResultBuilder builder) { if (allInsertions.isEmpty()) { return; } allInsertions .getFirst() .schemaDescription() - .ifPresent(o -> status.put(CommandStatus.PRIMARY_KEY_SCHEMA, o)); + .ifPresent(o -> builder.addStatus(CommandStatus.PRIMARY_KEY_SCHEMA, o)); } /** @@ -216,7 +209,7 @@ private CommandResult.Error getErrorObject(InsertAttempt insertAttempt) // new v2 error object, with family etc. // the builder will handle the debug mode and extended errors settings to return a V1 or V2 // error - return apiExceptionToError.apply(apiException); + return apiExceptionToError.buildLegacyCommandResultError(apiException); } if (useErrorObjectV2) { return getErrorObjectV2(throwable); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java index 1bbd8834e4..68a1a01b5a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java @@ -2,6 +2,7 @@ import com.datastax.oss.driver.api.core.cql.AsyncResultSet; import io.smallrye.mutiny.Uni; +import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CommandQueryExecutor; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.DriverExceptionHandler; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject; @@ -68,7 +69,7 @@ public final boolean isTerminal() { protected final UUID attemptId = UUID.randomUUID(); - private final List warnings = new ArrayList<>(); + private final List warnings = new ArrayList<>(); private Throwable failure; // Keep this private, so sub-classes set through setter incase we need to syncronize later @@ -447,11 +448,8 @@ public SubT maybeAddFailure(Throwable throwable) { * * @param warning The warning message to add, cannot be null or blank. */ - public void addWarning(String warning) { - if (warning == null || warning.isBlank()) { - throw new IllegalArgumentException("warning cannot be null or blank"); - } - warnings.add(warning); + public void addWarning(APIException warning) { + warnings.add(Objects.requireNonNull(warning, "warning cannot be null")); } /** @@ -462,7 +460,7 @@ public void addWarning(String warning) { * * @return An unmodifiable list of warnings, never null */ - public List warnings() { + public List warnings() { return List.copyOf(warnings); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java index cba4b3ee88..c24293849c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java @@ -1,5 +1,6 @@ package io.stargate.sgv2.jsonapi.service.operation; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandResultBuilder; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CqlPagingState; @@ -93,12 +94,9 @@ public ReadAttemptPage getOperationPage() { }; var resultBuilder = - new CommandResultBuilder( - singleResponse - ? CommandResultBuilder.ResponseType.SINGLE_DOCUMENT - : CommandResultBuilder.ResponseType.MULTI_DOCUMENT, - useErrorObjectV2, - debugMode); + singleResponse + ? CommandResult.singleDocumentBuilder(useErrorObjectV2, debugMode) + : CommandResult.multiDocumentBuilder(useErrorObjectV2, debugMode); return new ReadAttemptPage<>( attempts, resultBuilder, pagingState, includeSortVector, sortVector); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java index 8315c8143f..e910d96410 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java @@ -1,11 +1,8 @@ package io.stargate.sgv2.jsonapi.service.operation; -import com.fasterxml.jackson.databind.JsonNode; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.function.Supplier; /** @@ -33,20 +30,22 @@ public record ReadOperationPage( @Override public CommandResult get() { - Map status = - includeSortVector ? Collections.singletonMap(CommandStatus.SORT_VECTOR, vector) : null; + var builder = + singleResponse + ? CommandResult.singleDocumentBuilder(false, false) + : CommandResult.multiDocumentBuilder(false, false); - List jsonDocs = - documentSources.stream() - .limit(singleResponse ? 1 : Long.MAX_VALUE) - .map(DocumentSource::get) - .toList(); + if (includeSortVector) { + builder.addStatus(CommandStatus.SORT_VECTOR, vector); + } else { + builder.nextPageState(nextPageState); + } - var responseData = - singleResponse - ? new CommandResult.SingleResponseData(jsonDocs.isEmpty() ? null : jsonDocs.get(0)) - : new CommandResult.MultiResponseData(jsonDocs, nextPageState); + documentSources.stream() + .limit(singleResponse ? 1 : Long.MAX_VALUE) + .map(DocumentSource::get) + .forEach(builder::addDocument); - return new CommandResult(responseData, status); + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java index 24890af923..66793631c9 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java @@ -1,5 +1,6 @@ package io.stargate.sgv2.jsonapi.service.operation; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandResultBuilder; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject; @@ -35,12 +36,8 @@ public static class Builder @Override public SchemaAttemptPage getOperationPage() { - - var resultBuilder = - new CommandResultBuilder( - CommandResultBuilder.ResponseType.STATUS_ONLY, useErrorObjectV2, debugMode); - - return new SchemaAttemptPage<>(attempts, resultBuilder); + return new SchemaAttemptPage<>( + attempts, CommandResult.statusOnlyBuilder(useErrorObjectV2, debugMode)); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java index e261960b55..1b565a3ce2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java @@ -2,16 +2,18 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record CountOperationPage(long count, boolean moreData) implements Supplier { @Override public CommandResult get() { + + var builder = + CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.COUNTED_DOCUMENT, count); if (moreData) { - return new CommandResult( - Map.of(CommandStatus.COUNTED_DOCUMENT, count(), CommandStatus.MORE_DATA, true)); + builder.addStatus(CommandStatus.MORE_DATA, true); } - return new CommandResult(Map.of(CommandStatus.COUNTED_DOCUMENT, count())); + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java index 0f9b32d25e..0a9bcb3c9f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java @@ -11,7 +11,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -32,9 +31,16 @@ public record DeleteOperationPage( @Override public CommandResult get() { + // aaron - 9 octo 2024 - this class had multiple return statements, which is ok but when I + // changed to + // use the CommandResultBuilder, I left the structure as it was to reduce the amount of changes. + // when we move to use OperationAttempt for the collection commands we can refactor if (deletedInformation == null) { - return new CommandResult(null, Map.of(CommandStatus.DELETED_COUNT, -1), null); + return CommandResult.singleDocumentBuilder(false, false) + .addStatus(CommandStatus.DELETED_COUNT, -1) + .build(); } + int deletedCount = (int) deletedInformation.stream() @@ -76,15 +82,20 @@ public CommandResult get() { // return the result // note that we always target a single document to be returned // thus fixed to the SingleResponseData - if (moreData) - return new CommandResult( - deletedDoc.isEmpty() ? null : new CommandResult.SingleResponseData(deletedDoc.get(0)), - Map.of(CommandStatus.DELETED_COUNT, deletedCount, CommandStatus.MORE_DATA, true), - errors.isEmpty() ? null : errors); - else - return new CommandResult( - deletedDoc.isEmpty() ? null : new CommandResult.SingleResponseData(deletedDoc.get(0)), - Map.of(CommandStatus.DELETED_COUNT, deletedCount), - errors.isEmpty() ? null : errors); + + // aaron 9-oct-2024 the original code had this to create the "ResponseData"for the command + // result + // which looks like it would be statusOnly if there were no docs, otherwise singleDoc + // deletedDoc.isEmpty() ? null : new ResponseData.SingleResponseData(deletedDoc.get(0)), + var builder = + deletedDoc.isEmpty() + ? CommandResult.statusOnlyBuilder(false, false) + : CommandResult.singleDocumentBuilder(false, false).addDocument(deletedDoc.getFirst()); + + builder.addStatus(CommandStatus.DELETED_COUNT, deletedCount).addCommandResultError(errors); + if (moreData) { + builder.addStatus(CommandStatus.MORE_DATA, true); + } + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java index ab97b04f1f..13456eb323 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java @@ -2,12 +2,13 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record EstimatedCountResult(long count) implements Supplier { @Override public CommandResult get() { - return new CommandResult(Map.of(CommandStatus.COUNTED_DOCUMENT, count())); + return CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.COUNTED_DOCUMENT, count) + .build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java index bf497d8f98..c6feca7620 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java @@ -17,7 +17,6 @@ import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionTableMatcher; import java.util.List; -import java.util.Map; import java.util.function.Supplier; /** @@ -96,20 +95,20 @@ private record Result(boolean explain, List collections) @Override public CommandResult get() { + + var builder = CommandResult.statusOnlyBuilder(false, false); if (explain) { final List createCollectionCommands = collections.stream() .map(CollectionSchemaObject::collectionSettingToCreateCollectionCommand) .toList(); - Map statuses = - Map.of(CommandStatus.EXISTING_COLLECTIONS, createCollectionCommands); - return new CommandResult(statuses); + builder.addStatus(CommandStatus.EXISTING_COLLECTIONS, createCollectionCommands); } else { List tables = collections.stream().map(schemaObject -> schemaObject.name().table()).toList(); - Map statuses = Map.of(CommandStatus.EXISTING_COLLECTIONS, tables); - return new CommandResult(statuses); + builder.addStatus(CommandStatus.EXISTING_COLLECTIONS, tables); } + return builder.build(); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java index 740a280c9a..b71f2a0798 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java @@ -2,12 +2,13 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record SchemaChangeResult(boolean schemaChanged) implements Supplier { @Override public CommandResult get() { - return new CommandResult(Map.of(CommandStatus.OK, schemaChanged ? 1 : 0)); + return CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.OK, schemaChanged ? 1 : 0) + .build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java index df5b1c798f..70e0517e76 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java @@ -2,12 +2,11 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record ServiceRegistrationResult() implements Supplier { @Override public CommandResult get() { - return new CommandResult(Map.of(CommandStatus.OK, 1)); + return CommandResult.singleDocumentBuilder(false, false).addStatus(CommandStatus.OK, 1).build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java index 4041ac1d26..63cce04c54 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java @@ -9,7 +9,6 @@ import io.stargate.sgv2.jsonapi.util.ExceptionUtil; import java.util.ArrayList; import java.util.Collection; -import java.util.EnumMap; import java.util.List; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -29,13 +28,22 @@ public CommandResult get() { final DocumentId[] upsertedId = new DocumentId[1]; List updatedDocs = new ArrayList<>(updatedDocuments().size()); + var builder = + returnDocs + ? CommandResult.singleDocumentBuilder(false, false) + : CommandResult.multiDocumentBuilder(false, false); + // aggregate the errors by error code or error class Multimap groupedErrorUpdates = ArrayListMultimap.create(); updatedDocuments.forEach( update -> { - if (update.upserted()) upsertedId[0] = update.id(); - if (returnDocs) updatedDocs.add(update.document()); + if (update.upserted()) { + upsertedId[0] = update.id(); + } + if (returnDocs) { + updatedDocs.add(update.document()); + } // if (update.error() != null) { String key = ExceptionUtil.getThrowableGroupingKey(update.error()); @@ -56,28 +64,29 @@ public CommandResult get() { ExceptionUtil.getError( ERROR, documentIds, updatedDocuments.stream().findFirst().get().error())); }); - EnumMap updateStatus = new EnumMap<>(CommandStatus.class); + if (upsertedId[0] != null) { - updateStatus.put(CommandStatus.UPSERTED_ID, upsertedId[0]); + builder.addStatus(CommandStatus.UPSERTED_ID, upsertedId[0]); } - updateStatus.put(CommandStatus.MATCHED_COUNT, matchedCount()); - updateStatus.put(CommandStatus.MODIFIED_COUNT, modifiedCount()); + builder.addStatus(CommandStatus.MATCHED_COUNT, matchedCount()); + builder.addStatus(CommandStatus.MODIFIED_COUNT, modifiedCount()); if (pagingState != null) { - updateStatus.put(CommandStatus.MORE_DATA, true); - updateStatus.put(CommandStatus.PAGE_STATE, pagingState); + builder.addStatus(CommandStatus.MORE_DATA, true); + builder.addStatus(CommandStatus.PAGE_STATE, pagingState); } + // aaron - 9-oct-2024 - these two line comments were below.... // note that we always target a single document to be returned // thus fixed to the SingleResponseData - if (returnDocs) { - JsonNode node = updatedDocs.size() > 0 ? updatedDocs.get(0) : null; - return new CommandResult( - new CommandResult.SingleResponseData(node), - updateStatus, - errors.isEmpty() ? null : errors); - } else { - return new CommandResult(null, updateStatus, errors.isEmpty() ? null : errors); + // ... but I think they are wrong, because it would previously pass null for the data if + // returnedDocs was false + // so at the top of the function we make the appropriate builder + // (comment could have been about how it handles have zero docs and returnDocs is true) + if (returnDocs && !updatedDocs.isEmpty()) { + builder.addDocument(updatedDocs.getFirst()); } + builder.addCommandResultError(errors); + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java index 7abbf225f9..2bdb65eeac 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java @@ -40,9 +40,10 @@ private record Result(Map embeddingProviders) @Override public CommandResult get() { - Map statuses = - Map.of(CommandStatus.EXISTING_VECTOR_PROVIDERS, embeddingProviders); - return new CommandResult(statuses); + + return CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.EXISTING_VECTOR_PROVIDERS, embeddingProviders) + .build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java index 17acbc055a..3cf349e57b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java @@ -8,7 +8,6 @@ import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor; import io.stargate.sgv2.jsonapi.service.operation.Operation; import java.util.List; -import java.util.Map; import java.util.function.Supplier; /** @@ -65,11 +64,11 @@ private record Result(List keyspaces, boolean useKeyspaceNaming) @Override public CommandResult get() { - Map statuses = - useKeyspaceNaming - ? Map.of(CommandStatus.EXISTING_KEYSPACES, keyspaces) - : Map.of(CommandStatus.EXISTING_NAMESPACES, keyspaces); - return new CommandResult(statuses); + + var statusKey = + useKeyspaceNaming ? CommandStatus.EXISTING_KEYSPACES : CommandStatus.EXISTING_NAMESPACES; + + return CommandResult.statusOnlyBuilder(false, false).addStatus(statusKey, keyspaces).build(); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java index 4784a315f9..a7ca94a326 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java @@ -14,7 +14,6 @@ import io.stargate.sgv2.jsonapi.service.operation.query.WhereCQLClause; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.function.Supplier; import org.slf4j.Logger; @@ -67,12 +66,13 @@ public Uni> execute( resultSet -> Uni.createFrom() .item( - new Supplier>() { + new Supplier<>() { @Override public Supplier get() { return () -> - new CommandResult( - null, Map.of(CommandStatus.DELETED_COUNT, -1), List.of()); + CommandResult.singleDocumentBuilder(false, false) + .addStatus(CommandStatus.DELETED_COUNT, -1) + .build(); } })); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java index 6b9d065c05..48e323eebc 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java @@ -111,9 +111,7 @@ public ReadAttempt build(WhereCQLClause whereCQLClause; + private final OrderByCqlClause orderByCqlClause; private final CqlOptions whereCQLClause, + OrderByCqlClause orderByCqlClause, CqlOptions whereCQLClaus tableSchemaObject, selectCQLClause, whereCQLClause, + orderByCqlClause, atttemptCqlOptions, pagingState, documentSourceSupplier); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java index 1c75f57496..7ddbb6d0a4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java @@ -15,6 +15,7 @@ import io.stargate.sgv2.jsonapi.service.operation.*; import io.stargate.sgv2.jsonapi.service.operation.collections.CollectionReadType; import io.stargate.sgv2.jsonapi.service.operation.collections.FindCollectionOperation; +import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistries; import io.stargate.sgv2.jsonapi.service.operation.query.CQLOption; import io.stargate.sgv2.jsonapi.service.operation.query.DBLogicalExpression; import io.stargate.sgv2.jsonapi.service.operation.tables.*; @@ -22,6 +23,8 @@ import io.stargate.sgv2.jsonapi.service.resolver.matcher.CollectionFilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.FilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.TableFilterResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.SortClauseResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.TableSortClauseResolver; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.util.SortClauseUtil; import jakarta.enterprise.context.ApplicationScoped; @@ -41,6 +44,7 @@ public class FindCommandResolver implements CommandResolver { private final FilterResolver collectionFilterResolver; private final FilterResolver tableFilterResolver; + private final SortClauseResolver tableSortClauseResolver; @Inject public FindCommandResolver( @@ -58,6 +62,8 @@ public FindCommandResolver( this.collectionFilterResolver = new CollectionFilterResolver<>(operationsConfig); this.tableFilterResolver = new TableFilterResolver<>(operationsConfig); + this.tableSortClauseResolver = + new TableSortClauseResolver<>(operationsConfig, JSONCodecRegistries.DEFAULT_REGISTRY); } @Override @@ -72,6 +78,7 @@ public Operation resolveTableCommand(CommandContext ctx, Find Optional.ofNullable(command.options()) .map(FindCommand.Options::limit) .orElse(Integer.MAX_VALUE); + var cqlPageState = Optional.ofNullable(command.options()) .map(options -> CqlPagingState.from(options.pageState())) @@ -81,8 +88,10 @@ public Operation resolveTableCommand(CommandContext ctx, Find TableRowProjection.fromDefinition( objectMapper, command.tableProjectionDefinition(), ctx.schemaObject()); + var orderBy = tableSortClauseResolver.resolve(ctx, command); + var builder = - new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection) + new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection, orderBy) .addBuilderOption(CQLOption.ForSelect.limit(limit)) .addStatementOption(CQLOption.ForStatement.pageSize(operationsConfig.defaultPageSize())) .addPagingState(cqlPageState); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java index 24f47f583f..e7b4f21ba8 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java @@ -13,6 +13,7 @@ import io.stargate.sgv2.jsonapi.service.operation.*; import io.stargate.sgv2.jsonapi.service.operation.collections.CollectionReadType; import io.stargate.sgv2.jsonapi.service.operation.collections.FindCollectionOperation; +import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistries; import io.stargate.sgv2.jsonapi.service.operation.query.CQLOption; import io.stargate.sgv2.jsonapi.service.operation.query.DBLogicalExpression; import io.stargate.sgv2.jsonapi.service.operation.tables.*; @@ -20,6 +21,8 @@ import io.stargate.sgv2.jsonapi.service.resolver.matcher.CollectionFilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.FilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.TableFilterResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.SortClauseResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.TableSortClauseResolver; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.util.SortClauseUtil; import jakarta.enterprise.context.ApplicationScoped; @@ -37,6 +40,7 @@ public class FindOneCommandResolver implements CommandResolver { private final FilterResolver collectionFilterResolver; private final FilterResolver tableFilterResolver; + private final SortClauseResolver tableSortClauseResolver; @Inject public FindOneCommandResolver( @@ -55,6 +59,8 @@ public FindOneCommandResolver( this.collectionFilterResolver = new CollectionFilterResolver<>(operationsConfig); this.tableFilterResolver = new TableFilterResolver<>(operationsConfig); + this.tableSortClauseResolver = + new TableSortClauseResolver<>(operationsConfig, JSONCodecRegistries.DEFAULT_REGISTRY); } @Override @@ -70,8 +76,10 @@ public Operation resolveTableCommand( TableRowProjection.fromDefinition( objectMapper, command.tableProjectionDefinition(), ctx.schemaObject()); + var orderBy = tableSortClauseResolver.resolve(ctx, command); + var builder = - new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection) + new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection, orderBy) .addBuilderOption(CQLOption.ForSelect.limit(1)); // TODO, we may want the ability to resolve API filter clause into multiple diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java index b65d3fda05..633e5148eb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java @@ -2,17 +2,27 @@ import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.model.command.Sortable; -import io.stargate.sgv2.jsonapi.api.model.command.Updatable; import io.stargate.sgv2.jsonapi.config.OperationsConfig; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject; import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; -import io.stargate.sgv2.jsonapi.service.operation.query.UpdateValuesCQLClause; import io.stargate.sgv2.jsonapi.service.resolver.ClauseResolver; -public abstract class SortClauseResolver +/** + * Base for classes that resolve a sort clause on a {@link Command} into something the Operation can + * use to add ORDER BY to a query. + * + *

NOTE: this is for adding the CQL order by, while still do some in memory sorting for CQL that + * is handled differently. + * + * @param The type of the {@link Command} that is being resolved. + * @param The type of the {@link SchemaObject} that {@link Command} command is operating + * on. + */ +public abstract class SortClauseResolver< + CmdT extends Command & Sortable, SchemaT extends SchemaObject> extends ClauseResolver { - protected SortClauseResolver(OperationsConfig operationsConfig){ + protected SortClauseResolver(OperationsConfig operationsConfig) { super(operationsConfig); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java index f8f8cd1328..9c35dd74ae 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java @@ -1,68 +1,100 @@ package io.stargate.sgv2.jsonapi.service.resolver.sort; -import com.datastax.oss.driver.api.core.metadata.schema.TableMetadata; +import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.*; +import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; + import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.Sortable; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression; import io.stargate.sgv2.jsonapi.config.OperationsConfig; -import io.stargate.sgv2.jsonapi.exception.catchable.UnknownColumnException; +import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistry; import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; -import io.stargate.sgv2.jsonapi.service.shredding.CqlNamedValue; -import org.jboss.resteasy.reactive.server.core.RuntimeExceptionMapper; - +import io.stargate.sgv2.jsonapi.service.operation.tables.TableANNOrderByCQlClause; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiTypeName; +import io.stargate.sgv2.jsonapi.util.CqlVectorUtil; import java.util.Objects; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; - -public class TableSortClauseResolver extends SortClauseResolver { +/** + * Resolves a sort clause to deterimine if we want to apply a CQL ORDER BY clause to the operation. + * + *

Not all sort clauses result in a CQL order by, some may result in in memory sorting. + */ +public class TableSortClauseResolver + extends SortClauseResolver { + private static final Logger LOGGER = LoggerFactory.getLogger(TableSortClauseResolver.class); - private final JSONCodecRegistry codecRegistry; - public TableSortClauseResolver(OperationsConfig operationsConfig, JSONCodecRegistry codecRegistry) { + public TableSortClauseResolver( + OperationsConfig operationsConfig, JSONCodecRegistry codecRegistry) { super(operationsConfig); - this.codecRegistry = Objects.requireNonNull(codecRegistry, "codecRegistry must not be null"); } @Override public OrderByCqlClause resolve(CommandContext commandContext, CmdT command) { + Objects.requireNonNull(commandContext, "commandContext is required"); + Objects.requireNonNull(command, "command is required"); + var apiTableDef = commandContext.schemaObject().apiTableDef(); var sortClause = command.sortClause(); if (sortClause == null || sortClause.isEmpty()) { return OrderByCqlClause.NO_OP; } var vectorSorts = sortClause.tableVectorSorts(); - if (vectorSorts.size() > 1){ - throw new RuntimeException("TODO: throw too many ANN sorts"); + if (vectorSorts.isEmpty()) { + // For now, only supporting vector sort + return OrderByCqlClause.NO_OP; } - if (vectorSorts.size() == 1){ - var vectorSort = vectorSorts.getFirst(); - - - return new TableANNOrderByCQlClause(commandContext.getSchemaObject(), vectorSort.column(), vectorSort.vector()); + if (vectorSorts.size() > 1) { + throw SortException.Code.MORE_THAN_ONE_VECTOR_SORT.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put( + "sortColumns", + errFmtJoin(vectorSorts.stream().map(SortExpression::path).toList())); + })); } - } - private OrderByCqlClause resolveTableVectorSort(TableMetadata tableMetadata, SortExpression sortExpression) { + var vectorSort = vectorSorts.getFirst(); + var sortIdentifier = cqlIdentifierFromUserInput(vectorSort.path()); + var apiColumnDef = apiTableDef.allColumns().get(sortIdentifier); + if (apiColumnDef == null) { + throw SortException.Code.CANNOT_SORT_UNKNOWN_COLUMNS.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put("allColumns", errFmtApiColumnDef(apiTableDef.allColumns())); + map.put("unknownColumns", errFmt(sortIdentifier)); + })); + } - var column = cqlIdentifierFromUserInput(sortExpression.path()); - CqlNamedValue cqlNamedValue = null; - try { - var codec = codecRegistry.codecToCQL(tableMetadata, column, sortExpression.vector()); - cqlNamedValue = new CqlNamedValue(tableMetadata, codec.toCQL(rawJsonValue)); - } catch (UnknownColumnException e) { - // this should not happen, we checked above but the codecs are written to be very safe and - // will check and throw - throw ServerException.Code.UNEXPECTED_SERVER_ERROR.get(errVars(e)); - } catch (MissingJSONCodecException e) { - unsupportedErrors.put(metadata, e); - } catch (ToCQLCodecException e) { - codecErrors.put(metadata, e); + if (apiColumnDef.type().typeName() != ApiTypeName.VECTOR) { + throw SortException.Code.CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put("sortColumns", errFmt(sortIdentifier)); + })); } - } + // This is a bit of a hack, we should be using the codecs to convert but for now the Sort + // deserialization + // turns the JSON array into a float array, so we can just use that. + // Needs more refactoring to change how it works + var cqlVector = CqlVectorUtil.floatsToCqlVector(vectorSort.vector()); + return new TableANNOrderByCQlClause(apiColumnDef, cqlVector); + } } diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index acef21dde0..c0e655a859 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -46,7 +46,9 @@ # new line after each entry, using the `|-` to trim trailing newlines. # ================================================================================================================ -# Snippets +# ================================================================================================================ +# SNIPPETS +# ================================================================================================================ # ================================================================================================================ snippets: @@ -68,10 +70,21 @@ snippets: body: |- Resend the request using only defined columns. + - name: VECTOR_SORT_EXPLANATION + body: |- + A vector sort in the sort clause identifies the vector column by name and then provides either: + - The vector as an array of decimal numbers. + - The vector as a base64 encoded `{"$binary": "base64-encoded-vector"}` object. + - A string to be vectorized if enabled for the column. +# ================================================================================================================ +# ================================================================================================================ +# REQUEST Errors +# ================================================================================================================ +# ================================================================================================================ # ================================================================================================================ -# Request Errors +# Family: REQUEST Scope: NONE # ================================================================================================================ request-errors: @@ -84,6 +97,10 @@ request-errors: ${SNIPPET.CONTACT_SUPPORT} +# ================================================================================================================ +# Family: REQUEST Scope: DOCUMENT +# ================================================================================================================ + # DOCUMENT request errors - scope: DOCUMENT code: MISSING_PRIMARY_KEY_COLUMNS @@ -132,7 +149,10 @@ request-errors: Resend the request using only supported column values. - # FILTER request errors +# ================================================================================================================ +# Family: REQUEST Scope: FILTER +# ================================================================================================================ + - scope: FILTER code: INVALID_FILTER title: Invalid filter @@ -223,7 +243,10 @@ request-errors: - Missing Partition Keys: ${missingPartitionKeys}. - Out of order Partition Sort Keys: ${outOfOrderClusteringKeys}. - # UPDATE clause errors +# ================================================================================================================ +# Family: REQUEST Scope: UPDATE +# ================================================================================================================ + - scope: UPDATE code: UNKNOWN_TABLE_COLUMNS title: Cannot update on unknown table columns @@ -259,7 +282,9 @@ request-errors: Supported update operations are ${supportedUpdateOperations}. - # Warnings + # ================================================================================================================ + # Family: REQUEST Scope: WARNING + # ================================================================================================================ - scope: WARNING code: MISSING_INDEX @@ -354,7 +379,9 @@ request-errors: ${SNIPPET.INEFFICIENT_QUERY} - # Schema Errors + # ================================================================================================================ + # Family: REQUEST Scope: SCHEMA + # ================================================================================================================ # aaron errors - scope: SCHEMA @@ -619,12 +646,51 @@ request-errors: body: |- Column `${column}` doesn't exist in the table. +# ================================================================================================================ +# Family: REQUEST Scope: SORT +# ================================================================================================================ + - scope: SORT + code: MORE_THAN_ONE_VECTOR_SORT + title: More than one vector sort provided + body: |- + The command used a sort clause with more than one vector sort, only one vector sort is allowed. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the vector columns: ${vectorColumns}. + The command attempted to sort on the columns: ${sortColumns}. + + Resend the command with only one vector sort. + - scope: SORT + code: CANNOT_SORT_UNKNOWN_COLUMNS + title: Sorted columns are not present in the table schema + body: |- + The command attempted to sort using columns that are not in the table schema. + + The table ${keyspace}.${table} defines the columns: ${allColumns}. + The command attempted to sort the unknown columns: ${unknownColumns}. + + Resend the command using only columns that are defined in the table schema. + - scope: SORT + code: CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS + title: Vector sort requested for non-vector columns + body: |- + The command attemtped to vector sort columns that are not of vector type. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the columns: ${vectorColumns}. + The command attempted to sort the non vector columns: ${sortColumns}. + + Resend the command using only vector columns for vector sorting. # ================================================================================================================ -# Server Errors +# ================================================================================================================ +# Server Errors +# ================================================================================================================ # ================================================================================================================ server-errors: diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java index ecf801dd74..21033a0526 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java @@ -8,6 +8,7 @@ import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CqlPagingState; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.query.CqlOptions; +import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; import io.stargate.sgv2.jsonapi.service.operation.tables.TableDriverExceptionHandler; public class ReadAttemptTestData extends OperationAttemptTestData { @@ -41,6 +42,7 @@ private OperationAttemptFixture newReadAttemptFixture( testData.schemaObject().emptyTableSchemaObject(), testData.selectCQLClause().selectAllFromTable(), testData.whereCQLClause().emptySelect(), + OrderByCqlClause.NO_OP, new CqlOptions<>(), CqlPagingState.EMPTY, null, diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java index ce9954828f..f58f4e71ca 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java @@ -5,6 +5,7 @@ import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CqlPagingState; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.query.CqlOptions; +import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; import io.stargate.sgv2.jsonapi.service.operation.query.SelectCQLClause; import io.stargate.sgv2.jsonapi.service.operation.query.WhereCQLClause; @@ -17,6 +18,7 @@ public class TestReadAttempt extends ReadAttempt { TableSchemaObject schemaObject, SelectCQLClause selectCQLClause, WhereCQLClause cqlOptions, CqlPagingState pagingState, DocumentSourceSupplier documentSourceSupplier, @@ -26,6 +28,7 @@ public class TestReadAttempt extends ReadAttempt { schemaObject, selectCQLClause, whereCQLClause, + orderByCqlClause, cqlOptions, pagingState, documentSourceSupplier); From 879d16d72c0f998c1f8f2b54e97a5b946cd5102c Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Fri, 1 Nov 2024 10:42:58 +1300 Subject: [PATCH 14/18] Added integration tests --- .../model/command/clause/sort/SortClause.java | 12 +- .../deserializers/SortClauseDeserializer.java | 20 +- .../sgv2/jsonapi/exception/SortException.java | 2 + .../service/operation/ReadAttempt.java | 2 +- .../operation/query/OrderByCqlClause.java | 10 + .../tables/TableANNOrderByCQlClause.java | 5 + .../operation/tables/TableInsertAttempt.java | 2 +- .../service/resolver/FindCommandResolver.java | 2 + .../sort/TableSortClauseResolver.java | 60 ++++- .../service/schema/tables/ApiColumnDef.java | 6 +- .../schema/tables/ApiColumnDefContainer.java | 26 ++- .../service/schema/tables/ApiVectorType.java | 4 + src/main/resources/errors.yaml | 34 ++- .../AbstractKeyspaceIntegrationTestBase.java | 2 +- .../tables/AnnSortTableIntegrationTest.java | 210 ++++++++++++++++++ .../ProjectionSchemaIntegrationTest.java | 69 +++--- .../api/v1/util/DataApiCommandSenders.java | 2 - .../api/v1/util/DataApiResponseValidator.java | 9 + .../api/v1/util/KeyspaceTemplates.java | 21 ++ .../jsonapi/api/v1/util/TableTemplates.java | 104 +++++++-- .../jsonapi/api/v1/util/TemplateRunner.java | 2 +- .../api/v1/util/TestDataScenarios.java | 116 ---------- .../AllScalarTypesTableScenario.java | 55 +++++ .../v1/util/scenarios/TestDataScenario.java | 154 +++++++++++++ .../VectorDimension5TableScenario.java | 94 ++++++++ .../jsonapi/fixtures/data/DefaultData.java | 25 ++- .../jsonapi/fixtures/data/FixtureData.java | 10 + 27 files changed, 868 insertions(+), 190 deletions(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java delete mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java index ce37d337b7..f19140e795 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java @@ -33,7 +33,17 @@ public boolean isEmpty() { } public List tableVectorSorts() { - return sortExpressions.stream().filter(SortExpression::isTableVectorSort).toList(); + return sortExpressions == null + ? List.of() + : sortExpressions.stream().filter(SortExpression::isTableVectorSort).toList(); + } + + public List nonTableVectorSorts() { + return sortExpressions == null + ? List.of() + : sortExpressions.stream() + .filter(sortExpression -> !sortExpression.isTableVectorSort()) + .toList(); } public boolean hasVsearchClause() { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java index 8e26131dee..9e662a75a8 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java @@ -88,7 +88,14 @@ public SortClause deserialize(JsonParser parser, DeserializationContext ctxt) th // TODO: aaron 17-oct-2024 - this break seems unneeded as above it checks if there is only 1 // field, leaving for now break; - + } else if (inner.getValue().isArray()) { + // TODO: HACK: quick support for tables, if the value is an array we will assume the column + // is a vector then need to check on table pathway that the sort is correct. + // NOTE: does not check if there are more than one sort expression, the + // TableSortClauseResolver will take care of that so we can get proper ApiExceptions + // this is also why we do not break the look here + sortExpressions.add( + SortExpression.tableVectorSort(path, arrayNodeToVector((ArrayNode) inner.getValue()))); } else { if (path.isBlank()) { throw ErrorCodeV1.INVALID_SORT_CLAUSE_PATH.toApiException( @@ -100,16 +107,7 @@ public SortClause deserialize(JsonParser parser, DeserializationContext ctxt) th "sort clause path ('%s') contains character(s) not allowed", path); } - // TODO: HACK: quick support for tables, if the value is an array we will assume the column - // is a vector then need to check on table pathway that the sort is correct. - // the code above checks for $vector and $vectorize - if (inner.getValue().isArray()) { - sortExpressions.add( - SortExpression.tableVectorSort( - path, arrayNodeToVector((ArrayNode) inner.getValue()))); - continue; - - } else if (!inner.getValue().isInt() + if (!inner.getValue().isInt() || !(inner.getValue().intValue() == 1 || inner.getValue().intValue() == -1)) { throw ErrorCodeV1.INVALID_SORT_CLAUSE_VALUE.toApiException( "Sort ordering value can only be `1` for ascending or `-1` for descending (not `%s`)", diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java index 09450d0fc7..e63d38e634 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java @@ -16,6 +16,8 @@ public SortException(ErrorInstance errorInstance) { public enum Code implements ErrorCode { CANNOT_SORT_UNKNOWN_COLUMNS, CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, + CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, + CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, MORE_THAN_ONE_VECTOR_SORT, ; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java index 1ccaafc985..514aa2fb5a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java @@ -194,7 +194,7 @@ public Optional schemaDescription() { "Unsupported columns in the result set: %s" .formatted(errFmtApiColumnDef(readApiColumns.filterByUnsupported()))); } - return Optional.of(readApiColumns.toColumnsDef()); + return Optional.of(readApiColumns.toColumnsDesc()); } // This is a simple container for the result set so we can set one variable in the onSuccess diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java index e7aa428456..c7d93484c3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java @@ -23,4 +23,14 @@ public interface OrderByCqlClause extends Function, CQLClause { // No Op implementation just returns the select, use this when no order by is needed OrderByCqlClause NO_OP = select -> select; + + /** + * If true, this means that the query will need to be sorted in memory after the query is + * executed. + * + * @return true if the query needs to be sorted in memory + */ + default boolean inMemorySortNeeded() { + return true; + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java index 47f91bb214..d21b278be7 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java @@ -34,4 +34,9 @@ public TableANNOrderByCQlClause(ApiColumnDef apiColumnDef, CqlVector vect public Select apply(Select select) { return select.orderByAnnOf(apiColumnDef.name(), vector); } + + @Override + public boolean inMemorySortNeeded() { + return false; + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java index 6442c018d2..fc380a223c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java @@ -51,6 +51,6 @@ public Optional schemaDescription() { "Unsupported columns primary key: %s" + apiColumns.filterByUnsupported()); } - return Optional.of(apiColumns.toColumnsDef()); + return Optional.of(apiColumns.toColumnsDesc()); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java index 7ddbb6d0a4..07f5b85863 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java @@ -90,6 +90,8 @@ public Operation resolveTableCommand(CommandContext ctx, Find var orderBy = tableSortClauseResolver.resolve(ctx, command); + // make the Sorter resolver and pass ing the order by clause + var builder = new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection, orderBy) .addBuilderOption(CQLOption.ForSelect.limit(limit)) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java index 9c35dd74ae..28a04ab19b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java @@ -3,6 +3,8 @@ import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.*; import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.datastax.oss.driver.api.core.metadata.schema.IndexMetadata; import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.Sortable; @@ -13,16 +15,19 @@ import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistry; import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; import io.stargate.sgv2.jsonapi.service.operation.tables.TableANNOrderByCQlClause; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; import io.stargate.sgv2.jsonapi.service.schema.tables.ApiTypeName; import io.stargate.sgv2.jsonapi.util.CqlVectorUtil; +import java.util.List; import java.util.Objects; +import java.util.Optional; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Resolves a sort clause to deterimine if we want to apply a CQL ORDER BY clause to the operation. * - *

Not all sort clauses result in a CQL order by, some may result in in memory sorting. + *

Not all sort clauses result in a CQL order by, some may result in memory sorting. */ public class TableSortClauseResolver extends SortClauseResolver { @@ -38,6 +43,7 @@ public OrderByCqlClause resolve(CommandContext commandContext Objects.requireNonNull(commandContext, "commandContext is required"); Objects.requireNonNull(command, "command is required"); + // NOTE: Currently only supporting vector sort, next PR will deal with clustering key sorting var apiTableDef = commandContext.schemaObject().apiTableDef(); var sortClause = command.sortClause(); if (sortClause == null || sortClause.isEmpty()) { @@ -64,6 +70,25 @@ public OrderByCqlClause resolve(CommandContext commandContext })); } + // we have one vector sort - cannot have any other sorting + var nonVectorSorts = sortClause.nonTableVectorSorts(); + if (!nonVectorSorts.isEmpty()) { + throw SortException.Code.CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put( + "sortVectorColumns", + errFmtJoin(vectorSorts.stream().map(SortExpression::path).toList())); + map.put( + "sortNonVectorColumns", + errFmtJoin(nonVectorSorts.stream().map(SortExpression::path).toList())); + })); + } + var vectorSort = vectorSorts.getFirst(); var sortIdentifier = cqlIdentifierFromUserInput(vectorSort.path()); var apiColumnDef = apiTableDef.allColumns().get(sortIdentifier); @@ -89,6 +114,23 @@ public OrderByCqlClause resolve(CommandContext commandContext })); } + // HACK - waiting for index support on the APiTableDef + var optionalIndexMetadata = findIndexMetadata(commandContext.schemaObject(), apiColumnDef); + if (optionalIndexMetadata.isEmpty()) { + throw SortException.Code.CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put( + "indexedColumns", + errFmtJoin(indexedVectorColumns(commandContext.schemaObject()))); + map.put("sortColumns", errFmt(sortIdentifier)); + })); + } + // This is a bit of a hack, we should be using the codecs to convert but for now the Sort // deserialization // turns the JSON array into a float array, so we can just use that. @@ -97,4 +139,20 @@ public OrderByCqlClause resolve(CommandContext commandContext return new TableANNOrderByCQlClause(apiColumnDef, cqlVector); } + + private Optional findIndexMetadata( + TableSchemaObject schemaObject, ApiColumnDef targetColumn) { + return schemaObject.tableMetadata().getIndexes().values().stream() + .filter(index -> index.getTarget().equals(targetColumn.name().asInternal())) + .findFirst(); + } + + private List indexedVectorColumns(TableSchemaObject schemaObject) { + + var apiVectorColumns = schemaObject.apiTableDef().allColumns().filterBy(ApiTypeName.VECTOR); + return schemaObject.tableMetadata().getIndexes().values().stream() + .map(IndexMetadata::getTarget) + .filter(target -> apiVectorColumns.containsKey(CqlIdentifier.fromInternal(target))) + .toList(); + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java index 5c2c86dfc3..4ef746301c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java @@ -43,9 +43,9 @@ public class ApiColumnDef { private final CqlIdentifier name; private final ApiDataType type; - private ApiColumnDef(CqlIdentifier name, ApiDataType type) { - this.name = name; - this.type = type; + public ApiColumnDef(CqlIdentifier name, ApiDataType type) { + this.name = Objects.requireNonNull(name, "name is must not be null"); + this.type = Objects.requireNonNull(type, "type is must not be null"); } /** diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java index d474b9ab22..7576dcd162 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java @@ -20,6 +20,9 @@ public class ApiColumnDefContainer extends LinkedHashMap { private static final Logger LOGGER = LoggerFactory.getLogger(ApiColumnDefContainer.class); + private static final ApiColumnDefContainer IMMUTABLE_EMPTY = + new ApiColumnDefContainer(0).toUnmodifiable(); + public static final CqlColumnFactory FROM_CQL_FACTORY = new CqlColumnFactory(); public static final ColumnDescFactory FROM_COLUMN_DESC_FACTORY = new ColumnDescFactory(); @@ -44,6 +47,27 @@ public ApiColumnDefContainer(List columnDefs) { columnDefs.forEach(this::put); } + public static ApiColumnDefContainer of() { + return IMMUTABLE_EMPTY; + } + + public static ApiColumnDefContainer of(ApiColumnDef col1) { + return new ApiColumnDefContainer(List.of(col1)).toUnmodifiable(); + } + + public static ApiColumnDefContainer of(ApiColumnDef col1, ApiColumnDef col2) { + return new ApiColumnDefContainer(List.of(col1, col2)).toUnmodifiable(); + } + + public static ApiColumnDefContainer of(ApiColumnDef col1, ApiColumnDef col2, ApiColumnDef col3) { + return new ApiColumnDefContainer(List.of(col1, col2, col3)).toUnmodifiable(); + } + + public static ApiColumnDefContainer of( + ApiColumnDef col1, ApiColumnDef col2, ApiColumnDef col3, ApiColumnDef col4) { + return new ApiColumnDefContainer(List.of(col1, col2, col3, col4)).toUnmodifiable(); + } + public ApiColumnDefContainer toUnmodifiable() { return this instanceof UnmodifiableApiColumnDefContainer ? this @@ -91,7 +115,7 @@ public Map getVectorizeDefs() { columnDef -> ((ApiVectorType) columnDef.type()).getVectorizeDefinition())); } - public ColumnsDescContainer toColumnsDef() { + public ColumnsDescContainer toColumnsDesc() { ColumnsDescContainer columnsDesc = new ColumnsDescContainer(size()); forEach((name, columnDef) -> columnsDesc.put(name, columnDef.type().columnDesc())); return columnsDesc; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java index 6daf6b76b5..6be665426d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java @@ -59,6 +59,10 @@ public VectorizeDefinition getVectorizeDefinition() { return vectorizeDefinition; } + public static ApiVectorType from(int dimension) { + return from(dimension, null); + } + public static ApiVectorType from(int dimension, VectorizeDefinition vectorizeDefinition) { // Sanity check diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index c0e655a859..3ccec4defc 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -678,7 +678,7 @@ request-errors: code: CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS title: Vector sort requested for non-vector columns body: |- - The command attemtped to vector sort columns that are not of vector type. + The command attempted to vector sort columns that are not of vector type. ${SNIPPET.VECTOR_SORT_EXPLANATION} @@ -687,6 +687,38 @@ request-errors: Resend the command using only vector columns for vector sorting. + - scope: SORT + code: CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS + title: Vector sort requested for vector columns that are not indexed + body: |- + The command attempted to vector sort vector columns that are not indexed. + + Vector sorting is only supported on vector columns that have been indexed. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the vector columns: ${vectorColumns}. + And has indexes on the vector columns: ${indexedColumns}. + The command attempted to sort vector columns: ${sortColumns}. + + Resend the command using only vector columns that have been indexed. + + - scope: SORT + code: CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT + title: Vector sort requested for vector column and non vector columns + body: |- + The command attempted to vector sort vector columns and other non vector columns. + + Vector sorts must use a single vector column, and cannot include any other sort columns. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the vector columns: ${vectorColumns}. + The command attempted to sort the vector columns: ${sortVectorColumns}. + The command attempted to sort the non-vector columns: ${sortNonVectorColumns}. + + Resend the command using only vector columns that have been indexed. + # ================================================================================================================ # ================================================================================================================ # Server Errors diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java index ff654e6176..e165d0693c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java @@ -35,7 +35,7 @@ public abstract class AbstractKeyspaceIntegrationTestBase { // keyspace automatically created in this test - protected final String keyspaceName = "ks" + RandomStringUtils.randomAlphanumeric(16); + protected static final String keyspaceName = "ks" + RandomStringUtils.randomAlphanumeric(16); @BeforeAll public static void enableLog() { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java new file mode 100644 index 0000000000..1f97b78284 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java @@ -0,0 +1,210 @@ +package io.stargate.sgv2.jsonapi.api.v1.tables; + +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; + +import io.quarkus.test.common.WithTestResource; +import io.quarkus.test.junit.QuarkusIntegrationTest; +import io.stargate.sgv2.jsonapi.api.model.command.Command; +import io.stargate.sgv2.jsonapi.api.v1.util.scenarios.VectorDimension5TableScenario; +import io.stargate.sgv2.jsonapi.exception.SortException; +import io.stargate.sgv2.jsonapi.testresource.DseTestResource; +import java.util.ArrayList; +import java.util.Map; +import java.util.stream.Stream; +import org.junit.jupiter.api.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.testcontainers.shaded.com.google.common.collect.ImmutableMap; + +@QuarkusIntegrationTest +@WithTestResource(value = DseTestResource.class, restrictToAnnotatedClass = false) +@TestClassOrder(ClassOrderer.OrderAnnotation.class) +public class AnnSortTableIntegrationTest extends AbstractTableIntegrationTestBase { + + private static final String TABLE_NAME = "annSortTableTest"; + private static final VectorDimension5TableScenario SCENARIO = + new VectorDimension5TableScenario(keyspaceName, TABLE_NAME); + + @BeforeAll + public final void createScenario() { + SCENARIO.create(); + } + + @AfterAll + public final void dropScenario() { + SCENARIO.drop(); + } + + private static Stream findCommandNames() { + + var commands = new ArrayList(); + commands.add(Arguments.of(Command.CommandName.FIND)); + commands.add(Arguments.of(Command.CommandName.FIND_ONE)); + return commands.stream(); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findUnindexedVector(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, + SortException.class, + "And has indexes on the vector columns: %s.\nThe command attempted to sort vector columns: %s" + .formatted(SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findMoreThanOneVector(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.MORE_THAN_ONE_VECTOR_SORT, + SortException.class, + "The command attempted to sort on the columns: %s, %s." + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findUnknownVectorColumn(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL) + "unknown", + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_SORT_UNKNOWN_COLUMNS, + SortException.class, + "The command attempted to sort the unknown columns: %s." + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL) + + "unknown")); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findNonVectorCol(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, + SortException.class, + "The command attempted to sort the non vector columns: %s." + .formatted(SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findSortVectorAndNon(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL), + 1); // asc sort + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, + SortException.class, + "The command attempted to sort the vector columns: %s.\nThe command attempted to sort the non-vector columns: %s." + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findRandomVector(Command.CommandName commandName) { + // Doing a sort for a vector we do not know if is in the table + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL)); + + var limit = 3; + Map options = + commandName == Command.CommandName.FIND ? ImmutableMap.of("limit", limit) : null; + + var validator = + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort, options) + .wasSuccessful(); + + if (commandName == Command.CommandName.FIND) { + validator.hasDocuments(limit); + } else { + validator.hasSingleDocument(); + } + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findKnownVector(Command.CommandName commandName) { + // Doing a sort for a vector we know the vector is in the table, we can match on the expected + // doc + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + (Object) VectorDimension5TableScenario.KNOWN_VECTOR); + + var limit = 1; + Map options = + commandName == Command.CommandName.FIND ? ImmutableMap.of("limit", limit) : null; + + var validator = + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort, options) + .wasSuccessful(); + + if (commandName == Command.CommandName.FIND) { + validator + .hasDocuments(limit) + .hasDocumentInPosition(0, VectorDimension5TableScenario.KNOWN_VECTOR_ROW_JSON); + + } else { + validator.hasSingleDocument(VectorDimension5TableScenario.KNOWN_VECTOR_ROW_JSON); + } + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java index 5cebd114f9..030a735961 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java @@ -2,20 +2,19 @@ import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.*; +import com.datastax.oss.driver.api.core.CqlIdentifier; import io.quarkus.test.common.WithTestResource; import io.quarkus.test.junit.QuarkusIntegrationTest; import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.v1.util.DataApiResponseValidator; -import io.stargate.sgv2.jsonapi.api.v1.util.TestDataScenarios; +import io.stargate.sgv2.jsonapi.api.v1.util.scenarios.AllScalarTypesTableScenario; import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDefContainer; import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataTypeDefs; -import io.stargate.sgv2.jsonapi.service.schema.tables.PrimitiveApiDataTypeDef; import io.stargate.sgv2.jsonapi.testresource.DseTestResource; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.*; import org.junit.jupiter.params.ParameterizedTest; @@ -32,38 +31,46 @@ */ public class ProjectionSchemaIntegrationTest extends AbstractTableIntegrationTestBase { - static final String TABLE_NAME = "projectionSchemaTable"; + private static final String TABLE_NAME = "projectionSchemaTable"; + private static final AllScalarTypesTableScenario TEST_DATA_SCENARIO = + new AllScalarTypesTableScenario(keyspaceName, TABLE_NAME); @BeforeAll - public final void createDefaultTables() { - TEST_DATA_SCENARIOS.allScalarTypesRowsWithNulls(keyspaceName, TABLE_NAME); + public final void createScenario() { + TEST_DATA_SCENARIO.create(); + } + + @AfterAll + public final void dropScenario() { + TEST_DATA_SCENARIO.drop(); } private static Stream findWithProjectionSchemaTests() { var commands = List.of(Command.CommandName.FIND, Command.CommandName.FIND_ONE); - // we ask to project columns of these types - List> typeCombinations = + // we ask to project these columns + List projections = List.of( - List.of(), // for the default no projection - ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE, - List.of(ApiDataTypeDefs.DURATION), - List.of(ApiDataTypeDefs.TEXT, ApiDataTypeDefs.DURATION), - List.of(ApiDataTypeDefs.TEXT, ApiDataTypeDefs.DURATION, ApiDataTypeDefs.INT)); + ApiColumnDefContainer.of(), // for the default no projection + TEST_DATA_SCENARIO.nonPkColumns, + ApiColumnDefContainer.of(TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.TEXT)), + ApiColumnDefContainer.of( + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.TEXT), + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.DURATION)), + ApiColumnDefContainer.of( + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.TEXT), + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.DURATION), + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.INT))); var combinations = new ArrayList(); for (var command : commands) { - for (var types : typeCombinations) { - - var columns = - types.stream() - .collect(Collectors.toMap(TestDataScenarios::columnName, Function.identity())); - combinations.add(Arguments.of(command, columns)); + for (var projection : projections) { + combinations.add(Arguments.of(command, projection)); } // specials combinations.add( - Arguments.of(command, Map.of(TestDataScenarios.ID_COL, ApiDataTypeDefs.TEXT))); + Arguments.of(command, ApiColumnDefContainer.of(TEST_DATA_SCENARIO.primaryKey))); } return combinations.stream(); @@ -72,13 +79,16 @@ private static Stream findWithProjectionSchemaTests() { @ParameterizedTest @MethodSource("findWithProjectionSchemaTests") public void findFilterWithProjectionSchema( - Command.CommandName commandName, Map columns) { + Command.CommandName commandName, ApiColumnDefContainer projectionColumns) { // row-1 is the row with all the values set var validator = assertTableCommand(keyspaceName, TABLE_NAME) .templated() - .find(commandName, Map.of("id", "row-1"), columns.keySet().stream().toList()) + .find( + commandName, + Map.of("id", "row-1"), + projectionColumns.keySet().stream().map(CqlIdentifier::asInternal).toList()) .wasSuccessful() .hasProjectionSchema(); @@ -90,18 +100,21 @@ public void findFilterWithProjectionSchema( throw new IllegalArgumentException("Unexpected command name: " + commandName); } - assertProjectionSchema(validator, columns); + assertProjectionSchema(validator, projectionColumns); } @ParameterizedTest @MethodSource("findWithProjectionSchemaTests") public void findNoFilterWithProjectionSchema( - Command.CommandName commandName, Map projectionColumns) { + Command.CommandName commandName, ApiColumnDefContainer projectionColumns) { var validator = assertTableCommand(keyspaceName, TABLE_NAME) .templated() - .find(commandName, Map.of(), projectionColumns.keySet().stream().toList()) + .find( + commandName, + Map.of(), + projectionColumns.keySet().stream().map(CqlIdentifier::asInternal).toList()) .wasSuccessful() .hasProjectionSchema(); @@ -131,8 +144,8 @@ public void findManyProjectionMissingColumns() { } private DataApiResponseValidator assertProjectionSchema( - DataApiResponseValidator validator, Map columns) { - columns.forEach(validator::hasProjectionSchemaWith); + DataApiResponseValidator validator, ApiColumnDefContainer columns) { + columns.values().forEach(validator::hasProjectionSchemaWith); return validator; } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java index 15bac61009..2bba51b1b7 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java @@ -9,6 +9,4 @@ public static DataApiKeyspaceCommandSender assertNamespaceCommand(String keyspac public static DataApiTableCommandSender assertTableCommand(String keyspace, String tableName) { return new DataApiTableCommandSender(keyspace, tableName); } - - public static final TestDataScenarios TEST_DATA_SCENARIOS = new TestDataScenarios(); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java index 8082bc5ee7..f2a957d943 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java @@ -10,6 +10,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.config.constants.ErrorObjectV2Constants; import io.stargate.sgv2.jsonapi.exception.*; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataType; import java.util.Map; import org.hamcrest.Matcher; @@ -232,6 +233,10 @@ public DataApiResponseValidator hasSingleDocument() { return body("data.document", is(notNullValue())); } + public DataApiResponseValidator hasSingleDocument(String documentJSON) { + return body("data.document", jsonEquals(documentJSON)); + } + public DataApiResponseValidator hasEmptyDataDocuments() { return body("data.documents", is(empty())); } @@ -245,6 +250,10 @@ public DataApiResponseValidator hasProjectionSchema() { return hasField("status." + CommandStatus.PROJECTION_SCHEMA); } + public DataApiResponseValidator hasProjectionSchemaWith(ApiColumnDef columnDef) { + return hasProjectionSchemaWith(columnDef.name().asInternal(), columnDef.type()); + } + public DataApiResponseValidator hasProjectionSchemaWith(String columnName, ApiDataType type) { // expected format /** diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java index fbe82152ca..1a04f06671 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.api.v1.util; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDefContainer; import java.util.Map; public class KeyspaceTemplates extends TemplateRunner { @@ -14,6 +16,25 @@ public KeyspaceTemplates(DataApiKeyspaceCommandSender sender) { // DDL - TABLES // =================================================================================================================== + public DataApiResponseValidator createTable( + String tableName, ApiColumnDefContainer columns, ApiColumnDef primaryKeyDef) { + var json = + """ + { + "name": "%s", + "definition": { + "columns": %s, + "primaryKey": %s + } + } + """ + .formatted( + tableName, + asJSON(columns.toColumnsDesc()), + asJSON(primaryKeyDef.name().asInternal())); + return sender.postCreateTable(json); + } + public DataApiResponseValidator createTable( String tableName, Map columns, Object primaryKeyDef) { var json = diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java index 77e96c61b4..c8045a7f03 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java @@ -1,16 +1,13 @@ package io.stargate.sgv2.jsonapi.api.v1.util; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; import io.stargate.sgv2.jsonapi.api.model.command.Command; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; public class TableTemplates extends TemplateRunner { - private static final ObjectMapper MAPPER = new ObjectMapper(); - private DataApiTableCommandSender sender; public TableTemplates(DataApiTableCommandSender sender) { @@ -21,36 +18,87 @@ public TableTemplates(DataApiTableCommandSender sender) { // DML - FIND // ================================================================================================================== - private String findClause(Map filter, List columns) { - var projection = columns.stream().collect(Collectors.toMap(col -> col, col -> 1)); - - var clause = - Map.of( - "filter", filter, - "projection", projection); + private String findClause( + Map filter, + List projection, + Map sort, + Map options) { - try { - return MAPPER.writeValueAsString(clause); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); + var clause = new LinkedHashMap<>(); + if (filter != null) { + clause.put("filter", filter); + } + if (projection != null) { + // this is for the positive projection of selecting columns + clause.put("projection", projection.stream().collect(Collectors.toMap(col -> col, col -> 1))); + } + if (sort != null) { + clause.put("sort", sort); } + if (sort != null) { + clause.put("options", options); + } + return asJSON(clause); } public DataApiResponseValidator find( Command.CommandName commandName, Map filter, List columns) { + return find(commandName, filter, columns, null); + } + + public DataApiResponseValidator find( + Command.CommandName commandName, + Map filter, + List columns, + Map sort) { + return find(commandName, filter, columns, sort, null); + } + + public DataApiResponseValidator find( + Command.CommandName commandName, + Map filter, + List columns, + Map sort, + Map options) { return switch (commandName) { - case FIND_ONE -> findOne(filter, columns); - case FIND -> find(filter, columns); + case FIND_ONE -> findOne(filter, columns, sort, options); + case FIND -> find(filter, columns, sort, options); default -> throw new IllegalArgumentException("Unexpected command for find: " + commandName); }; } public DataApiResponseValidator findOne(Map filter, List columns) { - return sender.postFindOne(findClause(filter, columns)); + return findOne(filter, columns, null, null); + } + + public DataApiResponseValidator findOne( + Map filter, List columns, Map sort) { + return findOne(filter, columns, sort, null); + } + + public DataApiResponseValidator findOne( + Map filter, + List columns, + Map sort, + Map options) { + return sender.postFindOne(findClause(filter, columns, sort, options)); } - public DataApiResponseValidator find(Map filter, List columns) { - return sender.postFind(findClause(filter, columns)); + public DataApiResponseValidator find(Map filter, List projection) { + return find(filter, projection, null); + } + + public DataApiResponseValidator find( + Map filter, List projection, Map sort) { + return find(filter, projection, sort, null); + } + + public DataApiResponseValidator find( + Map filter, + List projection, + Map sort, + Map options) { + return sender.postFind(findClause(filter, projection, sort, options)); } public DataApiResponseValidator find(String filter) { @@ -122,6 +170,10 @@ public DataApiResponseValidator insertOne(String document) { return sender.postInsertOne(json); } + public DataApiResponseValidator insertManyMap(List> documents) { + return insertMany(documents.stream().map(TemplateRunner::asJSON).collect(Collectors.toList())); + } + public DataApiResponseValidator insertMany(String... documents) { return insertMany(List.of(documents)); } @@ -153,6 +205,18 @@ public DataApiResponseValidator createIndex(String indexName, String column) { return sender.postCreateIndex(json); } + public DataApiResponseValidator createVectorIndex(String indexName, String column) { + var json = + """ + { + "name": "%s", + "definition": {"column": "%s"} + } + """ + .formatted(indexName, column); + return sender.postCreateVectorIndex(json); + } + public DataApiResponseValidator alterTable(String alterOperation, Object columns) { var json = """ diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java index 4fc350489c..ac05416691 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java @@ -9,7 +9,7 @@ public abstract class TemplateRunner { protected TemplateRunner() {} - protected static String asJSON(Object ob) { + public static String asJSON(Object ob) { try { return MAPPER.writeValueAsString(ob); } catch (IOException e) { // should never happen diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java deleted file mode 100644 index cd78159274..0000000000 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java +++ /dev/null @@ -1,116 +0,0 @@ -package io.stargate.sgv2.jsonapi.api.v1.util; - -import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertNamespaceCommand; -import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; - -import com.fasterxml.jackson.databind.ObjectMapper; -import io.stargate.sgv2.jsonapi.fixtures.data.DefaultData; -import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; -import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataTypeDefs; -import io.stargate.sgv2.jsonapi.service.schema.tables.PrimitiveApiDataTypeDef; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Re-usable scenarios of tables and sample data for testing. - * - *

Will create the table, insert rows, and verify - */ -public class TestDataScenarios { - - private static final Logger LOGGER = LoggerFactory.getLogger(TestDataScenarios.class); - private static final ObjectMapper MAPPER = new ObjectMapper(); - - public static final String ID_COL = "id"; - public static final String COL_NAME_PREFIX = "col_"; - - private static final DefaultData dataSource = new DefaultData(); - - /** - * Create a table with a col for all {@link - * io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting#ALL_SCALAR_TYPES_FOR_CREATE} and - * insert a rows, one with all set, one for each col with a null value. - * - * @param keyspaceName - * @param tableName - */ - public void allScalarTypesRowsWithNulls(String keyspaceName, String tableName) { - - createTableWithTypes( - keyspaceName, tableName, ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE); - - var rows = new ArrayList<>(); - for (int i = -1; i < ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE.size(); i++) { - - // linked to keep order - var row = new LinkedHashMap(); - row.put(ID_COL, "row" + i); - addRowValues(row, ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE, i); - rows.add(row); - } - var rowsJson = - rows.stream() - .map( - row -> { - try { - return MAPPER.writeValueAsString(row); - } catch (Exception e) { - throw new RuntimeException(e); - } - }) - .toList(); - LOGGER.warn("Inserting rows {}", rowsJson); - - assertTableCommand(keyspaceName, tableName).templated().insertMany(rowsJson).wasSuccessful(); - } - - private void createTableWithTypes( - String keyspaceName, String tableName, List types) { - - // linked to keep order - var columns = new LinkedHashMap(); - columns.put(ID_COL, columnDef(ApiDataTypeDefs.TEXT)); - buildColumns(columns, types); - - LOGGER.warn("Creating table {} with columns {}", tableName, columns); - assertNamespaceCommand(keyspaceName) - .templated() - .createTable(tableName, columns, ID_COL) - .wasSuccessful(); - } - - private LinkedHashMap buildColumns( - LinkedHashMap columns, List types) { - - types.forEach( - type -> { - columns.put(columnName(type), columnDef(type)); - }); - return columns; - } - - private static Map columnDef(PrimitiveApiDataTypeDef type) { - return Map.of("type", type.typeName().apiName()); - } - - public static String columnName(PrimitiveApiDataTypeDef type) { - return COL_NAME_PREFIX + type.typeName().apiName(); - } - - public Object columnValue(PrimitiveApiDataTypeDef type) { - return dataSource.fromJSON(type.cqlType()).value(); - } - - private LinkedHashMap addRowValues( - LinkedHashMap row, List types, int nullIndex) { - for (int i = 0; i < types.size(); i++) { - var type = types.get(i); - row.put(columnName(type), i == nullIndex ? null : columnValue(type)); - } - return row; - } -} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java new file mode 100644 index 0000000000..880b0a7c43 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java @@ -0,0 +1,55 @@ +package io.stargate.sgv2.jsonapi.api.v1.util.scenarios; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import io.stargate.sgv2.jsonapi.fixtures.data.DefaultData; +import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDefContainer; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataType; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Creates a table with: - PK of the single field column {@link TestDataScenario#ID_COL} - a column + * for each scalar type in {@link ApiDataTypesForTesting#ALL_SCALAR_TYPES_FOR_CREATE} - the non PK + * columns are named with the prefix {@link #COL_NAME_PREFIX} then the data type name - gets data + * from the {@link DefaultData} class - inserts row with the PK prefix "row" - the row "row-1" (-1) + * has a value for every column - the row "row-0" and beyond have a null for a single column in each + * row, one column at a time - call {@link #columnForDatatype(ApiDataType)} to get the column for a + * specific data type + */ +public class AllScalarTypesTableScenario extends TestDataScenario { + + public static final String COL_NAME_PREFIX = "col_"; + + public AllScalarTypesTableScenario(String keyspaceName, String tableName) { + super(keyspaceName, tableName, ID_COL, createColumns(), new DefaultData()); + } + + public ApiColumnDef columnForDatatype(ApiDataType type) { + return nonPkColumns.get(columnNameForType(type)); + } + + public static CqlIdentifier columnNameForType(ApiDataType type) { + return CqlIdentifier.fromInternal(COL_NAME_PREFIX + type.typeName().apiName()); + } + + private static ApiColumnDefContainer createColumns() { + var columns = new ApiColumnDefContainer(); + columns.put(ID_COL); + addColumnsForTypes( + columns, + ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE, + AllScalarTypesTableScenario::columnNameForType); + return columns; + } + + @Override + protected void insertRows() { + AtomicInteger pkCounter = new AtomicInteger(-1); + + var rows = + createRowForEachNonPkColumnWithNull(apiColumnDef -> "row" + pkCounter.getAndIncrement()); + + insertManyRows(rows); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java new file mode 100644 index 0000000000..5750b5fdb7 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java @@ -0,0 +1,154 @@ +package io.stargate.sgv2.jsonapi.api.v1.util.scenarios; + +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertNamespaceCommand; +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonLiteral; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonType; +import io.stargate.sgv2.jsonapi.fixtures.data.FixtureData; +import io.stargate.sgv2.jsonapi.service.schema.tables.*; +import java.util.*; +import java.util.function.Function; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Base for classes that build a test data scenario */ +public abstract class TestDataScenario { + private static final Logger LOGGER = LoggerFactory.getLogger(TestDataScenario.class); + private static final ObjectMapper MAPPER = new ObjectMapper(); + + public static final ApiColumnDef ID_COL = + new ApiColumnDef(CqlIdentifier.fromInternal("id"), ApiDataTypeDefs.TEXT); + + protected final String keyspaceName; + protected final String tableName; + + public final FixtureData dataSource; + public final ApiColumnDef primaryKey; + public final ApiColumnDefContainer allColumns; + public final ApiColumnDefContainer nonPkColumns; + + protected TestDataScenario( + String keyspaceName, + String tableName, + ApiColumnDef primaryKey, + ApiColumnDefContainer allColumns, + FixtureData dataSource) { + this.keyspaceName = keyspaceName; + this.tableName = tableName; + this.dataSource = dataSource; + + this.primaryKey = primaryKey; + this.allColumns = allColumns.toUnmodifiable(); + var workingNonPkColumns = new ApiColumnDefContainer(allColumns); + workingNonPkColumns.remove(primaryKey.name()); + this.nonPkColumns = workingNonPkColumns.toUnmodifiable(); + } + + public void create() { + createTable(); + createIndexes(); + insertRows(); + } + + public void drop() { + assertNamespaceCommand(keyspaceName).templated().dropTable(tableName, false).wasSuccessful(); + } + + public static String fieldName(ApiColumnDef apiColumnDef) { + return apiColumnDef.name().asInternal(); + } + + public Object columnValue(ApiColumnDef apiColumnDef) { + var jsonLiteral = dataSource.fromJSON(apiColumnDef); + if (jsonLiteral.type() == JsonType.ARRAY) { + List> literals = (List>) jsonLiteral.value(); + return literals.stream().map(JsonLiteral::value).toArray(); + } + return jsonLiteral.value(); + } + + protected void createTable() { + createTableWithTypes(keyspaceName, tableName, primaryKey, allColumns); + } + + protected void createIndexes() {} + ; + + protected void insertRows() {} + ; + + // ========= Helper methods ========= + + protected void createTableWithTypes( + String keyspaceName, + String tableName, + ApiColumnDef primaryKey, + ApiColumnDefContainer columns) { + + LOGGER.warn("Creating table {}.{} with columns {}", keyspaceName, tableName, columns); + assertNamespaceCommand(keyspaceName) + .templated() + .createTable(tableName, columns, primaryKey) + .wasSuccessful(); + } + + protected void insertManyRows(List> rows) { + if (LOGGER.isWarnEnabled()) { + for (var row : rows) { + LOGGER.warn("Inserting row {}", row); + } + } + assertTableCommand(keyspaceName, tableName).templated().insertManyMap(rows).wasSuccessful(); + } + + protected static void addColumnsForTypes( + ApiColumnDefContainer columns, List types, String columnPrefix) { + addColumnsForTypes( + columns, + types, + type -> CqlIdentifier.fromInternal(columnPrefix + type.typeName().apiName())); + } + + protected static void addColumnsForTypes( + ApiColumnDefContainer columns, + List types, + Function nameMapper) { + types.forEach( + type -> { + columns.put(new ApiColumnDef(nameMapper.apply(type), type)); + }); + } + + protected void addNonPKColValuesWithNull(Map row, int nullIndex) { + + var i = 0; + for (var apiColumnDef : nonPkColumns.values()) { + row.put(fieldName(apiColumnDef), i == nullIndex ? null : columnValue(apiColumnDef)); + i++; + } + } + + protected List> createRowForEachNonPkColumnWithNull( + Function pkValueSupplier) { + var rows = new ArrayList>(); + + // and a row with all the columns set + var row = new HashMap(); + row.put(fieldName(primaryKey), pkValueSupplier.apply(primaryKey)); + addNonPKColValuesWithNull(row, -1); + rows.add(row); + + var i = 0; + for (var apiColumnDef : nonPkColumns.values()) { + row = new HashMap(); + row.put(fieldName(primaryKey), pkValueSupplier.apply(primaryKey)); + addNonPKColValuesWithNull(row, i++); + rows.add(row); + } + + return rows; + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java new file mode 100644 index 0000000000..aae4db7782 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java @@ -0,0 +1,94 @@ +package io.stargate.sgv2.jsonapi.api.v1.util.scenarios; + +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; +import static io.stargate.sgv2.jsonapi.api.v1.util.TemplateRunner.asJSON; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import io.stargate.sgv2.jsonapi.fixtures.data.DefaultData; +import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; +import io.stargate.sgv2.jsonapi.service.schema.tables.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Creates a table with: - PK of the single field column {@link TestDataScenario#ID_COL} - a column + * for each scalar type in {@link ApiDataTypesForTesting#ALL_SCALAR_TYPES_FOR_CREATE} - the non PK + * columns are named with the prefix {@link #COL_NAME_PREFIX} then the data type name - gets data + * from the {@link DefaultData} class - inserts row with the PK prefix "row" - the row "row-1" (-1) + * has a value for every column - the row "row-0" and beyond have a null for a single column in each + * row, one column at a time - call {@link #columnForDatatype(ApiDataType)} to get the column for a + * specific data type + */ +public class VectorDimension5TableScenario extends TestDataScenario { + + public static final ApiColumnDef CONTENT_COL = + new ApiColumnDef(CqlIdentifier.fromCql("content"), ApiDataTypeDefs.TEXT); + public static final ApiColumnDef INDEXED_VECTOR_COL = + new ApiColumnDef(CqlIdentifier.fromCql("indexed_vector"), ApiVectorType.from(5)); + public static final ApiColumnDef UNINDEXED_VECTOR_COL = + new ApiColumnDef(CqlIdentifier.fromCql("unindexed_vector"), ApiVectorType.from(5)); + + public static final ArrayNode KNOWN_VECTOR = JsonNodeFactory.instance.arrayNode(5); + public static final Map KNOWN_VECTOR_ROW = new LinkedHashMap<>(); + public static final String KNOWN_VECTOR_ROW_JSON; + + static { + for (int i = 0; i < 5; i++) { + KNOWN_VECTOR.add(1.0f); + } + + KNOWN_VECTOR_ROW.put(fieldName(ID_COL), "row-1"); + KNOWN_VECTOR_ROW.put( + fieldName(CONTENT_COL), "This is the content for row-1 - all 1.0 vectors "); + KNOWN_VECTOR_ROW.put(fieldName(INDEXED_VECTOR_COL), KNOWN_VECTOR); + KNOWN_VECTOR_ROW.put(fieldName(UNINDEXED_VECTOR_COL), KNOWN_VECTOR); + + KNOWN_VECTOR_ROW_JSON = asJSON(KNOWN_VECTOR_ROW); + } + + public VectorDimension5TableScenario(String keyspaceName, String tableName) { + super(keyspaceName, tableName, ID_COL, createColumns(), new DefaultData()); + } + + private static ApiColumnDefContainer createColumns() { + + var columns = new ApiColumnDefContainer(); + columns.put(ID_COL); + columns.put(CONTENT_COL); + columns.put(INDEXED_VECTOR_COL); + columns.put(UNINDEXED_VECTOR_COL); + return columns; + } + + @Override + protected void createIndexes() { + assertTableCommand(keyspaceName, tableName) + .templated() + .createVectorIndex( + "idx_" + tableName + "_" + fieldName(INDEXED_VECTOR_COL), fieldName(INDEXED_VECTOR_COL)) + .wasSuccessful(); + } + + @Override + protected void insertRows() { + + var rows = new ArrayList>(); + for (int i = 0; i < 20; i++) { + var row = new HashMap(); + row.put(fieldName(ID_COL), "row" + i); + row.put(fieldName(CONTENT_COL), "This is the content for row " + i); + var vector = columnValue(INDEXED_VECTOR_COL); + row.put(fieldName(INDEXED_VECTOR_COL), vector); + row.put(fieldName(UNINDEXED_VECTOR_COL), vector); + rows.add(row); + } + + rows.add(KNOWN_VECTOR_ROW); + + insertManyRows(rows); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java index 5325505cbc..990f6a3553 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java @@ -2,13 +2,17 @@ import com.datastax.oss.driver.api.core.type.DataType; import com.datastax.oss.driver.api.core.type.DataTypes; +import com.datastax.oss.driver.api.core.type.VectorType; +import com.datastax.oss.driver.internal.core.type.DefaultVectorType; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.JsonNodeFactory; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.EJSONWrapper; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonLiteral; import io.stargate.sgv2.jsonapi.fixtures.CqlFixture; +import io.stargate.sgv2.jsonapi.service.cqldriver.override.ExtendedVectorType; import io.stargate.sgv2.jsonapi.service.shredding.tables.RowShredder; import java.math.BigDecimal; +import java.util.Random; /** * Implementations fo the {@link FixtureData} interface that provide test data for use by the {@link @@ -22,6 +26,8 @@ */ public class DefaultData implements FixtureData { + private static final Random RANDOM = new Random(); + @Override public JsonLiteral fromJSON(DataType type) { // We want to get the value type that the RowShredder will generate, so just call it directly @@ -63,15 +69,30 @@ protected JsonNode getJsonNode(DataType type) { } else if (DataTypes.TIME.equals(type)) { return JsonNodeFactory.instance.textNode("13:13:04.010"); } else if (DataTypes.TIMESTAMP.equals(type)) { - return JsonNodeFactory.instance.textNode("1975-04-03T13:13:04Z"); + return JsonNodeFactory.instance.textNode("1970-02-01T13:13:04Z"); } else if (DataTypes.TINYINT.equals(type)) { return JsonNodeFactory.instance.numberNode(Byte.valueOf("1")); } else if (DataTypes.UUID.equals(type)) { return JsonNodeFactory.instance.textNode("550e8400-e29b-41d4-a716-446655440000"); } else if (DataTypes.VARINT.equals(type)) { return JsonNodeFactory.instance.numberNode(BigDecimal.valueOf(1L)); + } else if (type.getClass().getName().startsWith(DefaultVectorType.VECTOR_CLASS_NAME)) { + // See Driver class DataTypes#custom() for the above check + return vectorNode((VectorType) type); + } else if (type.getClass().getName().startsWith(ExtendedVectorType.class.getName())) { + // See Driver class DataTypes#custom() for the above check + return vectorNode((VectorType) type); + } + throw new UnsupportedOperationException( + "Unknown type: %s className %s".formatted(type, type.getClass().getName())); + } + + private JsonNode vectorNode(VectorType vt) { + var arrayNode = JsonNodeFactory.instance.arrayNode(vt.getDimensions()); + for (int i = 0; i < vt.getDimensions(); i++) { + arrayNode.add(RANDOM.nextFloat()); } - throw new UnsupportedOperationException("Unknown type: " + type); + return arrayNode; } /** Override so the test description gets a simple name */ diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java index 3df2502cce..c07011529e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java @@ -3,6 +3,8 @@ import com.datastax.oss.driver.api.core.type.DataType; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonLiteral; import io.stargate.sgv2.jsonapi.fixtures.CqlFixture; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataType; import java.util.List; /** @@ -29,6 +31,14 @@ public interface FixtureData { /** Data for types that are not supported by the API. */ List UNSUPPORTED_TYPES = List.of(new UnsupportedTypesData()); + default JsonLiteral fromJSON(ApiColumnDef columnDef) { + return fromJSON(columnDef.type()); + } + + default JsonLiteral fromJSON(ApiDataType apiDataType) { + return fromJSON(apiDataType.cqlType()); + } + /** * Returns a Java value that we want we would have read from Jackson. * From 5f5d809e5fddbbb78c9346621853a5e7f18de702 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Fri, 1 Nov 2024 10:43:42 +1300 Subject: [PATCH 15/18] fmt fix --- .../jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java index 1f97b78284..dbbab78f04 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java @@ -60,7 +60,8 @@ public void findUnindexedVector(Command.CommandName commandName) { SortException.Code.CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, SortException.class, "And has indexes on the vector columns: %s.\nThe command attempted to sort vector columns: %s" - .formatted(SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL))); } From ee1181de22a67a38979d41024f22feddf802fe11 Mon Sep 17 00:00:00 2001 From: Yuqi Du Date: Thu, 31 Oct 2024 15:19:15 -0700 Subject: [PATCH 16/18] error typo, filter on complex type --- .../jsonapi/exception/FilterException.java | 1 + .../tables/WhereCQLClauseAnalyzer.java | 30 +++++++++-- src/main/resources/errors.yaml | 13 ++++- .../testdata/LogicalExpressionTestData.java | 37 ++++++++++++++ .../testdata/TableMetadataTestData.java | 33 ++++++++++-- .../fixtures/testdata/TestDataNames.java | 36 ++++++++++--- .../testdata/WhereAnalyzerTestData.java | 8 +++ .../tables/SelectWhereAnalyzerTest.java | 51 +++++++++++++++---- 8 files changed, 184 insertions(+), 25 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/FilterException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/FilterException.java index d7bdcaac8e..bade1d8096 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/FilterException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/FilterException.java @@ -19,6 +19,7 @@ public enum Code implements ErrorCode { UNSUPPORTED_COLUMN_TYPES, COMPARISON_FILTER_AGAINST_DURATION, FILTER_REQUIRED_FOR_UPDATE_DELETE, + FILTER_ON_COMPLEX_COLUMNS, NON_PRIMARY_KEY_FILTER_FOR_UPDATE_DELETE, INCOMPLETE_PRIMARY_KEY_FILTER, FULL_PRIMARY_KEY_REQUIRED_FOR_UPDATE_DELETE; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java index 578c7b5576..18317da288 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WhereCQLClauseAnalyzer.java @@ -51,7 +51,9 @@ AnalysisStrategy getStrategy(WhereCQLClauseAnalyzer analyzer) { case SELECT -> new AnalysisStrategy( List.of( - analyzer::checkAllColumnsExist, analyzer::checkComparisonFilterAgainstDuration), + analyzer::checkAllColumnsExist, + analyzer::checkComparisonFilterAgainstDuration, + analyzer::checkFilteringOnComplexColumns), List.of( analyzer::warnMissingIndexOnScalar, analyzer::warnNotEqUnsupportedByIndexing, @@ -64,7 +66,8 @@ AnalysisStrategy getStrategy(WhereCQLClauseAnalyzer analyzer) { analyzer::checkNoFilters, analyzer::checkAllColumnsExist, analyzer::checkNonPrimaryKeyFilters, - analyzer::checkFullPrimaryKey), + analyzer::checkFullPrimaryKey, + analyzer::checkFilteringOnComplexColumns), List.of()); case DELETE_MANY -> new AnalysisStrategy( @@ -72,7 +75,8 @@ AnalysisStrategy getStrategy(WhereCQLClauseAnalyzer analyzer) { analyzer::checkNoFilters, analyzer::checkAllColumnsExist, analyzer::checkNonPrimaryKeyFilters, - analyzer::checkValidPrimaryKey), + analyzer::checkValidPrimaryKey, + analyzer::checkFilteringOnComplexColumns), List.of()); }; } @@ -205,6 +209,25 @@ private void checkAllColumnsExist(Map identifierToFi } } + /** In first tables feature preview, only scalar column types are supported for filter */ + private void checkFilteringOnComplexColumns(Map identifierToFilter) { + + var filterOnComplexColumns = + identifierToFilter.keySet().stream() + .filter(identifier -> !apiTableDef.allColumns().get(identifier).type().isPrimitive()) + .toList(); + + if (!filterOnComplexColumns.isEmpty()) { + throw FilterException.Code.FILTER_ON_COMPLEX_COLUMNS.get( + errVars( + tableSchemaObject, + map -> { + map.put("allColumns", errFmtColumnMetadata(tableMetadata.getColumns().values())); + map.put("complexColumns", errFmtCqlIdentifier(filterOnComplexColumns)); + })); + } + } + /** * Check if all primary keys components (partition keys, clustering keys) are specified in filter. * @@ -314,7 +337,6 @@ private Optional warnMissingIndexOnScalar( .toList() : identifierToFilter.keySet().stream().toList(); - // TODO: better check if identifier is not found in all columns var scalarTypeFilters = filtersToCheck.stream() .filter(identifier -> apiTableDef.allColumns().get(identifier).type().isPrimitive()) diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index acef21dde0..c9a4f7fc5b 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -164,6 +164,17 @@ request-errors: Resend the request using only supported column types. + - scope: FILTER + code: FILTER_ON_COMPLEX_COLUMNS + title: Unsupported complex columns for filter + body: |- + Complex type is not supported for filter. + + The table ${keyspace}.${table} defines the columns: ${allColumns}. + The request included the following columns that have unsupported complex columns: ${complexColumns}. + + Resend the request using only supported column types. + - scope: FILTER code: COMPARISON_FILTER_AGAINST_DURATION title: Cannot perform comparison filter against duration data type @@ -304,7 +315,7 @@ request-errors: code: ZERO_FILTER_OPERATIONS title: Zero operations provided in query filter body: |- - Zero filters were provided in the filer for this query. + Zero filters were provided in the filter for this query. Providing zero filters will return all rows in the table, which may have poor performance when the table is large. For the best performance, include one or more filters using the primary key or indexes. diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/LogicalExpressionTestData.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/LogicalExpressionTestData.java index 3df9ab794f..197e6e396a 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/LogicalExpressionTestData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/LogicalExpressionTestData.java @@ -244,6 +244,23 @@ public static TableFilter filter( return new TextTableFilter(column.asInternal(), operator, (String) value); } + // Sample Collection type values + // TODO, note tables feature does not support complex type now, the unit tests just mimic an + // invalid filter + // usage against complex datatype columns + if (type.equals(DataTypes.setOf(DataTypes.TEXT))) { + return new TextTableFilter(column.asInternal(), operator, (String) value); + } + if (type.equals(DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT))) { + return new TextTableFilter(column.asInternal(), operator, (String) value); + } + if (type.equals(DataTypes.listOf(DataTypes.TEXT))) { + return new TextTableFilter(column.asInternal(), operator, (String) value); + } + if (type.equals(DataTypes.vectorOf(DataTypes.FLOAT, 3))) { + return new TextTableFilter(column.asInternal(), operator, (String) value); + } + throw new IllegalArgumentException("Unsupported type"); } @@ -306,6 +323,26 @@ public static Object value(DataType type) { return "123e4567-e89b-12d3-a456-426655440000"; // Sample TIMEUUID } + if (type.equals(DataTypes.TIMEUUID)) { + return "123e4567-e89b-12d3-a456-426655440000"; // Sample TIMEUUID + } + + // Sample Collection type values + // TODO, note tables feature does not support complex type now, the unit tests just mimic an + // invalid filter + // usage against complex datatype columns + if (type.equals(DataTypes.setOf(DataTypes.TEXT))) { + return null; + } + if (type.equals(DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT))) { + return null; + } + if (type.equals(DataTypes.listOf(DataTypes.TEXT))) { + return null; + } + if (type.equals(DataTypes.vectorOf(DataTypes.FLOAT, 3))) { + return null; + } throw new IllegalArgumentException("Unsupported type"); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TableMetadataTestData.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TableMetadataTestData.java index 50c0f3323c..26f2de602e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TableMetadataTestData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TableMetadataTestData.java @@ -123,7 +123,7 @@ public TableMetadata tableAllDatatypesIndexed() { Map.entry(names.COL_CLUSTERING_KEY_1, DataTypes.TEXT), Map.entry(names.COL_CLUSTERING_KEY_2, DataTypes.TEXT), Map.entry(names.COL_CLUSTERING_KEY_3, DataTypes.TEXT), - // supported datatype columns + // primitive datatype columns Map.entry(names.CQL_TEXT_COLUMN, DataTypes.TEXT), Map.entry(names.CQL_ASCII_COLUMN, DataTypes.ASCII), Map.entry(names.CQL_BLOB_COLUMN, DataTypes.BLOB), @@ -142,10 +142,15 @@ public TableMetadata tableAllDatatypesIndexed() { Map.entry(names.CQL_TIME_COLUMN, DataTypes.TIME), Map.entry(names.CQL_INET_COLUMN, DataTypes.INET), Map.entry(names.CQL_UUID_COLUMN, DataTypes.UUID), - Map.entry(names.CQL_TIMEUUID_COLUMN, DataTypes.TIMEUUID)), + Map.entry(names.CQL_TIMEUUID_COLUMN, DataTypes.TIMEUUID), + // collection datatype columns + Map.entry(names.CQL_SET_COLUMN, DataTypes.setOf(DataTypes.TEXT)), + Map.entry(names.CQL_MAP_COLUMN, DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT)), + Map.entry(names.CQL_LIST_COLUMN, DataTypes.listOf(DataTypes.TEXT)), + Map.entry(names.CQL_VECTOR_COLUMN, DataTypes.vectorOf(DataTypes.FLOAT, 3))), ImmutableMap.of(), ImmutableMap.ofEntries( - // index all datatypes column in the table,(Blob,Duration can NOT be indexed) + // index scalar datatypes column in the table,(Blob,Duration can NOT be indexed) Map.entry( names.CQL_TEXT_COLUMN_INDEX, indexMetadata(names.CQL_TEXT_COLUMN_INDEX, names.CQL_TEXT_COLUMN)), @@ -196,7 +201,20 @@ public TableMetadata tableAllDatatypesIndexed() { indexMetadata(names.CQL_UUID_COLUMN_INDEX, names.CQL_UUID_COLUMN)), Map.entry( names.CQL_TIMEUUID_COLUMN_INDEX, - indexMetadata(names.CQL_TIMEUUID_COLUMN_INDEX, names.CQL_TIMEUUID_COLUMN)))); + indexMetadata(names.CQL_TIMEUUID_COLUMN_INDEX, names.CQL_TIMEUUID_COLUMN)), + // index collection datatypes column in the table + Map.entry( + names.CQL_SET_COLUMN_INDEX, + indexMetadata(names.CQL_SET_COLUMN_INDEX, names.CQL_SET_COLUMN)), + Map.entry( + names.CQL_LIST_COLUMN, + indexMetadata(names.CQL_LIST_COLUMN_INDEX, names.CQL_LIST_COLUMN)), + Map.entry( + names.CQL_MAP_COLUMN_INDEX, + indexMetadata(names.CQL_MAP_COLUMN_INDEX, names.CQL_MAP_COLUMN)), + Map.entry( + names.CQL_VECTOR_COLUMN_INDEX, + indexMetadata(names.CQL_VECTOR_COLUMN_INDEX, names.CQL_VECTOR_COLUMN)))); } public TableMetadata tableAllDatatypesNotIndexed() { @@ -241,7 +259,12 @@ public TableMetadata tableAllDatatypesNotIndexed() { Map.entry(names.CQL_TIME_COLUMN, DataTypes.TIME), Map.entry(names.CQL_INET_COLUMN, DataTypes.INET), Map.entry(names.CQL_UUID_COLUMN, DataTypes.UUID), - Map.entry(names.CQL_TIMEUUID_COLUMN, DataTypes.TIMEUUID)), + Map.entry(names.CQL_TIMEUUID_COLUMN, DataTypes.TIMEUUID), + // collection datatype columns + Map.entry(names.CQL_SET_COLUMN, DataTypes.setOf(DataTypes.TEXT)), + Map.entry(names.CQL_MAP_COLUMN, DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT)), + Map.entry(names.CQL_LIST_COLUMN, DataTypes.listOf(DataTypes.TEXT)), + Map.entry(names.CQL_VECTOR_COLUMN, DataTypes.vectorOf(DataTypes.FLOAT, 3))), ImmutableMap.of(), ImmutableMap.of()); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TestDataNames.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TestDataNames.java index 7c61f593d8..b466aabaa4 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TestDataNames.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/TestDataNames.java @@ -37,11 +37,12 @@ public class TestDataNames { public final CqlIdentifier COL_UNKNOWN_1 = CqlIdentifier.fromInternal("unknown-1-" + System.currentTimeMillis()); - // All column datatypes + // ================================================================================================================== + // Primitive(Scalar) dataTypes + // ================================================================================================================== + public final CqlIdentifier CQL_TEXT_COLUMN = CqlIdentifier.fromInternal("text-column-" + System.currentTimeMillis()); - // TODO public final CqlIdentifier CQL_UUID_COLUMN = - // CqlIdentifier.fromInternal("uuid-column-" + System.currentTimeMillis()); public final CqlIdentifier CQL_ASCII_COLUMN = CqlIdentifier.fromInternal("ascii-column-" + System.currentTimeMillis()); public final CqlIdentifier CQL_BLOB_COLUMN = @@ -79,7 +80,7 @@ public class TestDataNames { public final CqlIdentifier CQL_TIMEUUID_COLUMN = CqlIdentifier.fromInternal("timeuuid-column-" + System.currentTimeMillis()); - public final List ALL_CQL_DATATYPE_COLUMNS = + public final List ALL_SCALAR_DATATYPE_COLUMNS = List.of( CQL_TEXT_COLUMN, CQL_ASCII_COLUMN, @@ -107,8 +108,6 @@ public class TestDataNames { // All column datatypes index name, (Blob,Duration can NOT be indexed) public final CqlIdentifier CQL_TEXT_COLUMN_INDEX = CqlIdentifier.fromInternal("text-column-index-" + System.currentTimeMillis()); - // TODO public final CqlIdentifier CQL_UUID_COLUMN_INDEX = - // CqlIdentifier.fromInternal("uuid-column-" + System.currentTimeMillis()); public final CqlIdentifier CQL_ASCII_COLUMN_INDEX = CqlIdentifier.fromInternal("ascii-column-index-" + System.currentTimeMillis()); public final CqlIdentifier CQL_BOOLEAN_COLUMN_INDEX = @@ -141,4 +140,29 @@ public class TestDataNames { CqlIdentifier.fromInternal("uuid-column-index-" + System.currentTimeMillis()); public final CqlIdentifier CQL_TIMEUUID_COLUMN_INDEX = CqlIdentifier.fromInternal("timeuuid-column-index-" + System.currentTimeMillis()); + + // ================================================================================================================== + // Collection(set, map, list, vector) dataTypes + // ================================================================================================================== + + public final CqlIdentifier CQL_MAP_COLUMN = + CqlIdentifier.fromInternal("map-column-" + System.currentTimeMillis()); + public final CqlIdentifier CQL_LIST_COLUMN = + CqlIdentifier.fromInternal("list-column-" + System.currentTimeMillis()); + public final CqlIdentifier CQL_SET_COLUMN = + CqlIdentifier.fromInternal("set-column-" + System.currentTimeMillis()); + public final CqlIdentifier CQL_VECTOR_COLUMN = + CqlIdentifier.fromInternal("vector-column-" + System.currentTimeMillis()); + + public final List ALL_COLLECTION_DATATYPE_COLUMNS = + List.of(CQL_MAP_COLUMN, CQL_LIST_COLUMN, CQL_SET_COLUMN, CQL_VECTOR_COLUMN); + + public final CqlIdentifier CQL_MAP_COLUMN_INDEX = + CqlIdentifier.fromInternal("map-column-index-" + System.currentTimeMillis()); + public final CqlIdentifier CQL_LIST_COLUMN_INDEX = + CqlIdentifier.fromInternal("list-column-index-" + System.currentTimeMillis()); + public final CqlIdentifier CQL_SET_COLUMN_INDEX = + CqlIdentifier.fromInternal("set-column-index-" + System.currentTimeMillis()); + public final CqlIdentifier CQL_VECTOR_COLUMN_INDEX = + CqlIdentifier.fromInternal("vector-column-index-" + System.currentTimeMillis()); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/WhereAnalyzerTestData.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/WhereAnalyzerTestData.java index c094c973ab..bf15f1c6b1 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/WhereAnalyzerTestData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/testdata/WhereAnalyzerTestData.java @@ -159,6 +159,14 @@ public WhereAnalyzerFixture assertExceptionOnUnknownColumns(CqlIdentifier... col return assertExceptionContains(warning); } + public WhereAnalyzerFixture assertExceptionOnComplexColumns(CqlIdentifier... columns) { + var identifiers = Arrays.stream(columns).sorted(CQL_IDENTIFIER_COMPARATOR).toList(); + var warning = + "The request included the following columns that have unsupported complex columns: %s." + .formatted(errFmtCqlIdentifier(identifiers)); + return assertExceptionContains(warning); + } + public WhereAnalyzerFixture assertExceptionOnDurationColumns(CqlIdentifier... columns) { var identifiers = Arrays.stream(columns).sorted(CQL_IDENTIFIER_COMPARATOR).toList(); var warning = diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java index ff9f1d51fe..4dede872b2 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java @@ -8,15 +8,19 @@ import io.stargate.sgv2.jsonapi.fixtures.testdata.TestData; import io.stargate.sgv2.jsonapi.fixtures.testdata.TestDataNames; import io.stargate.sgv2.jsonapi.service.operation.filters.table.NativeTypeTableFilter; +import java.util.stream.Stream; import org.junit.jupiter.api.Nested; 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 the {@link WhereCQLClauseAnalyzer}. Focus on Select Statement type */ public class SelectWhereAnalyzerTest { private static final TestData TEST_DATA = new TestData(); - private TestDataNames names() { + private static TestDataNames names() { return TEST_DATA.names; } @@ -456,7 +460,7 @@ class DataTypesSanityCheck { @Test public void eq_indexed_column() { - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN) || cqlDatatypeColumn.equals(names().CQL_DURATION_COLUMN)) { continue; @@ -488,7 +492,7 @@ public void eq_indexed_column() { @Test public void eq_not_indexed_column() { - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN)) { continue; } @@ -518,7 +522,7 @@ public void eq_not_indexed_column() { public void ne_indexed_column() { // If column is on SAI index, perform $ne on it. // -> For TEXT, ASCII, BOOLEAN, DURATION, BLOB. ALLOW FILTERING is needed. - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN)) { continue; } @@ -569,7 +573,7 @@ public void ne_indexed_column() { @Test public void ne_not_indexed_column() { - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN)) { continue; } @@ -597,7 +601,7 @@ public void ne_not_indexed_column() { @Test public void in_indexed_column() { - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN)) { continue; } @@ -629,7 +633,7 @@ public void in_indexed_column() { @Test public void in_not_indexed_column() { - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN)) { continue; } @@ -657,7 +661,7 @@ public void in_not_indexed_column() { @Test public void comparison_api_filter_indexed_column() { - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN)) { continue; } @@ -705,7 +709,7 @@ public void comparison_api_filter_indexed_column() { @Test public void comparison_api_filter_not_indexed_column() { - for (CqlIdentifier cqlDatatypeColumn : names().ALL_CQL_DATATYPE_COLUMNS) { + for (CqlIdentifier cqlDatatypeColumn : names().ALL_SCALAR_DATATYPE_COLUMNS) { if (cqlDatatypeColumn.equals(names().CQL_BLOB_COLUMN)) { continue; } @@ -725,4 +729,33 @@ public void comparison_api_filter_not_indexed_column() { } } } + + @Nested + class ComplexDataType { + + // ================================================================================================================== + // Currently, only PrimitiveApiDataType(Scalar) is supported for table filter + // ================================================================================================================== + + private static Stream complexDataTypes() { + return names().ALL_COLLECTION_DATATYPE_COLUMNS.stream().map((Arguments::of)); + } + + @ParameterizedTest + @MethodSource("complexDataTypes") + public void eq_complex_column(CqlIdentifier complexColumnIdentifier) { + var fixture = + TEST_DATA + .whereAnalyzer() + .tableAllColumnDatatypesIndexed( + "$eq_on_%s".formatted(complexColumnIdentifier.asInternal()), + WhereCQLClauseAnalyzer.StatementType.SELECT); + fixture + .expression() + .eqOn(complexColumnIdentifier) + .analyzeThrows(FilterException.class) + .assertFilterExceptionCode(FilterException.Code.FILTER_ON_COMPLEX_COLUMNS) + .assertExceptionOnComplexColumns(complexColumnIdentifier); + } + } } From 683cf511ae3877a4910c08e11f28e0007ce08008 Mon Sep 17 00:00:00 2001 From: Yuqi Du Date: Thu, 31 Oct 2024 15:48:56 -0700 Subject: [PATCH 17/18] CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND --- .../sgv2/jsonapi/exception/SortException.java | 5 +-- .../resolver/DeleteOneCommandResolver.java | 9 ++++++ .../resolver/UpdateOneCommandResolver.java | 9 ++++++ src/main/resources/errors.yaml | 7 +++++ .../tables/SelectWhereAnalyzerTest.java | 31 ++++++++++++++++++- 5 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java index e63d38e634..6e65b5593d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java @@ -14,10 +14,11 @@ public SortException(ErrorInstance errorInstance) { } public enum Code implements ErrorCode { + CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, + CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND, CANNOT_SORT_UNKNOWN_COLUMNS, - CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, - CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, + CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, MORE_THAN_ONE_VECTOR_SORT, ; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java index 56d4e50047..a7cf9ccb90 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.service.resolver; +import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.*; + import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; @@ -9,6 +11,7 @@ import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonApiMetricsConfig; import io.stargate.sgv2.jsonapi.config.DebugModeConfig; import io.stargate.sgv2.jsonapi.config.OperationsConfig; +import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.*; import io.stargate.sgv2.jsonapi.service.operation.collections.CollectionReadType; @@ -66,6 +69,12 @@ public DeleteOneCommandResolver( public Operation resolveTableCommand( CommandContext ctx, DeleteOneCommand command) { + // Sort clause is not supported for table deleteOne command. + if (command.sortClause() != null) { + throw SortException.Code.CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND.get( + errVars(ctx.schemaObject(), map -> {})); + } + var builder = new DeleteAttemptBuilder<>(ctx.schemaObject(), true); var where = diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java index 37c62fbc2c..524cf8c066 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.service.resolver; +import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.errVars; + import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; @@ -9,6 +11,7 @@ import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonApiMetricsConfig; import io.stargate.sgv2.jsonapi.config.DebugModeConfig; import io.stargate.sgv2.jsonapi.config.OperationsConfig; +import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.embedding.DataVectorizerService; import io.stargate.sgv2.jsonapi.service.operation.*; @@ -80,6 +83,12 @@ public Class getCommandClass() { public Operation resolveTableCommand( CommandContext ctx, UpdateOneCommand command) { + // Sort clause is not supported for table updateOne command. + if (command.sortClause() != null) { + throw SortException.Code.CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND.get( + errVars(ctx.schemaObject(), map -> {})); + } + var builder = new UpdateAttemptBuilder<>(ctx.schemaObject()); var where = diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index 9238db1489..4e3f9f50d1 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -745,6 +745,13 @@ request-errors: Resend the command using only vector columns that have been indexed. + - scope: SORT + code: CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND + title: Sort is not support for table DELETE and UPDATE command + body: |- + Table DELETE and UPDATE command do not support sort. + + # ================================================================================================================ # ================================================================================================================ # Server Errors diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java index f653fc2a20..1910e05350 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/tables/SelectWhereAnalyzerTest.java @@ -453,7 +453,7 @@ public void gtOneDurationFullPk() { class DataTypesSanityCheck { private static Stream allScalarDatatype() { - return names().ALL_CQL_DATATYPE_COLUMNS.stream().map(Arguments::of); + return names().ALL_SCALAR_DATATYPE_COLUMNS.stream().map(Arguments::of); } private static final Set allow_filtering_needed_for_comparison = @@ -821,4 +821,33 @@ public void comparison_api_filter_not_indexed_column(CqlIdentifier cqlDatatypeCo .assertWarnOnUnindexedColumns(cqlDatatypeColumn); } } + + @Nested + class ComplexDataType { + + // ================================================================================================================== + // Currently, only PrimitiveApiDataType(Scalar) is supported for table filter + // ================================================================================================================== + + private static Stream complexDataTypes() { + return names().ALL_COLLECTION_DATATYPE_COLUMNS.stream().map((Arguments::of)); + } + + @ParameterizedTest + @MethodSource("complexDataTypes") + public void eq_complex_column(CqlIdentifier complexColumnIdentifier) { + var fixture = + TEST_DATA + .whereAnalyzer() + .tableAllColumnDatatypesIndexed( + "$eq_on_%s".formatted(complexColumnIdentifier.asInternal()), + WhereCQLClauseAnalyzer.StatementType.SELECT); + fixture + .expression() + .eqOn(complexColumnIdentifier) + .analyzeThrows(FilterException.class) + .assertFilterExceptionCode(FilterException.Code.FILTER_ON_COMPLEX_COLUMNS) + .assertExceptionOnComplexColumns(complexColumnIdentifier); + } + } } From 824ee65f5790d848056f0ccd146b401de34f7ce2 Mon Sep 17 00:00:00 2001 From: Yuqi Du Date: Thu, 31 Oct 2024 20:40:57 -0700 Subject: [PATCH 18/18] CANNOT_SORT_TABLE_DELETE_COMMAND CANNOT_SORT_TABLE_UPDATE_COMMAND --- .../sgv2/jsonapi/exception/SortException.java | 3 ++- .../service/resolver/DeleteOneCommandResolver.java | 2 +- .../service/resolver/UpdateOneCommandResolver.java | 2 +- src/main/resources/errors.yaml | 12 +++++++++--- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java index 6e65b5593d..953cb98fc7 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java @@ -15,7 +15,8 @@ public SortException(ErrorInstance errorInstance) { public enum Code implements ErrorCode { CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, - CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND, + CANNOT_SORT_TABLE_UPDATE_COMMAND, + CANNOT_SORT_TABLE_DELETE_COMMAND, CANNOT_SORT_UNKNOWN_COLUMNS, CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java index a7cf9ccb90..aa5f127453 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java @@ -71,7 +71,7 @@ public Operation resolveTableCommand( // Sort clause is not supported for table deleteOne command. if (command.sortClause() != null) { - throw SortException.Code.CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND.get( + throw SortException.Code.CANNOT_SORT_TABLE_DELETE_COMMAND.get( errVars(ctx.schemaObject(), map -> {})); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java index 524cf8c066..d02c8d6f0f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java @@ -85,7 +85,7 @@ public Operation resolveTableCommand( // Sort clause is not supported for table updateOne command. if (command.sortClause() != null) { - throw SortException.Code.CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND.get( + throw SortException.Code.CANNOT_SORT_TABLE_UPDATE_COMMAND.get( errVars(ctx.schemaObject(), map -> {})); } diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index 4e3f9f50d1..64b51b0c92 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -746,10 +746,16 @@ request-errors: Resend the command using only vector columns that have been indexed. - scope: SORT - code: CANNOT_SORT_TABLE_DELETE_UPDATE_COMMAND - title: Sort is not support for table DELETE and UPDATE command + code: CANNOT_SORT_TABLE_DELETE_COMMAND + title: Sort is not support for table DELETE command body: |- - Table DELETE and UPDATE command do not support sort. + Table UPDATE command does not support sort. + + - scope: SORT + code: CANNOT_SORT_TABLE_UPDATE_COMMAND + title: Sort is not support for table UPDATE command + body: |- + Table DELETE command does not support sort. # ================================================================================================================