CNDB-13074: Reject analysis options on frozen collections (#1610)

Reject creating indexes with analysis options on frozen collections.
We don't have a way to correctly index them, and we don't support querying either.

Author: adelapena

src/java/org/apache/cassandra/index/sai/IndexContext.java

@@ -178,14 +178,16 @@ public IndexContext(@Nonnull String keyspace,
         this.cfs = cfs;
         this.primaryKeyFactory = Version.latest().onDiskFormat().newPrimaryKeyFactory(clusteringComparator);
 
+        String columnName = column.name.toString();
+
         if (config != null)
         {
             String fullIndexName = String.format("%s.%s.%s", this.keyspace, this.table, this.config.name);
             this.indexWriterConfig = IndexWriterConfig.fromOptions(fullIndexName, validator, config.options);
             this.isAnalyzed = AbstractAnalyzer.isAnalyzed(config.options);
-            this.analyzerFactory = AbstractAnalyzer.fromOptions(getValidator(), config.options);
+            this.analyzerFactory = AbstractAnalyzer.fromOptions(columnName, validator, config.options);
             this.queryAnalyzerFactory = AbstractAnalyzer.hasQueryAnalyzer(config.options)
-                                        ? AbstractAnalyzer.fromOptionsQueryAnalyzer(getValidator(), config.options)
+                                        ? AbstractAnalyzer.fromOptionsQueryAnalyzer(validator, config.options)
                                         : this.analyzerFactory;
             this.vectorSimilarityFunction = indexWriterConfig.getSimilarityFunction();
             this.hasEuclideanSimilarityFunc = vectorSimilarityFunction == VectorSimilarityFunction.EUCLIDEAN;
@@ -199,7 +201,7 @@ public IndexContext(@Nonnull String keyspace,
         {
             this.indexWriterConfig = IndexWriterConfig.emptyConfig();
             this.isAnalyzed = AbstractAnalyzer.isAnalyzed(Collections.EMPTY_MAP);
-            this.analyzerFactory = AbstractAnalyzer.fromOptions(getValidator(), Collections.EMPTY_MAP);
+            this.analyzerFactory = AbstractAnalyzer.fromOptions(columnName, validator, Collections.EMPTY_MAP);
             this.queryAnalyzerFactory = this.analyzerFactory;
             this.vectorSimilarityFunction = null;
             this.hasEuclideanSimilarityFunc = false;

src/java/org/apache/cassandra/index/sai/StorageAttachedIndex.java

@@ -330,7 +330,7 @@ public static Map<String, String> validateOptions(Map<String, String> options, T
         AbstractType<?> type = TypeUtil.cellValueType(target.left, target.right);
 
         // Validate analyzers by building them
-        try (AbstractAnalyzer.AnalyzerFactory analyzerFactory = AbstractAnalyzer.fromOptions(type, options))
+        try (AbstractAnalyzer.AnalyzerFactory analyzerFactory = AbstractAnalyzer.fromOptions(targetColumn, type, options))
         {
             if (AbstractAnalyzer.hasQueryAnalyzer(options))
                 AbstractAnalyzer.fromOptionsQueryAnalyzer(type, options).close();

src/java/org/apache/cassandra/index/sai/analyzer/AbstractAnalyzer.java

@@ -187,7 +187,7 @@ public static boolean isAnalyzed(Map<String, String> options) {
         return options.containsKey(LuceneAnalyzer.INDEX_ANALYZER) || NonTokenizingOptions.hasNonDefaultOptions(options);
     }
 
-    public static AnalyzerFactory fromOptions(AbstractType<?> type, Map<String, String> options)
+    public static AnalyzerFactory fromOptions(String target, AbstractType<?> type, Map<String, String> options)
     {
         boolean containsIndexAnalyzer = options.containsKey(LuceneAnalyzer.INDEX_ANALYZER);
         boolean containsNonTokenizingOptions = NonTokenizingOptions.hasNonDefaultOptions(options);
@@ -206,6 +206,9 @@ public static AnalyzerFactory fromOptions(AbstractType<?> type, Map<String, Stri
                                               " combination of case_sensitive, normalize, or ascii options. options=" + options);
         }
 
+        if ((containsIndexAnalyzer || containsNonTokenizingOptions) && type.isCollection() && !type.isMultiCell())
+            throw new InvalidRequestException("Cannot use an analyzer on " + target + " because it's a frozen collection.");
+
         if (containsIndexAnalyzer)
         {
             String json = options.get(LuceneAnalyzer.INDEX_ANALYZER);

test/unit/org/apache/cassandra/cql3/CQLTester.java

@@ -204,7 +204,7 @@ public abstract class CQLTester
     private static final User SUPER_USER = new User("cassandra", "cassandra");
 
     /**
-     * Whether to use coorfinator execution in {@link #execute(String, Object...)}, so queries get full validation and
+     * Whether to use coordinator execution in {@link #execute(String, Object...)}, so queries get full validation and
      * go through reconciliation. When enabled, calls to {@link #execute(String, Object...)} will behave as calls to
      * {@link #executeWithCoordinator(String, Object...)}. Otherwise, they will behave as calls to
      * {@link #executeInternal(String, Object...)}.

test/unit/org/apache/cassandra/index/sai/SAITester.java

@@ -230,6 +230,23 @@ public static void setUpClass()
         CQLTester.enableCoordinatorExecution();
     }
 
+    /**
+     * Creates a SAI index on the current table, waiting for it to become queryable.
+     *
+     * @param column the name of the indexed column, maybe with {@code FULL()}, {@code KEYS()} or {@code VALUES()} spec
+     * @param options the index options, of the form {@code "{'option1': value1, 'option2': value2, ...}"}.
+     * @return the name of the created index
+     */
+    public String createSAIIndex(String column, @Nullable String options)
+    {
+        String query = String.format(CREATE_INDEX_TEMPLATE, column);
+
+        if (options != null)
+            query += " WITH OPTIONS = " + options;
+
+        return createIndex(query);
+    }
+
     public static IndexContext createIndexContext(String name, AbstractType<?> validator, ColumnFamilyStore cfs)
     {
         return new IndexContext(cfs.getKeyspaceName(),

test/unit/org/apache/cassandra/index/sai/cql/AnalyzerTest.java

@@ -18,9 +18,16 @@
 
 package org.apache.cassandra.index.sai.cql;
 
+import org.apache.cassandra.exceptions.InvalidRequestException;
+import org.apache.cassandra.index.sai.SAITester;
+import org.apache.cassandra.index.sai.analyzer.NonTokenizingOptions;
+import org.assertj.core.api.Assertions;
 import org.junit.Test;
 
-public class AnalyzerTest extends VectorTester
+import javax.annotation.Nullable;
+import java.util.Arrays;
+
+public class AnalyzerTest extends SAITester
 {
     @Test
     public void createAnalyzerWrongTypeTest()
@@ -49,4 +56,72 @@ public void createAnalyzerWrongTypeTest()
         execute("INSERT INTO %s (pk1, pk2, val) VALUES (0, 'c', -2)");
         execute("INSERT INTO %s (pk1, pk2, val) VALUES (1, 'c', -3)");
     }
+
+    /**
+     * Test that we cannot use an analyzer, tokenizing or not, on a frozen collection.
+     */
+    @Test
+    public void analyzerOnFrozenCollectionTest()
+    {
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, l frozen<list<text>>, s frozen<set<text>>, m frozen<map<text, text>>)");
+
+        for (String column : Arrays.asList("l", "s", "m"))
+        {
+            assertRejectsNonFullIndexCreationOnFrozenCollection(column);
+            column = String.format("full(%s)", column);
+
+            // non-tokenizing options that produce an analyzer should be rejected
+            assertRejectsAnalyzerOnFrozenCollection(column, String.format("{'%s': %s}", NonTokenizingOptions.CASE_SENSITIVE, false));
+            assertRejectsAnalyzerOnFrozenCollection(column, String.format("{'%s': %s}", NonTokenizingOptions.NORMALIZE, true));
+            assertRejectsAnalyzerOnFrozenCollection(column, String.format("{'%s': %s}", NonTokenizingOptions.ASCII, true));
+
+            // non-tokenizing options that do not produce an analyzer should be accepted
+            assertAcceptsIndexOptions(column, String.format("{'%s': %s}", NonTokenizingOptions.CASE_SENSITIVE, true));
+            assertAcceptsIndexOptions(column, String.format("{'%s': %s}", NonTokenizingOptions.NORMALIZE, false));
+            assertAcceptsIndexOptions(column, String.format("{'%s': %s}", NonTokenizingOptions.ASCII, false));
+
+            // Lucene analyzer should always be rejected
+            assertRejectsAnalyzerOnFrozenCollection(column, "{'index_analyzer': 'standard'}");
+            assertRejectsAnalyzerOnFrozenCollection(column,
+                    "{'index_analyzer': " +
+                    "  '{\"tokenizer\":{\"name\":\"ngram\", \"args\":{\"minGramSize\":\"2\", \"maxGramSize\":\"3\"}}," +
+                    "    \"filters\":[{\"name\":\"lowercase\"}]}'}");
+            assertRejectsAnalyzerOnFrozenCollection(column,
+                    "{'index_analyzer':'\n" +
+                            "  {\"tokenizer\":{\"name\" : \"whitespace\"},\n" +
+                            "   \"filters\":[{\"name\":\"stop\", \"args\": {\"words\": \"the, test\", \"format\": \"wordset\"}}]}'}");
+
+            // no options should be accepted
+            assertAcceptsIndexOptions(column, null);
+            assertAcceptsIndexOptions(column, "{}");
+        }
+    }
+
+    private void assertRejectsNonFullIndexCreationOnFrozenCollection(String column)
+    {
+        Assertions.assertThatThrownBy(() -> createSAIIndex(column, null))
+                  .isInstanceOf(InvalidRequestException.class)
+                  .hasMessageContaining("Cannot create values() index on frozen column " + column);
+
+        Assertions.assertThatThrownBy(() -> createSAIIndex("KEYS(" + column + ')', null))
+                  .isInstanceOf(InvalidRequestException.class)
+                  .hasMessageContaining("Cannot create keys() index on frozen column " + column);
+
+        Assertions.assertThatThrownBy(() -> createSAIIndex("VALUES(" + column + ')', null))
+                  .isInstanceOf(InvalidRequestException.class)
+                  .hasMessageContaining("Cannot create values() index on frozen column " + column);
+    }
+
+    private void assertAcceptsIndexOptions(String column, @Nullable String options)
+    {
+        String index = createSAIIndex(column, options);
+        dropIndex("DROP INDEX %s." + index); // clear for further tests
+    }
+
+    private void assertRejectsAnalyzerOnFrozenCollection(String column, String options)
+    {
+        Assertions.assertThatThrownBy(() -> createSAIIndex(column, options))
+                  .isInstanceOf(InvalidRequestException.class)
+                  .hasMessageContaining("Cannot use an analyzer on " + column + " because it's a frozen collection.");
+    }
 }