SASI index options validation

patch by xedin; reviewed by beobal for CASSANDRA-11136

Author: xedin

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)

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.

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;
+    }
+}

src/java/org/apache/cassandra/index/internal/CassandraIndex.java

@@ -5,19 +5,17 @@
 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 Callable<?> getMetadataReloadTask(IndexMetadata indexDef)
     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 @@ private static String getSSTableNames(Collection<SSTableReader> sstables)
      */
     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 static final CFMetaData indexCfsMetadata(CFMetaData baseCfsMetadata, Inde
      */
     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,

src/java/org/apache/cassandra/index/sasi/SASIIndex.java

@@ -22,11 +22,10 @@
 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.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.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 SASIIndex(ColumnFamilyStore baseCfs, IndexMetadata config)
         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 SASIIndex(ColumnFamilyStore baseCfs, IndexMetadata config)
         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();
     }
 

src/java/org/apache/cassandra/index/sasi/conf/ColumnIndex.java

@@ -32,7 +32,6 @@
 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 ColumnIndex(AbstractType<?> keyValidator, ColumnDefinition column, IndexM
         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.
      *