Repository: cassandra
Updated Branches:
  refs/heads/trunk b11fba750 -> f8e86eb34


SASI index options validation

patch by xedin; reviewed by beobal for CASSANDRA-11136


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/f8e86eb3
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f8e86eb3
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f8e86eb3

Branch: refs/heads/trunk
Commit: f8e86eb341aa5bb36e747ce4f31f56dcbaea7089
Parents: b11fba7
Author: Pavel Yaskevich <[email protected]>
Authored: Sun Feb 7 21:21:22 2016 -0800
Committer: Pavel Yaskevich <[email protected]>
Committed: Wed Feb 10 11:41:03 2016 -0800

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 doc/SASI.md                                     |  8 +-
 .../apache/cassandra/index/TargetParser.java    | 90 ++++++++++++++++++++
 .../index/internal/CassandraIndex.java          | 62 +-------------
 .../apache/cassandra/index/sasi/SASIIndex.java  | 45 ++++++++--
 .../cassandra/index/sasi/conf/ColumnIndex.java  |  6 --
 .../cassandra/index/sasi/conf/IndexMode.java    | 65 ++++++++------
 .../cassandra/thrift/ThriftConversion.java      |  5 +-
 .../index/internal/CustomCassandraIndex.java    |  4 +-
 .../cassandra/index/sasi/SASIIndexTest.java     | 71 +++++++++++++++
 .../schema/LegacySchemaMigratorTest.java        |  4 +-
 11 files changed, 251 insertions(+), 110 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index e6067a6..04ce8d7 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.4
+ * SASI index options validation (CASSANDRA-11136)
  * Optimize disk seek using min/max column name meta data when the LIMIT 
clause is used
    (CASSANDRA-8180)
  * Add LIKE support to CQL3 (CASSANDRA-11067)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/doc/SASI.md
----------------------------------------------------------------------
diff --git a/doc/SASI.md b/doc/SASI.md
index 90fcf65..a4762c9 100644
--- a/doc/SASI.md
+++ b/doc/SASI.md
@@ -70,12 +70,12 @@ first being the default. The `last_name` index is created 
with the
 mode `CONTAINS` which matches terms on suffixes instead of prefix
 only. Examples of this are available below and more detail can be
 found in the section on
-[OnDiskIndex](https://github.com/xedin/sasi#ondiskindexbuilder).The
+[OnDiskIndex](#ondiskindexbuilder).The
 `created_at` column is created with its mode set to `SPARSE`, which is
 meant to improve performance of querying large, dense number ranges
 like timestamps for data inserted every millisecond. Details of the
 `SPARSE` implementation can also be found in the section on the
-[OnDiskIndex](https://github.com/xedin/sasi#ondiskindexbuilder). The `age`
+[OnDiskIndex](#ondiskindexbuilder). The `age`
 index is created with the default `PREFIX` mode and no
 case-sensitivity or text analysis options are specified since the
 field is numeric.
@@ -186,7 +186,7 @@ pitfalls of such a query. With SASI, while the requirement 
to include
 performance pitfalls do not exist because filtering is not
 performed. Details on how SASI joins data from multiple predicates is
 available below in the
-[Implementation Details](https://github.com/xedin/sasi#implementation-details)
+[Implementation Details](#implementation-details)
 section.
 
 ```
@@ -507,7 +507,7 @@ converts from Cassandra's internal representation of
 `IndexExpression`s, which has also been modified to support encoding
 queries that contain ORs and groupings of expressions using
 parentheses (see the
-[Cassandra Internal 
Changes](https://github.com/xedin/sasi#cassandra-internal-changes)
+[Cassandra Internal Changes](#cassandra-internal-changes)
 section below for more details). This process produces a tree of
 
[`Operation`](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/index/sasi/plan/Operation.java)s,
 which in turn may contain 
[`Expression`](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/index/sasi/plan/Expression.java)s,
 all of which
 provide an alternative, more efficient, representation of the query.

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/src/java/org/apache/cassandra/index/TargetParser.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/TargetParser.java 
b/src/java/org/apache/cassandra/index/TargetParser.java
new file mode 100644
index 0000000..849ad16
--- /dev/null
+++ b/src/java/org/apache/cassandra/index/TargetParser.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.index;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.config.ColumnDefinition;
+import org.apache.cassandra.cql3.ColumnIdentifier;
+import org.apache.cassandra.cql3.statements.IndexTarget;
+import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.schema.IndexMetadata;
+import org.apache.cassandra.utils.Pair;
+
+public class TargetParser
+{
+    private static final Pattern TARGET_REGEX = 
Pattern.compile("^(keys|entries|values|full)\\((.+)\\)$");
+    private static final Pattern TWO_QUOTES = Pattern.compile("\"\"");
+    private static final String QUOTE = "\"";
+
+    public static Pair<ColumnDefinition, IndexTarget.Type> parse(CFMetaData 
cfm, IndexMetadata indexDef)
+    {
+        String target = indexDef.options.get("target");
+        assert target != null : String.format("No target definition found for 
index %s", indexDef.name);
+        Pair<ColumnDefinition, IndexTarget.Type> result = parse(cfm, target);
+        if (result == null)
+            throw new ConfigurationException(String.format("Unable to parse 
targets for index %s (%s)", indexDef.name, target));
+        return result;
+    }
+
+    public static Pair<ColumnDefinition, IndexTarget.Type> parse(CFMetaData 
cfm, String target)
+    {
+        // if the regex matches then the target is in the form "keys(foo)", 
"entries(bar)" etc
+        // if not, then it must be a simple column name and implictly its type 
is VALUES
+        Matcher matcher = TARGET_REGEX.matcher(target);
+        String columnName;
+        IndexTarget.Type targetType;
+        if (matcher.matches())
+        {
+            targetType = IndexTarget.Type.fromString(matcher.group(1));
+            columnName = matcher.group(2);
+        }
+        else
+        {
+            columnName = target;
+            targetType = IndexTarget.Type.VALUES;
+        }
+
+        // in the case of a quoted column name the name in the target string
+        // will be enclosed in quotes, which we need to unwrap. It may also
+        // include quote characters internally, escaped like so:
+        //      abc"def -> abc""def.
+        // Because the target string is stored in a CQL compatible form, we
+        // need to un-escape any such quotes to get the actual column name
+        if (columnName.startsWith(QUOTE))
+        {
+            columnName = 
StringUtils.substring(StringUtils.substring(columnName, 1), 0, -1);
+            columnName = TWO_QUOTES.matcher(columnName).replaceAll(QUOTE);
+        }
+
+        // if it's not a CQL table, we can't assume that the column name is 
utf8, so
+        // in that case we have to do a linear scan of the cfm's columns to 
get the matching one
+        if (cfm.isCQLTable())
+            return Pair.create(cfm.getColumnDefinition(new 
ColumnIdentifier(columnName, true)), targetType);
+        else
+            for (ColumnDefinition column : cfm.allColumns())
+                if (column.name.toString().equals(columnName))
+                    return Pair.create(column, targetType);
+
+        return null;
+    }
+}

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java 
b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
index 030abf5..f503e59 100644
--- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
+++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
@@ -5,19 +5,17 @@ import java.util.*;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Future;
 import java.util.function.BiFunction;
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
 import com.google.common.collect.ImmutableSet;
-import org.apache.commons.lang3.StringUtils;
+import org.apache.cassandra.index.TargetParser;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ColumnDefinition;
-import org.apache.cassandra.cql3.ColumnIdentifier;
 import org.apache.cassandra.cql3.Operator;
 import org.apache.cassandra.cql3.statements.IndexTarget;
 import org.apache.cassandra.db.*;
@@ -56,8 +54,6 @@ public abstract class CassandraIndex implements Index
 {
     private static final Logger logger = 
LoggerFactory.getLogger(CassandraIndex.class);
 
-    public static final Pattern TARGET_REGEX = 
Pattern.compile("^(keys|entries|values|full)\\((.+)\\)$");
-
     public final ColumnFamilyStore baseCfs;
     protected IndexMetadata metadata;
     protected ColumnFamilyStore indexCfs;
@@ -195,7 +191,7 @@ public abstract class CassandraIndex implements Index
     private void setMetadata(IndexMetadata indexDef)
     {
         metadata = indexDef;
-        Pair<ColumnDefinition, IndexTarget.Type> target = 
parseTarget(baseCfs.metadata, indexDef);
+        Pair<ColumnDefinition, IndexTarget.Type> target = 
TargetParser.parse(baseCfs.metadata, indexDef);
         functions = getFunctions(indexDef, target);
         CFMetaData cfm = indexCfsMetadata(baseCfs.metadata, indexDef);
         indexCfs = ColumnFamilyStore.createColumnFamilyStore(baseCfs.keyspace,
@@ -708,7 +704,7 @@ public abstract class CassandraIndex implements Index
      */
     public static final CFMetaData indexCfsMetadata(CFMetaData 
baseCfsMetadata, IndexMetadata indexMetadata)
     {
-        Pair<ColumnDefinition, IndexTarget.Type> target = 
parseTarget(baseCfsMetadata, indexMetadata);
+        Pair<ColumnDefinition, IndexTarget.Type> target = 
TargetParser.parse(baseCfsMetadata, indexMetadata);
         CassandraIndexFunctions utils = getFunctions(indexMetadata, target);
         ColumnDefinition indexedColumn = target.left;
         AbstractType<?> indexedValueType = 
utils.getIndexedValueType(indexedColumn);
@@ -752,57 +748,7 @@ public abstract class CassandraIndex implements Index
      */
     public static CassandraIndex newIndex(ColumnFamilyStore baseCfs, 
IndexMetadata indexMetadata)
     {
-        return getFunctions(indexMetadata, parseTarget(baseCfs.metadata, 
indexMetadata)).newIndexInstance(baseCfs, indexMetadata);
-    }
-
-    private static final Pattern TWO_QUOTES = Pattern.compile("\"\"");
-    private static final String QUOTE = "\"";
-
-    // Public because it's also used to convert index metadata into a 
thrift-compatible format
-    public static Pair<ColumnDefinition, IndexTarget.Type> 
parseTarget(CFMetaData cfm,
-                                                                       
IndexMetadata indexDef)
-    {
-        String target = indexDef.options.get("target");
-        assert target != null : String.format("No target definition found for 
index %s", indexDef.name);
-
-        // if the regex matches then the target is in the form "keys(foo)", 
"entries(bar)" etc
-        // if not, then it must be a simple column name and implictly its type 
is VALUES
-        Matcher matcher = TARGET_REGEX.matcher(target);
-        String columnName;
-        IndexTarget.Type targetType;
-        if (matcher.matches())
-        {
-            targetType = IndexTarget.Type.fromString(matcher.group(1));
-            columnName = matcher.group(2);
-        }
-        else
-        {
-            columnName = target;
-            targetType = IndexTarget.Type.VALUES;
-        }
-
-        // in the case of a quoted column name the name in the target string
-        // will be enclosed in quotes, which we need to unwrap. It may also
-        // include quote characters internally, escaped like so:
-        //      abc"def -> abc""def.
-        // Because the target string is stored in a CQL compatible form, we
-        // need to un-escape any such quotes to get the actual column name
-        if (columnName.startsWith(QUOTE))
-        {
-            columnName = 
StringUtils.substring(StringUtils.substring(columnName, 1), 0, -1);
-            columnName = TWO_QUOTES.matcher(columnName).replaceAll(QUOTE);
-        }
-
-        // if it's not a CQL table, we can't assume that the column name is 
utf8, so
-        // in that case we have to do a linear scan of the cfm's columns to 
get the matching one
-        if (cfm.isCQLTable())
-            return Pair.create(cfm.getColumnDefinition(new 
ColumnIdentifier(columnName, true)), targetType);
-        else
-            for (ColumnDefinition column : cfm.allColumns())
-                if (column.name.toString().equals(columnName))
-                    return Pair.create(column, targetType);
-
-        throw new RuntimeException(String.format("Unable to parse targets for 
index %s (%s)", indexDef.name, target));
+        return getFunctions(indexMetadata, 
TargetParser.parse(baseCfs.metadata, indexMetadata)).newIndexInstance(baseCfs, 
indexMetadata);
     }
 
     static CassandraIndexFunctions getFunctions(IndexMetadata indexDef,

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/sasi/SASIIndex.java 
b/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
index 012d923..b460910 100644
--- a/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
+++ b/src/java/org/apache/cassandra/index/sasi/SASIIndex.java
@@ -22,11 +22,10 @@ import java.util.concurrent.Callable;
 import java.util.function.BiFunction;
 
 import com.googlecode.concurrenttrees.common.Iterables;
-import org.apache.cassandra.config.CFMetaData;
-import org.apache.cassandra.config.ColumnDefinition;
-import org.apache.cassandra.config.DatabaseDescriptor;
-import org.apache.cassandra.config.Schema;
+
+import org.apache.cassandra.config.*;
 import org.apache.cassandra.cql3.Operator;
+import org.apache.cassandra.cql3.statements.IndexTarget;
 import org.apache.cassandra.db.*;
 import org.apache.cassandra.db.compaction.CompactionManager;
 import org.apache.cassandra.db.compaction.OperationType;
@@ -36,12 +35,15 @@ import org.apache.cassandra.db.marshal.AbstractType;
 import org.apache.cassandra.db.partitions.PartitionIterator;
 import org.apache.cassandra.db.partitions.PartitionUpdate;
 import org.apache.cassandra.db.rows.Row;
+import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.index.Index;
 import org.apache.cassandra.index.IndexRegistry;
 import org.apache.cassandra.index.SecondaryIndexBuilder;
-import org.apache.cassandra.index.internal.CassandraIndex;
+import org.apache.cassandra.index.TargetParser;
 import org.apache.cassandra.index.sasi.conf.ColumnIndex;
+import org.apache.cassandra.index.sasi.conf.IndexMode;
+import org.apache.cassandra.index.sasi.disk.OnDiskIndexBuilder.Mode;
 import org.apache.cassandra.index.sasi.disk.PerSSTableIndexWriter;
 import org.apache.cassandra.index.sasi.plan.QueryPlan;
 import org.apache.cassandra.index.transactions.IndexTransaction;
@@ -51,6 +53,7 @@ import org.apache.cassandra.io.sstable.format.SSTableReader;
 import org.apache.cassandra.notifications.*;
 import org.apache.cassandra.schema.IndexMetadata;
 import org.apache.cassandra.utils.FBUtilities;
+import org.apache.cassandra.utils.Pair;
 import org.apache.cassandra.utils.concurrent.OpOrder;
 
 public class SASIIndex implements Index, INotificationConsumer
@@ -95,7 +98,7 @@ public class SASIIndex implements Index, INotificationConsumer
         this.baseCfs = baseCfs;
         this.config = config;
 
-        ColumnDefinition column = CassandraIndex.parseTarget(baseCfs.metadata, 
config).left;
+        ColumnDefinition column = TargetParser.parse(baseCfs.metadata, 
config).left;
         this.index = new ColumnIndex(baseCfs.metadata.getKeyValidator(), 
column, config);
 
         Tracker tracker = baseCfs.getTracker();
@@ -116,8 +119,36 @@ public class SASIIndex implements Index, 
INotificationConsumer
         CompactionManager.instance.submitIndexBuild(new 
SASIIndexBuilder(baseCfs, toRebuild));
     }
 
-    public static Map<String, String> validateOptions(Map<String, String> 
options)
+    public static Map<String, String> validateOptions(Map<String, String> 
options, CFMetaData cfm)
     {
+        String targetColumn = options.get("target");
+        if (targetColumn == null)
+            throw new ConfigurationException("unknown target column");
+
+        Pair<ColumnDefinition, IndexTarget.Type> target = 
TargetParser.parse(cfm, targetColumn);
+        if (target == null)
+            throw new ConfigurationException("failed to retrieve target column 
for: " + targetColumn);
+
+        IndexMode.validateAnalyzer(options);
+
+        IndexMode mode = IndexMode.getMode(target.left, options);
+        if (mode.mode == Mode.SPARSE)
+        {
+            if (mode.isLiteral)
+                throw new ConfigurationException("SPARSE mode is only 
supported on non-literal columns.");
+
+            if (mode.isAnalyzed)
+                throw new ConfigurationException("SPARSE mode doesn't support 
analyzers.");
+        }
+
+        ColumnFamilyStore store = 
Schema.instance.getColumnFamilyStoreIncludingIndexes(Pair.create(cfm.ksName, 
cfm.cfName));
+        if (store != null && store.indexManager.listIndexes()
+                                               .stream()
+                                               .filter((index) -> 
index.dependsOn(target.left)
+                                                               && 
index.getClass().isAssignableFrom(SASIIndex.class))
+                                               .findFirst().isPresent())
+            throw new ConfigurationException("Index on '" + targetColumn + "' 
already exists, SASI doesn't support multiple indexes per column.");
+
         return Collections.emptyMap();
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/src/java/org/apache/cassandra/index/sasi/conf/ColumnIndex.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/sasi/conf/ColumnIndex.java 
b/src/java/org/apache/cassandra/index/sasi/conf/ColumnIndex.java
index 3363d21..29e7c28 100644
--- a/src/java/org/apache/cassandra/index/sasi/conf/ColumnIndex.java
+++ b/src/java/org/apache/cassandra/index/sasi/conf/ColumnIndex.java
@@ -32,7 +32,6 @@ import org.apache.cassandra.db.marshal.AsciiType;
 import org.apache.cassandra.db.marshal.UTF8Type;
 import org.apache.cassandra.db.rows.Cell;
 import org.apache.cassandra.db.rows.Row;
-import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.index.sasi.analyzer.AbstractAnalyzer;
 import org.apache.cassandra.index.sasi.conf.view.View;
 import org.apache.cassandra.index.sasi.disk.Token;
@@ -70,11 +69,6 @@ public class ColumnIndex
         this.component = new Component(Component.Type.SECONDARY_INDEX, 
String.format(FILE_NAME_FORMAT, getIndexName()));
     }
 
-    public void validate() throws ConfigurationException
-    {
-        mode.validate(config);
-    }
-
     /**
      * Initialize this column index with specific set of SSTables.
      *

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java 
b/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java
index 41ed718..b9c5653 100644
--- a/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java
+++ b/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java
@@ -71,27 +71,6 @@ public class IndexMode
         this.maxCompactionFlushMemoryInMb = maxFlushMemMb;
     }
 
-    public void validate(Optional<IndexMetadata> config) throws 
ConfigurationException
-    {
-        if (!config.isPresent())
-            return;
-
-        Map<String, String> indexOptions = config.get().options;
-        // validate that a valid analyzer class was provided if specified
-        if (indexOptions.containsKey(INDEX_ANALYZER_CLASS_OPTION))
-        {
-            try
-            {
-                Class.forName(indexOptions.get(INDEX_ANALYZER_CLASS_OPTION));
-            }
-            catch (ClassNotFoundException e)
-            {
-                throw new ConfigurationException(String.format("Invalid 
analyzer class option specified [%s]",
-                        indexOptions.get(INDEX_ANALYZER_CLASS_OPTION)));
-            }
-        }
-    }
-
     public AbstractAnalyzer getAnalyzer(AbstractType<?> validator)
     {
         AbstractAnalyzer analyzer = new NoOpAnalyzer();
@@ -114,15 +93,45 @@ public class IndexMode
         return analyzer;
     }
 
-    public static IndexMode getMode(ColumnDefinition column, 
Optional<IndexMetadata> config)
+    public static void validateAnalyzer(Map<String, String> indexOptions) 
throws ConfigurationException
+    {
+        // validate that a valid analyzer class was provided if specified
+        if (indexOptions.containsKey(INDEX_ANALYZER_CLASS_OPTION))
+        {
+            try
+            {
+                Class.forName(indexOptions.get(INDEX_ANALYZER_CLASS_OPTION));
+            }
+            catch (ClassNotFoundException e)
+            {
+                throw new ConfigurationException(String.format("Invalid 
analyzer class option specified [%s]",
+                                                               
indexOptions.get(INDEX_ANALYZER_CLASS_OPTION)));
+            }
+        }
+    }
+
+    public static IndexMode getMode(ColumnDefinition column, 
Optional<IndexMetadata> config) throws ConfigurationException
+    {
+        return getMode(column, config.isPresent() ? config.get().options : 
null);
+    }
+
+    public static IndexMode getMode(ColumnDefinition column, Map<String, 
String> indexOptions) throws ConfigurationException
     {
-        Map<String, String> indexOptions = config.isPresent() ? 
config.get().options : null;
         if (indexOptions == null || indexOptions.isEmpty())
             return IndexMode.NOT_INDEXED;
 
-        Mode mode = indexOptions.get(INDEX_MODE_OPTION) == null
-                        ? Mode.PREFIX
-                        : Mode.mode(indexOptions.get(INDEX_MODE_OPTION));
+        Mode mode;
+
+        try
+        {
+            mode = indexOptions.get(INDEX_MODE_OPTION) == null
+                            ? Mode.PREFIX
+                            : Mode.mode(indexOptions.get(INDEX_MODE_OPTION));
+        }
+        catch (IllegalArgumentException e)
+        {
+            throw new ConfigurationException("Incorrect index mode: " + 
indexOptions.get(INDEX_MODE_OPTION));
+        }
 
         boolean isAnalyzed = false;
         Class analyzerClass = null;
@@ -141,7 +150,7 @@ public class IndexMode
         }
         catch (ClassNotFoundException e)
         {
-            // should not happen as we already validated we could instantiate 
an instance in validateOptions()
+            // should not happen as we already validated we could instantiate 
an instance in validateAnalyzer()
             logger.error("Failed to find specified analyzer class [{}]. 
Falling back to default analyzer",
                          indexOptions.get(INDEX_ANALYZER_CLASS_OPTION));
         }
@@ -158,7 +167,7 @@ public class IndexMode
         }
         catch (Exception e)
         {
-            logger.error("failed to parse {} option, defaulting to 'false' for 
{} index.", INDEX_IS_LITERAL_OPTION, config.get().name);
+            logger.error("failed to parse {} option, defaulting to 'false'.", 
INDEX_IS_LITERAL_OPTION);
         }
 
         Long maxMemMb = indexOptions.get(INDEX_MAX_FLUSH_MEMORY_OPTION) == null

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/src/java/org/apache/cassandra/thrift/ThriftConversion.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/ThriftConversion.java 
b/src/java/org/apache/cassandra/thrift/ThriftConversion.java
index 3443b6e..35adddf 100644
--- a/src/java/org/apache/cassandra/thrift/ThriftConversion.java
+++ b/src/java/org/apache/cassandra/thrift/ThriftConversion.java
@@ -18,7 +18,6 @@
 package org.apache.cassandra.thrift;
 
 import java.util.*;
-import java.util.regex.Matcher;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
@@ -35,7 +34,7 @@ import 
org.apache.cassandra.db.compaction.AbstractCompactionStrategy;
 import org.apache.cassandra.db.filter.RowFilter;
 import org.apache.cassandra.db.marshal.*;
 import org.apache.cassandra.exceptions.*;
-import org.apache.cassandra.index.internal.CassandraIndex;
+import org.apache.cassandra.index.TargetParser;
 import org.apache.cassandra.io.compress.ICompressor;
 import org.apache.cassandra.locator.AbstractReplicationStrategy;
 import org.apache.cassandra.locator.LocalStrategy;
@@ -591,7 +590,7 @@ public class ThriftConversion
         IndexMetadata matchedIndex = null;
         for (IndexMetadata index : cfMetaData.getIndexes())
         {
-            Pair<ColumnDefinition, IndexTarget.Type> target  = 
CassandraIndex.parseTarget(cfMetaData, index);
+            Pair<ColumnDefinition, IndexTarget.Type> target  = 
TargetParser.parse(cfMetaData, index);
             if (target.left.equals(column))
             {
                 // we already found an index for this column, we've no option 
but to

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java 
b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
index 4045b6a..9cb2460 100644
--- a/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
+++ b/test/unit/org/apache/cassandra/index/internal/CustomCassandraIndex.java
@@ -8,6 +8,7 @@ import java.util.function.BiFunction;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
+import org.apache.cassandra.index.TargetParser;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -40,7 +41,6 @@ import org.apache.cassandra.utils.concurrent.Refs;
 
 import static org.apache.cassandra.index.internal.CassandraIndex.getFunctions;
 import static 
org.apache.cassandra.index.internal.CassandraIndex.indexCfsMetadata;
-import static org.apache.cassandra.index.internal.CassandraIndex.parseTarget;
 
 /**
  * Clone of KeysIndex used in CassandraIndexTest#testCustomIndexWithCFS to 
verify
@@ -139,7 +139,7 @@ public class CustomCassandraIndex implements Index
     private void setMetadata(IndexMetadata indexDef)
     {
         metadata = indexDef;
-        Pair<ColumnDefinition, IndexTarget.Type> target = 
parseTarget(baseCfs.metadata, indexDef);
+        Pair<ColumnDefinition, IndexTarget.Type> target = 
TargetParser.parse(baseCfs.metadata, indexDef);
         functions = getFunctions(indexDef, target);
         CFMetaData cfm = indexCfsMetadata(baseCfs.metadata, indexDef);
         indexCfs = ColumnFamilyStore.createColumnFamilyStore(baseCfs.keyspace,

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java 
b/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java
index b32bfc5..bd3bb0c 100644
--- a/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java
+++ b/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java
@@ -1688,6 +1688,77 @@ public class SASIIndexTest
         }
     }
 
+    @Test
+    public void testInvalidIndexOptions()
+    {
+        ColumnFamilyStore store = 
Keyspace.open(KS_NAME).getColumnFamilyStore(CF_NAME);
+
+        try
+        {
+            // invalid index mode
+            SASIIndex.validateOptions(new HashMap<String, String>()
+                                      {{ put("target", "address"); put("mode", 
"NORMAL"); }},
+                                      store.metadata);
+            Assert.fail();
+        }
+        catch (ConfigurationException e)
+        {
+            Assert.assertTrue(e.getMessage().contains("Incorrect index mode"));
+        }
+
+        try
+        {
+            // invalid SPARSE on the literal index
+            SASIIndex.validateOptions(new HashMap<String, String>()
+                                      {{ put("target", "address"); put("mode", 
"SPARSE"); }},
+                                      store.metadata);
+            Assert.fail();
+        }
+        catch (ConfigurationException e)
+        {
+            Assert.assertTrue(e.getMessage().contains("non-literal"));
+        }
+
+        try
+        {
+            // invalid SPARSE on the explicitly literal index
+            SASIIndex.validateOptions(new HashMap<String, String>()
+                                      {{ put("target", "height"); put("mode", 
"SPARSE"); put("is_literal", "true"); }},
+                    store.metadata);
+            Assert.fail();
+        }
+        catch (ConfigurationException e)
+        {
+            Assert.assertTrue(e.getMessage().contains("non-literal"));
+        }
+
+        try
+        {
+            //  SPARSE with analyzer
+            SASIIndex.validateOptions(new HashMap<String, String>()
+                                      {{ put("target", "height"); put("mode", 
"SPARSE"); put("analyzed", "true"); }},
+                                      store.metadata);
+            Assert.fail();
+        }
+        catch (ConfigurationException e)
+        {
+            Assert.assertTrue(e.getMessage().contains("doesn't support 
analyzers"));
+        }
+
+        try
+        {
+            // new index for column which already has a SASI index
+            SASIIndex.validateOptions(new HashMap<String, String>()
+                                      {{ put("target", "first_name"); 
put("mode", "PREFIX"); }},
+                                      store.metadata);
+            Assert.fail();
+        }
+        catch (ConfigurationException e)
+        {
+            Assert.assertTrue(e.getMessage().contains("already exists"));
+        }
+    }
+
     private static ColumnFamilyStore loadData(Map<String, Pair<String, 
Integer>> data, boolean forceFlush)
     {
         return loadData(data, System.currentTimeMillis(), forceFlush);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/f8e86eb3/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java 
b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
index feb2778..0340fd3 100644
--- a/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
+++ b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
@@ -34,7 +34,7 @@ import org.apache.cassandra.cql3.ColumnIdentifier;
 import org.apache.cassandra.cql3.functions.*;
 import org.apache.cassandra.db.*;
 import org.apache.cassandra.db.marshal.*;
-import org.apache.cassandra.index.internal.CassandraIndex;
+import org.apache.cassandra.index.TargetParser;
 import org.apache.cassandra.thrift.ThriftConversion;
 
 import static java.lang.String.format;
@@ -681,7 +681,7 @@ public class LegacySchemaMigratorTest
         // index targets can be parsed by CassandraIndex.parseTarget
         // which should be true for any pre-3.0 index
         for (IndexMetadata index : indexes)
-          if (CassandraIndex.parseTarget(table, index).left.equals(column))
+          if (TargetParser.parse(table, index).left.equals(column))
                 return Optional.of(index);
 
         return Optional.empty();

Reply via email to