Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List tables command #1510

Merged
merged 45 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
42d0fb5
Made the SchemaObject store and return List of VectorConfig. This is …
maheshrajamani Oct 5, 2024
c016640
Changes for TableSchemaObject to deserialize vector and vectorize inf…
maheshrajamani Oct 7, 2024
d590679
Changed the vector config to unmodifiable list
maheshrajamani Oct 7, 2024
aa656ef
Fixed the comments
maheshrajamani Oct 7, 2024
02bc38e
Fixed the comments
maheshrajamani Oct 7, 2024
24da975
Use $vectorize field name from constant `DocumentConstants.Fields.VEC…
maheshrajamani Oct 7, 2024
ecabca3
Use $vectorize field name from constant `DocumentConstants.Fields.VEC…
maheshrajamani Oct 7, 2024
bc30acb
Use $vectorize field name from constant `DocumentConstants.Fields.VEC…
maheshrajamani Oct 7, 2024
f0cb7bd
Backup commit for list tables
maheshrajamani Oct 7, 2024
550ce05
Updated the code based on review.
maheshrajamani Oct 7, 2024
0a24bd0
Merge branch 'main' into schema-outject-multiple-vectorize
maheshrajamani Oct 7, 2024
5220f57
Merge branch 'main' of github.com:stargate/data-api into schema-outje…
maheshrajamani Oct 7, 2024
d94ce09
Merged the refactor code based on review.
maheshrajamani Oct 7, 2024
d47be3a
Working listTables
maheshrajamani Oct 7, 2024
2683c66
Fixed the vectorize config deserializer
maheshrajamani Oct 7, 2024
e7e92cb
Changes based on review
maheshrajamani Oct 7, 2024
78ba059
Merge branch 'schema-outject-multiple-vectorize' of github.com:starga…
maheshrajamani Oct 7, 2024
366a503
Merge branch 'main' into schema-outject-multiple-vectorize
maheshrajamani Oct 7, 2024
27f3bef
Resolve the merge compile error
maheshrajamani Oct 7, 2024
11b589e
Resolve the merge compile error
maheshrajamani Oct 7, 2024
6ad43dc
Resolve the merge compile error
maheshrajamani Oct 7, 2024
dd2f2cb
Formatted the file
maheshrajamani Oct 7, 2024
5e45661
Fix for IT
maheshrajamani Oct 7, 2024
7483d58
Resolved conflict after merge
maheshrajamani Oct 7, 2024
da04424
Added UNSUPPORTED type support
maheshrajamani Oct 8, 2024
d6cd08f
Made the SchemaObject store and return List of VectorConfig. This is …
maheshrajamani Oct 5, 2024
9f386d2
Backup commit for list tables
maheshrajamani Oct 7, 2024
fc48d0d
Updated the code based on review.
maheshrajamani Oct 7, 2024
e01992e
Working listTables
maheshrajamani Oct 7, 2024
df3bfd5
Added UNSUPPORTED type support
maheshrajamani Oct 8, 2024
9a2b136
Rebased to main
maheshrajamani Oct 8, 2024
7986177
Rebased to main
maheshrajamani Oct 8, 2024
27b7fa4
Merge branch 'list-tables-command' of github.com:stargate/data-api in…
maheshrajamani Oct 8, 2024
b3e7cb1
Removed unused method
maheshrajamani Oct 8, 2024
e42aaa2
Added comments
maheshrajamani Oct 8, 2024
e6096a9
Added tests
maheshrajamani Oct 8, 2024
3a49f2e
Formatted the files
maheshrajamani Oct 8, 2024
b98a216
Added vector service and primary key definition
maheshrajamani Oct 8, 2024
e173c80
Added serializer for Primary key ordering columns
maheshrajamani Oct 8, 2024
0ffba5f
Refactor as per the review
maheshrajamani Oct 8, 2024
8a150e8
Merge branch 'main' into list-tables-command
maheshrajamani Oct 8, 2024
f7103aa
Refactor as per the review comments
maheshrajamani Oct 8, 2024
57c3c69
Added test to confirm listTables doesn't return collections
maheshrajamani Oct 8, 2024
6d9de5a
Added comment as suggested.
maheshrajamani Oct 8, 2024
6b6ccc8
Added comment as suggested.
maheshrajamani Oct 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ enum CommandName {
FIND_ONE("findOne"),
INSERT_MANY("insertMany"),
INSERT_ONE("insertOne"),
LIST_TABLES("listTables"),
UPDATE_MANY("updateMany"),
UPDATE_ONE("updateOne"),
BEGIN_OFFLINE_SESSION("beginOfflineSession"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ public enum CommandStatus {
/** Status for reporting existing collections. */
@JsonProperty("collections")
EXISTING_COLLECTIONS,
/** Status for reporting existing collections. */
@JsonProperty("tables")
EXISTING_TABLES,

/**
* List of response entries, one for each document we tried to insert with {@code insertMany}
* command. Each entry has 2 mandatory fields: {@code _id} (document id), and {@code status} (one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateTableCommand;
import io.stargate.sgv2.jsonapi.api.model.command.impl.DropTableCommand;
import io.stargate.sgv2.jsonapi.api.model.command.impl.ListTablesCommand;

/** Interface for all commands executed against a keyspace. */
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.WRAPPER_OBJECT)
@JsonSubTypes({
@JsonSubTypes.Type(value = CreateTableCommand.class),
@JsonSubTypes.Type(value = DropTableCommand.class)
@JsonSubTypes.Type(value = DropTableCommand.class),
@JsonSubTypes.Type(value = ListTablesCommand.class),
})
public interface TableOnlyCommand extends KeyspaceCommand {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.stargate.sgv2.jsonapi.api.model.command.impl;

import com.fasterxml.jackson.annotation.JsonTypeName;
import io.stargate.sgv2.jsonapi.api.model.command.TableOnlyCommand;
import org.eclipse.microprofile.openapi.annotations.enums.SchemaType;
import org.eclipse.microprofile.openapi.annotations.media.Schema;

@Schema(description = "Command that lists all available tables in a namespace.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should say keyspace not namespace

@JsonTypeName("listTables")
public record ListTablesCommand(Options options) implements TableOnlyCommand {
public record Options(
@Schema(
description = "include table properties.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "include" to "Include" also this is not the most descriptive.

is this more truthful ? "When True the schema for the table is returned, when false just the table name"

type = SchemaType.BOOLEAN,
implementation = Boolean.class)
boolean explain) {}

/** {@inheritDoc} */
@Override
public CommandName commandName() {
return CommandName.LIST_TABLES;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package io.stargate.sgv2.jsonapi.api.model.command.serializer;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
import io.stargate.sgv2.jsonapi.api.model.command.table.definition.datatype.ColumnType;
import io.stargate.sgv2.jsonapi.api.model.command.table.definition.datatype.ComplexTypes;
import java.io.IOException;

/**
* Custom serializer to encode the column type to the JSON payload This is required because
* composite and custom column types may need additional properties to be serialized
*/
public class ColumnDefinitionSerializer extends JsonSerializer<ColumnType> {

@Override
public void serialize(
ColumnType columnType, JsonGenerator jsonGenerator, SerializerProvider serializerProvider)
throws IOException {
jsonGenerator.writeStartObject();
jsonGenerator.writeStringField("type", columnType.getApiName());
if (columnType instanceof ComplexTypes.MapType mt) {
jsonGenerator.writeStringField("keyType", mt.keyTypeName());
jsonGenerator.writeStringField("valueType", mt.valueTypeName());
} else if (columnType instanceof ComplexTypes.ListType lt) {
jsonGenerator.writeStringField("valueType", lt.valueTypeName());
} else if (columnType instanceof ComplexTypes.SetType st) {
jsonGenerator.writeStringField("valueType", st.valueTypeName());
} else if (columnType instanceof ComplexTypes.VectorType vt) {
jsonGenerator.writeNumberField("dimension", vt.getDimension());
if (vt.getVectorConfig() != null)
jsonGenerator.writeObjectField("service", vt.getVectorConfig());
} else if (columnType instanceof ComplexTypes.UnsupportedType ut) {
jsonGenerator.writeObjectField(
"apiSupport", new ApiSupport(false, false, false, ut.cqlFormat()));
}
jsonGenerator.writeEndObject();
}

public record ApiSupport(
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
boolean createTable, boolean insert, boolean read, String cqlDefinition) {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package io.stargate.sgv2.jsonapi.api.model.command.serializer;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
import io.stargate.sgv2.jsonapi.api.model.command.table.definition.PrimaryKey;
import java.io.IOException;

/**
* Custom serializer to encode the column type to the JSON payload This is required because
* composite and custom column types may need additional properties to be serialized
*/
public class OrderingKeysSerializer extends JsonSerializer<PrimaryKey.OrderingKey[]> {

@Override
public void serialize(
PrimaryKey.OrderingKey[] orderingKeys,
JsonGenerator jsonGenerator,
SerializerProvider serializerProvider)
throws IOException {
jsonGenerator.writeStartObject();
if (orderingKeys != null) {
for (PrimaryKey.OrderingKey orderingKey : orderingKeys) {
jsonGenerator.writeNumberField(
orderingKey.column(), orderingKey.order() == PrimaryKey.OrderingKey.Order.ASC ? 1 : -1);
}
}
jsonGenerator.writeEndObject();
}

/**
* This is used when a unsupported type column is present in a table. How to use this class will
* evolve as different unsupported types are analyzed.
*
* @param createTable
* @param insert
* @param read
* @param cqlDefinition
*/
private record ApiSupport(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this record here and in ColumnDefinitionSerializer ?

Copy link
Contributor Author

@maheshrajamani maheshrajamani Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a comment in there, this is just a place holder object to generate apiSupport for unsupported columns, This needs to be analyzed for what operation can be supported for unsupported types.

boolean createTable, boolean insert, boolean read, String cqlDefinition) {}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package io.stargate.sgv2.jsonapi.api.model.command.table.definition;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import io.stargate.sgv2.jsonapi.api.model.command.deserializers.PrimaryKeyDeserializer;
import io.stargate.sgv2.jsonapi.api.model.command.serializer.OrderingKeysSerializer;
import jakarta.annotation.Nullable;
import jakarta.validation.constraints.NotNull;
import org.eclipse.microprofile.openapi.annotations.enums.SchemaType;
Expand All @@ -16,9 +19,15 @@
// implementation = Object.class,
// description = "Represents the table primary key")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove the commented out lines now ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually these are needed when we go public, why remove them?

public record PrimaryKey(
@NotNull @Schema(description = "Columns that make the partition keys", type = SchemaType.ARRAY)
@NotNull
@Schema(description = "Columns that make the partition keys", type = SchemaType.ARRAY)
@JsonProperty("partitionBy")
@JsonInclude(JsonInclude.Include.NON_NULL)
String[] keys,
@Nullable @Schema(description = "Columns that make the ordering keys", type = SchemaType.ARRAY)
@Nullable
@Schema(description = "Columns that make the ordering keys", type = SchemaType.ARRAY)
@JsonProperty("partitionSort")
@JsonSerialize(using = OrderingKeysSerializer.class)
OrderingKey[] orderingKeys) {

public record OrderingKey(String column, Order order) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,29 @@
package io.stargate.sgv2.jsonapi.api.model.command.table.definition.datatype;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import io.stargate.sgv2.jsonapi.api.model.command.deserializers.ColumnDefinitionDeserializer;
import io.stargate.sgv2.jsonapi.api.model.command.impl.VectorizeConfig;
import io.stargate.sgv2.jsonapi.api.model.command.serializer.ColumnDefinitionSerializer;
import io.stargate.sgv2.jsonapi.exception.SchemaException;
import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataType;
import java.util.List;
import java.util.Map;

/** Interface for column types. This is used to define the type of a column in a table. */
@JsonDeserialize(using = ColumnDefinitionDeserializer.class)
@JsonSerialize(using = ColumnDefinitionSerializer.class)
public interface ColumnType {
// Returns api data type.
ApiDataType getApiDataType();

/*
Returns the name of the column type to be used in the API request.
*/
default String getApiName() {
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
return getApiDataType().getApiName();
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this list getSupportedTypes() exist seperate to the definitions of the data types we have else where ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no consolidated place to get all supported data types. Error messages needs to be revisited, May be not a good idea to return list of supported types.

static List<String> getSupportedTypes() {
return List.of(
"ascii",
Expand Down Expand Up @@ -46,42 +56,6 @@ static ColumnType fromString(
// TODO: the name of the type should be a part of the ColumnType interface, and use a map for
// the lookup
switch (type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls refactor fromString() to remove the use of string literals like "map" etc

the errors can be better, e.g. the error here :

      if (keyType == null || valueType == null) {
        throw SchemaException.Code.MAP_TYPE_INCORRECT_DEFINITION.get();
      }

we know which is missing can we say that in the error ?

also, when this makes the recursive call for the collection types, and the key or value type is unknown it will generate an error that is then lost / thrown away and a more generic one returned.

another error that can do with more detail:

      if (dimension <= 0) {
        throw SchemaException.Code.VECTOR_TYPE_INCORRECT_DEFINITION.get();
      }

include the dimension in the error message

case "ascii":
return PrimitiveTypes.ASCII;
case "bigint":
return PrimitiveTypes.BIGINT;
case "blob":
return PrimitiveTypes.BINARY;
case "boolean":
return PrimitiveTypes.BOOLEAN;
case "date":
return PrimitiveTypes.DATE;
case "decimal":
return PrimitiveTypes.DECIMAL;
case "double":
return PrimitiveTypes.DOUBLE;
case "duration":
return PrimitiveTypes.DURATION;
case "float":
return PrimitiveTypes.FLOAT;
case "inet":
return PrimitiveTypes.INET;
case "int":
return PrimitiveTypes.INT;
case "smallint":
return PrimitiveTypes.SMALLINT;
case "text":
return PrimitiveTypes.TEXT;
case "time":
return PrimitiveTypes.TIME;
case "timestamp":
return PrimitiveTypes.TIMESTAMP;
case "tinyint":
return PrimitiveTypes.TINYINT;
case "uuid":
return PrimitiveTypes.UUID;
case "varint":
return PrimitiveTypes.VARINT;
case "map":
{
if (keyType == null || valueType == null) {
Expand Down Expand Up @@ -134,6 +108,10 @@ static ColumnType fromString(
}
default:
{
ColumnType columnType = PrimitiveTypes.fromString(type);
if (columnType != null) {
return columnType;
}
Map<String, String> errorMessageFormattingValues =
Map.of(
"type",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ public ApiDataType getApiDataType() {
(PrimitiveApiDataType) keyType.getApiDataType(),
(PrimitiveApiDataType) valueType.getApiDataType());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class says :

/** Interface for complex column types like collections */
public class ComplexTypes {

but it is not an interface

}

public String keyTypeName() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recommend this returns the type, not the name, to avoid doing string comparisons , same for similar functions here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used in serialization for response json where a text value (int, map, etc) are needed.

return keyType.getApiDataType().getApiName();
}

public String valueTypeName() {
return valueType.getApiDataType().getApiName();
}
}

/** A list type implementation */
Expand All @@ -38,6 +46,10 @@ public ListType(ColumnType valueType) {
public ApiDataType getApiDataType() {
return new ComplexApiDataType.ListType((PrimitiveApiDataType) valueType.getApiDataType());
}

public String valueTypeName() {
return valueType.getApiDataType().getApiName();
}
}

/** A set type implementation */
Expand All @@ -52,6 +64,10 @@ public SetType(ColumnType valueType) {
public ApiDataType getApiDataType() {
return new ComplexApiDataType.SetType((PrimitiveApiDataType) valueType.getApiDataType());
}

public String valueTypeName() {
return valueType.getApiDataType().getApiName();
}
}

/* Vector type */
Expand Down Expand Up @@ -81,4 +97,30 @@ public int getDimension() {
return vectorSize;
}
}

/**
* Unsupported type implementation, returned in response when cql table has unsupported format
* column
*/
public static class UnsupportedType implements ColumnType {
private final String cqlFormat;

public UnsupportedType(String cqlFormat) {
this.cqlFormat = cqlFormat;
}

@Override
public ApiDataType getApiDataType() {
throw new UnsupportedOperationException("Unsupported type");
}

@Override
public String getApiName() {
return "UNSUPPORTED";
}

public String cqlFormat() {
return cqlFormat;
}
}
}
Loading