diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiIndexType.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiIndexType.java index 5cda8f50f..e26408686 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiIndexType.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiIndexType.java @@ -7,7 +7,7 @@ public enum ApiIndexType { // map, set, list COLLECTION, - // Something on a scalar column, and non analsed text index + // Something on a scalar column, and non analysed text index REGULAR, // Not available yet TEXT_ANALYSED, diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiTableDef.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiTableDef.java index 1bd323ac2..e84447861 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiTableDef.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiTableDef.java @@ -162,7 +162,7 @@ public ApiIndexDefContainer indexesIncludingUnsupported() { } /** - * Factory for creating a {@link ApiTableDef} from a users decription sent in a command {@link + * Factory for creating a {@link ApiTableDef} from a users description sent in a command {@link * TableDefinitionDesc}. * *

Use the singleton {@link #FROM_TABLE_DESC_FACTORY} to create an instance. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/CQLSAIIndex.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/CQLSAIIndex.java index 333dbacd6..d797fe8ec 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/CQLSAIIndex.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/CQLSAIIndex.java @@ -4,6 +4,7 @@ import com.datastax.oss.driver.api.core.metadata.schema.IndexKind; import com.datastax.oss.driver.api.core.metadata.schema.IndexMetadata; import com.datastax.oss.driver.internal.core.adminrequest.AdminRow; +import com.datastax.oss.driver.internal.core.util.Strings; import io.stargate.sgv2.jsonapi.exception.checked.UnknownCqlIndexFunctionException; import io.stargate.sgv2.jsonapi.exception.checked.UnsupportedCqlIndexException; import io.stargate.sgv2.jsonapi.util.defaults.Properties; @@ -93,6 +94,10 @@ static boolean indexClassIsSai(String className) { * {'class_name': 'StorageAttachedIndex', 'target': 'entries(query_timestamp_values)'} * {'class_name': 'StorageAttachedIndex', 'target': 'comment_vector'} * {'class_name': 'StorageAttachedIndex', 'similarity_function': 'cosine', 'target': 'my_vector'} + * ------------------------------------------------ + * If column is doubleQuoted in the original CQL index + * {'class_name': 'StorageAttachedIndex', 'target': '"Age"'} + * {'class_name': 'StorageAttachedIndex', 'target': 'values("Age")'} * * * The target can just be the name of the column, or the name of the column in parentheses @@ -102,11 +107,14 @@ static boolean indexClassIsSai(String className) { *

The Reg Exp below will match: * *

*/ - private static Pattern INDEX_TARGET_PATTERN = Pattern.compile("^(\\w+)?(?:\\((\\w+)\\))?$"); + private static Pattern INDEX_TARGET_PATTERN = + Pattern.compile("^(\"?\\w+\"?)?(?:\\((\"?\\w+\"?)\\))?$"); /** * Parses the target from the IndexMetadata to extract the column name and the function if there @@ -144,7 +152,15 @@ static IndexTarget indexTarget(IndexMetadata indexMetadata) columnName = matcher.group(2); functionName = matcher.group(1); } - + // At this point, after matcher, we may get a columnName doubleQuoted string from the index + // target + // E.G. 'target': 'values("a_list_column")' -> "a_list_column", 'target': + // 'values("a_regular_column")' -> "a_regular_column" + // 1. We can NOT use fromInternal which will keep the double quote in the identifier + // 2. We can NOT use fromCQL, since it will lowercase the unquoted string, E.G. 'BigApple' -> + // bigapple + // So we should just stripe the doubleQuote if needed + columnName = Strings.unDoubleQuote(columnName); return functionName == null ? new IndexTarget(CqlIdentifier.fromInternal(columnName), null) : new IndexTarget( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/IndexFactoryFromCql.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/IndexFactoryFromCql.java index 74b57ca46..84e6eb6f9 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/IndexFactoryFromCql.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/IndexFactoryFromCql.java @@ -16,9 +16,10 @@ public abstract class IndexFactoryFromCql extends FactoryFromCql { public ApiIndexDef create(ApiColumnDefContainer allColumns, IndexMetadata indexMetadata) throws UnsupportedCqlIndexException { - // This first check is to see if there is anyway we can support this index, becase we are only + // This first check is to see if there is anyway we can support this index, because we are only // supporting - // SAI indexes, later on we may find something that we could support but dont like a new type of + // SAI indexes, later on we may find something that we could support but don't like a new type + // of // sai if (!isSupported(indexMetadata)) { return createUnsupported(indexMetadata); @@ -33,7 +34,7 @@ public ApiIndexDef create(ApiColumnDefContainer allColumns, IndexMetadata indexM if (apiColumnDef == null) { // Cassandra should not let there be an index on a column that is not on the table throw new IllegalStateException( - "Could not find target column index.name:%s target: " + "Could not find target column index.name:%s target:%s" .formatted(indexMetadata.getName(), indexTarget.targetColumn())); } // will throw if we cannot work it out diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIndexIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIndexIntegrationTest.java index 737032b8f..171b56261 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIndexIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIndexIntegrationTest.java @@ -2,6 +2,7 @@ import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertNamespaceCommand; import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; +import static net.javacrumbs.jsonunit.JsonMatchers.jsonEquals; import static org.hamcrest.Matchers.*; import io.quarkus.test.common.WithTestResource; @@ -28,6 +29,7 @@ public final void createSimpleTable() { Map.entry("id", Map.of("type", "text")), Map.entry("age", Map.of("type", "int")), Map.entry("comment", Map.of("type", "text")), + Map.entry("CapitalLetterColumn", Map.of("type", "text")), Map.entry("vehicle_id", Map.of("type", "text")), Map.entry("vehicle_id_1", Map.of("type", "text")), Map.entry("vehicle_id_2", Map.of("type", "text")), @@ -212,16 +214,75 @@ public void createMapIndex() { @Test public void createIndexForQuotedColumn() { + var createIndexJson = + """ + { + "name": "physicalAddress_idx_0", + "definition": { + "column": "physicalAddress" + } + } + """; assertTableCommand(keyspaceName, testTableName) - .postCreateIndex( - """ - { - "name": "physicalAddress_idx", - "definition": { - "column": "physicalAddress" - } - } - """) + .postCreateIndex(createIndexJson) + .wasSuccessful(); + + var createIndexJsonExpected = + """ + { + "name": "physicalAddress_idx_0", + "definition": { + "column": "physicalAddress", + "options": { + "ascii": false, + "caseSensitive": true, + "normalize": false + } + } + } + """; + + assertTableCommand(keyspaceName, testTableName) + .templated() + .listIndexes(true) + .wasSuccessful() + .body("status.indexes", hasItem(jsonEquals(createIndexJsonExpected))); + + assertNamespaceCommand(keyspaceName) + .templated() + .dropIndex("physicalAddress_idx_0", false) + .wasSuccessful(); + } + + @Test + public void createIndexWithOptionForQuotedColumn() { + var createIndexJson = + """ + { + "name": "physicalAddress_idx_1", + "definition": { + "column": "physicalAddress", + "options": { + "ascii": false, + "caseSensitive": true, + "normalize": false + } + } + } + """; + assertTableCommand(keyspaceName, testTableName) + .postCreateIndex(createIndexJson) + .wasSuccessful(); + + assertTableCommand(keyspaceName, testTableName) + .templated() + .listIndexes(true) + .wasSuccessful() + .body("status.indexes", hasItem(jsonEquals(createIndexJson))); + + assertNamespaceCommand(keyspaceName) + .templated() + .dropIndex("physicalAddress_idx_1", false) .wasSuccessful(); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ListIndexesIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ListIndexesIntegrationTest.java index f0cb56d19..c128a06be 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ListIndexesIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ListIndexesIntegrationTest.java @@ -3,12 +3,17 @@ import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertNamespaceCommand; import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; import static net.javacrumbs.jsonunit.JsonMatchers.jsonEquals; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.hasSize; +import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.*; +import com.datastax.oss.driver.api.core.cql.SimpleStatement; +import com.datastax.oss.driver.api.core.type.DataTypes; +import com.datastax.oss.driver.api.querybuilder.SchemaBuilder; +import com.datastax.oss.driver.api.querybuilder.schema.CreateTable; import io.quarkus.test.common.WithTestResource; import io.quarkus.test.junit.QuarkusIntegrationTest; import io.stargate.sgv2.jsonapi.testresource.DseTestResource; +import io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.ClassOrderer; import org.junit.jupiter.api.MethodOrderer; @@ -101,11 +106,11 @@ public final void createDefaultTablesAndIndexes() { assertNamespaceCommand(keyspaceName) .postCreateTable(tableData.formatted(TABLE)) .wasSuccessful(); - + // index1, name_idx assertTableCommand(keyspaceName, TABLE).postCreateIndex(createIndex).wasSuccessful(); - + // index2, city_idx assertTableCommand(keyspaceName, TABLE).postCreateIndex(createWithoutOptions).wasSuccessful(); - + // index3, content_idx assertTableCommand(keyspaceName, TABLE) .postCreateVectorIndex(createVectorIndex) .wasSuccessful(); @@ -129,7 +134,7 @@ public void listIndexesOnly() { @Test @Order(2) - public void listIndexessWithDefinition() { + public void listIndexesWithDefinition() { assertTableCommand(keyspaceName, TABLE) .templated() @@ -146,4 +151,185 @@ public void listIndexessWithDefinition() { jsonEquals(createVectorIndex))); } } + + // ================================================================================================================== + // This subClass is to test some index corner cases. + // 1. Currently, Data API does not support create index on set/list/map column, so we need to + // create these indexes in CQL, and then test ListIndexes command + // 2. Data API resolves index target from IndexMetaData, detail in {@link CQLSAIINDEX}, test + // columns with doubleQuote + // ================================================================================================================== + @Nested + public class CreatedIndexOnPreExistedCqlTable { + private static final String PRE_EXISTED_CQL_TABLE = "perExistedCqlTable"; + + @Test + @Order(1) + public final void createPerExistedCqlTable() { + // Build the CREATE TABLE statement + CreateTable createTable = + SchemaBuilder.createTable( + CqlIdentifierUtil.cqlIdentifierFromUserInput(keyspaceName), + CqlIdentifierUtil.cqlIdentifierFromUserInput(PRE_EXISTED_CQL_TABLE)) + .withPartitionKey("id", DataTypes.TEXT) // Primary key + .withColumn( + CqlIdentifierUtil.cqlIdentifierFromUserInput("TextQuoted"), + DataTypes.TEXT) // doubleQuoted column + .withColumn("\"setColumn\"", DataTypes.setOf(DataTypes.TEXT, false)) // set column + .withColumn( + "\"mapColumn\"", + DataTypes.mapOf(DataTypes.TEXT, DataTypes.INT, false)) // map column + .withColumn("\"listColumn\"", DataTypes.listOf(DataTypes.TEXT, false)); // list column + assertThat(executeCqlStatement(createTable.build())).isTrue(); + + // Create an index on the doubleQuoted column "TextQuoted" + String createTextIndexCql = + String.format( + "CREATE CUSTOM INDEX IF NOT EXISTS \"idx_textQuoted\" ON \"%s\".\"%s\" (\"TextQuoted\") USING 'StorageAttachedIndex'", + keyspaceName, PRE_EXISTED_CQL_TABLE); + assertThat(executeCqlStatement(SimpleStatement.newInstance(createTextIndexCql))).isTrue(); + + // Create an index on the entire set + String createSetIndexCql = + String.format( + "CREATE CUSTOM INDEX IF NOT EXISTS idx_set ON \"%s\".\"%s\" (\"setColumn\") USING 'StorageAttachedIndex'", + keyspaceName, PRE_EXISTED_CQL_TABLE); + assertThat(executeCqlStatement(SimpleStatement.newInstance(createSetIndexCql))).isTrue(); + + // Create an index on the entire list + String createListIndexCql = + String.format( + "CREATE CUSTOM INDEX IF NOT EXISTS idx_list ON \"%s\".\"%s\" (\"listColumn\") USING 'StorageAttachedIndex'", + keyspaceName, PRE_EXISTED_CQL_TABLE); + assertThat(executeCqlStatement(SimpleStatement.newInstance(createListIndexCql))).isTrue(); + + // Create an index on the keys of the map + String createMapKeyIndexCql = + String.format( + "CREATE CUSTOM INDEX IF NOT EXISTS idx_map_keys ON \"%s\".\"%s\" (KEYS(\"mapColumn\")) USING 'StorageAttachedIndex'", + keyspaceName, PRE_EXISTED_CQL_TABLE); + assertThat(executeCqlStatement(SimpleStatement.newInstance(createMapKeyIndexCql))).isTrue(); + + // Create an index on the values of the map + String createMapValueIndexCql = + String.format( + "CREATE CUSTOM INDEX IF NOT EXISTS idx_map_values ON \"%s\".\"%s\" (VALUES(\"mapColumn\")) USING 'StorageAttachedIndex'", + keyspaceName, PRE_EXISTED_CQL_TABLE); + assertThat(executeCqlStatement(SimpleStatement.newInstance(createMapValueIndexCql))).isTrue(); + + // Create an index on the entries of the map + String createMapEntryIndexCql = + String.format( + "CREATE CUSTOM INDEX IF NOT EXISTS idx_map_entries ON \"%s\".\"%s\" (ENTRIES(\"mapColumn\")) USING 'StorageAttachedIndex'", + keyspaceName, PRE_EXISTED_CQL_TABLE); + assertThat(executeCqlStatement(SimpleStatement.newInstance(createMapEntryIndexCql))).isTrue(); + } + + @Test + @Order(2) + public void listIndexesWithDefinition() { + // TODO, currently ApiIndexType treats index on map/set/list as unsupported, so the index on + // these columns have UNKNOWN column in the definition + var expected_idx_set = + """ + { + "name": "idx_set", + "definition": { + "column": "UNKNOWN", + "apiSupport": { + "createIndex": false, + "filter": false, + "cqlDefinition": "CREATE CUSTOM INDEX idx_set ON \\"%s\\".\\"%s\\" (values(\\"setColumn\\"))\\nUSING 'StorageAttachedIndex'" + } + } + } + """ + .formatted(keyspaceName, PRE_EXISTED_CQL_TABLE); + var expected_idx_map_values = + """ + { + "name": "idx_map_values", + "definition": { + "column": "UNKNOWN", + "apiSupport": { + "createIndex": false, + "filter": false, + "cqlDefinition": "CREATE CUSTOM INDEX idx_map_values ON \\"%s\\".\\"%s\\" (values(\\"mapColumn\\"))\\nUSING 'StorageAttachedIndex'" + } + } + } + """ + .formatted(keyspaceName, PRE_EXISTED_CQL_TABLE); + var expected_idx_map_keys = + """ + { + "name": "idx_map_keys", + "definition": { + "column": "UNKNOWN", + "apiSupport": { + "createIndex": false, + "filter": false, + "cqlDefinition": "CREATE CUSTOM INDEX idx_map_keys ON \\"%s\\".\\"%s\\" (keys(\\"mapColumn\\"))\\nUSING 'StorageAttachedIndex'" + } + } + } + """ + .formatted(keyspaceName, PRE_EXISTED_CQL_TABLE); + var expected_idx_map_entries = + """ + { + "name": "idx_map_entries", + "definition": { + "column": "UNKNOWN", + "apiSupport": { + "createIndex": false, + "filter": false, + "cqlDefinition": "CREATE CUSTOM INDEX idx_map_entries ON \\"%s\\".\\"%s\\" (entries(\\"mapColumn\\"))\\nUSING 'StorageAttachedIndex'" + } + } + } + """ + .formatted(keyspaceName, PRE_EXISTED_CQL_TABLE); + var expected_idx_list = + """ + { + "name": "idx_list", + "definition": { + "column": "UNKNOWN", + "apiSupport": { + "createIndex": false, + "filter": false, + "cqlDefinition": "CREATE CUSTOM INDEX idx_list ON \\"%s\\".\\"%s\\" (values(\\"listColumn\\"))\\nUSING 'StorageAttachedIndex'" + } + } + } + """ + .formatted(keyspaceName, PRE_EXISTED_CQL_TABLE); + var expected_idx_quoted = + """ + { + "name": "idx_textQuoted", + "definition": { + "column": "TextQuoted", + "options": { + } + } + } + """; + assertTableCommand(keyspaceName, PRE_EXISTED_CQL_TABLE) + .templated() + .listIndexes(true) + .wasSuccessful() + .body("status.indexes", hasSize(6)) + .body( + "status.indexes", + containsInAnyOrder( // Validate that the indexes are in any order + jsonEquals(expected_idx_set), + jsonEquals(expected_idx_map_keys), + jsonEquals(expected_idx_map_values), + jsonEquals(expected_idx_map_entries), + jsonEquals(expected_idx_list), + jsonEquals(expected_idx_quoted))); + } + } }