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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -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 a frozen collection.");

if (containsIndexAnalyzer)
{
String json = options.get(LuceneAnalyzer.INDEX_ANALYZER);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/org/apache/cassandra/cql3/CQLTester.java
Original file line number Diff line number Diff line change
Expand Up @@ -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...)}.
Expand Down
17 changes: 17 additions & 0 deletions test/unit/org/apache/cassandra/index/sai/SAITester.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
77 changes: 76 additions & 1 deletion test/unit/org/apache/cassandra/index/sai/cql/AnalyzerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 a frozen collection.");
}
}