From 3315a1200d5b74b9e3cdc56364307dcfea91288c Mon Sep 17 00:00:00 2001 From: Jonathan Ellis Date: Thu, 12 Dec 2024 08:57:37 -0600 Subject: [PATCH] validateOptions treats analyzed and un-analyzed indexes as distinct, testTwoIndexes passes --- .../index/sai/StorageAttachedIndex.java | 30 +++++++++++-------- .../cassandra/index/sai/cql/BM25Test.java | 9 +++++- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java index 74482d97a7c9..691f0fd99308 100644 --- a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java +++ b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java @@ -297,20 +297,26 @@ public static Map validateOptions(Map options, T throw new InvalidRequestException("Failed to retrieve target column for: " + targetColumn); } - // In order to support different index target on non-frozen map, ie. KEYS, VALUE, ENTRIES, we need to put index - // name as part of index file name instead of column name. We only need to check that the target is different - // between indexes. This will only allow indexes in the same column with a different IndexTarget.Type. - // - // Note that: "metadata.indexes" already includes current index - if (metadata.indexes.stream().filter(index -> index.getIndexClassName().equals(StorageAttachedIndex.class.getName())) - .map(index -> TargetParser.parse(metadata, index.options.get(IndexTarget.TARGET_OPTION_NAME))) - .filter(Objects::nonNull).filter(t -> t.equals(target)).count() > 1) - { - throw new InvalidRequestException("Cannot create more than one storage-attached index on the same column: " + target.left); - } + // Check for duplicate indexes considering both target and analyzer configuration + boolean isAnalyzed = AbstractAnalyzer.isAnalyzed(options); + long duplicateCount = metadata.indexes.stream() + .filter(index -> index.getIndexClassName().equals(StorageAttachedIndex.class.getName())) + .filter(index -> { + // Indexes on the same column with different target (KEYS, VALUES, ENTRIES) + // are allowed on non-frozen Maps + var existingTarget = TargetParser.parse(metadata, index.options.get(IndexTarget.TARGET_OPTION_NAME)); + if (existingTarget == null || !existingTarget.equals(target)) + return false; + // Also allow different indexes if one is analyzed and the other isn't + return isAnalyzed == AbstractAnalyzer.isAnalyzed(index.options); + }) + .count(); + // >1 because "metadata.indexes" already includes current index + if (duplicateCount > 1) + throw new InvalidRequestException(String.format("Cannot create duplicate storage-attached index on column: %s", target.left)); // Analyzer is not supported against PK columns - if (AbstractAnalyzer.isAnalyzed(options)) + if (isAnalyzed) { for (ColumnMetadata column : metadata.primaryKeyColumns()) { diff --git a/test/unit/org/apache/cassandra/index/sai/cql/BM25Test.java b/test/unit/org/apache/cassandra/index/sai/cql/BM25Test.java index 44f5c76982f7..cd6457f162fe 100644 --- a/test/unit/org/apache/cassandra/index/sai/cql/BM25Test.java +++ b/test/unit/org/apache/cassandra/index/sai/cql/BM25Test.java @@ -51,11 +51,18 @@ public void testTwoIndexes() throws Throwable // create un-analyzed index createTable("CREATE TABLE %s (k int PRIMARY KEY, v text)"); createIndex("CREATE CUSTOM INDEX ON %s(v) USING 'org.apache.cassandra.index.sai.StorageAttachedIndex'"); - execute("INSERT INTO %s (k, v) VALUES (1, 'apple')"); + + // BM25 should fail with only an equality index assertThatThrownBy(() -> execute("SELECT k FROM %s WHERE v : 'apple' ORDER BY v BM25 OF 'apple' LIMIT 3")) .isInstanceOf(InvalidRequestException.class) .hasMessage("BM25 ordering on column v requires an analyzed index"); + + // create analyzed index + analyzeIndex(); + // BM25 query should work now + var result = execute("SELECT k FROM %s WHERE v : 'apple' ORDER BY v BM25 OF 'apple' LIMIT 3"); + assertRows(result, row(1)); } @Test