improved validation of column_metadata

patch by jbellis; reviewed by gdusbabek for CASSANDRA-1813 git-svn-id: https://svn.apache.org/repos/asf/cassandra/branches/cassandra-0.7@1041932 13f79535-47bb-0310-9956-ffa450edef68
apache · Dec 3, 2010 · 59b566b · 59b566b
1 parent bf22c52
commit 59b566b
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 13 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -26,6 +26,7 @@ dev
  * fix consistencylevel calculations for NetworkTopologyStrategy
    (CASSANDRA-1804)
  * cli support index type enum names (CASSANDRA-1810)
+ * improved validation of column_metadata (CASSANDRA-1813)
 
 
 0.7.0-rc1

diff --git a/src/java/org/apache/cassandra/config/ColumnDefinition.java b/src/java/org/apache/cassandra/config/ColumnDefinition.java
@@ -103,7 +103,6 @@ public static ColumnDefinition inflate(org.apache.cassandra.avro.ColumnDef cd)
 
     public static ColumnDefinition fromColumnDef(ColumnDef thriftColumnDef) throws ConfigurationException
     {
-        validateIndexType(thriftColumnDef);
         return new ColumnDefinition(thriftColumnDef.name, thriftColumnDef.validation_class, thriftColumnDef.index_type, thriftColumnDef.index_name);
     }
 
@@ -123,10 +122,7 @@ public static Map<ByteBuffer, ColumnDefinition> fromColumnDef(List<ColumnDef> th
 
         Map<ByteBuffer, ColumnDefinition> cds = new TreeMap<ByteBuffer, ColumnDefinition>();
         for (ColumnDef thriftColumnDef : thriftDefs)
-        {
-            validateIndexType(thriftColumnDef);
             cds.put(thriftColumnDef.name, fromColumnDef(thriftColumnDef));
-        }
 
         return Collections.unmodifiableMap(cds);
     }
@@ -146,12 +142,6 @@ public static Map<ByteBuffer, ColumnDefinition> fromColumnDefs(Iterable<org.apac
         return Collections.unmodifiableMap(cds);
     }
 
-    public static void validateIndexType(org.apache.cassandra.thrift.ColumnDef thriftColumnDef) throws ConfigurationException
-    {
-        if ((thriftColumnDef.index_name != null) && (thriftColumnDef.index_type == null))
-            throw new ConfigurationException("index_name cannot be set if index_type is not also set");
-    }
-
     public static void validateIndexType(org.apache.cassandra.avro.ColumnDef avroColumnDef) throws ConfigurationException
     {
         if ((avroColumnDef.index_name != null) && (avroColumnDef.index_type == null))

diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyType.java b/src/java/org/apache/cassandra/db/ColumnFamilyType.java
@@ -25,11 +25,12 @@ public enum ColumnFamilyType
     Standard,
     Super;
 
-    public final static ColumnFamilyType create(String name)
+    public static ColumnFamilyType create(String name)
     {
         try
         {
-            return name == null ? null : ColumnFamilyType.valueOf(name);
+            // TODO thrift optional parameter in CfDef is leaking down here which it shouldn't
+            return name == null ? ColumnFamilyType.Standard : ColumnFamilyType.valueOf(name);
         }
         catch (IllegalArgumentException e)
         {

diff --git a/src/java/org/apache/cassandra/thrift/ThriftValidation.java b/src/java/org/apache/cassandra/thrift/ThriftValidation.java
@@ -383,14 +383,20 @@ public static void validateCfDef(CfDef cf_def) throws InvalidRequestException
     {
         try
         {
+            ColumnFamilyType cfType = ColumnFamilyType.create(cf_def.column_type);
+            if (cfType == null)
+                throw new InvalidRequestException("invalid column type " + cf_def.column_type);
+
             DatabaseDescriptor.getComparator(cf_def.comparator_type);
             DatabaseDescriptor.getComparator(cf_def.subcomparator_type);
             DatabaseDescriptor.getComparator(cf_def.default_validation_class);
+            if (cfType != ColumnFamilyType.Super && cf_def.subcomparator_type != null)
+                throw new InvalidRequestException("subcomparator_type is invalid for standard columns");
 
             if (cf_def.column_metadata == null)
                 return;
 
-            AbstractType comparator = cf_def.subcomparator_type == null
+            AbstractType comparator = cfType == ColumnFamilyType.Standard
                                     ? DatabaseDescriptor.getComparator(cf_def.comparator_type)
                                     : DatabaseDescriptor.getComparator(cf_def.subcomparator_type);
             for (ColumnDef c : cf_def.column_metadata)
@@ -406,6 +412,12 @@ public static void validateCfDef(CfDef cf_def) throws InvalidRequestException
                     throw new InvalidRequestException(String.format("Column name %s is not valid for comparator %s",
                                                                     FBUtilities.bytesToHex(c.name), cf_def.comparator_type));
                 }
+
+                if ((c.index_name != null) && (c.index_type == null))
+                    throw new ConfigurationException("index_name cannot be set without index_type");
+
+                if (cfType == ColumnFamilyType.Super && c.index_type != null)
+                    throw new InvalidRequestException("Secondary indexes are not supported on supercolumns");
             }
         }
         catch (ConfigurationException e)