cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [3/3] git commit: Move CF and KS validation out of thrift
Date Mon, 09 Apr 2012 16:44:42 GMT
Move CF and KS validation out of thrift

patch by slebresne; reviewed by jbellis for CASSANDRA-4037
(backport from cassandra-1.1 for CASSANDRA-4093 sakes)

Conflicts:

	CHANGES.txt
	src/java/org/apache/cassandra/cql/AlterTableStatement.java
	src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
	src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
	src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java


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

Branch: refs/heads/cassandra-1.1.0
Commit: 522730a0fc7ec0847c05867e769c275a160ba171
Parents: 9a6d0c7
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Fri Mar 16 09:30:34 2012 +0100
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Mon Apr 9 17:54:21 2012 +0200

----------------------------------------------------------------------
 CHANGES.txt                                        |    1 +
 .../org/apache/cassandra/config/CFMetaData.java    |  163 +++++++++++--
 .../apache/cassandra/config/ColumnDefinition.java  |   13 +-
 .../org/apache/cassandra/config/KSMetaData.java    |   43 +++-
 .../apache/cassandra/cql/AlterTableStatement.java  |   68 +++---
 .../cassandra/cql/CreateColumnFamilyStatement.java |    9 +-
 .../cassandra/cql/CreateKeyspaceStatement.java     |   20 --
 .../org/apache/cassandra/cql/QueryProcessor.java   |   38 ++--
 .../cql3/statements/AlterTableStatement.java       |   53 +++--
 .../statements/CreateColumnFamilyStatement.java    |    7 +-
 .../cql3/statements/CreateIndexStatement.java      |   21 +-
 .../cql3/statements/CreateKeyspaceStatement.java   |    6 +-
 .../cql3/statements/DropIndexStatement.java        |   22 +-
 .../apache/cassandra/thrift/CassandraServer.java   |   22 +-
 .../apache/cassandra/thrift/ThriftValidation.java  |  178 ---------------
 test/unit/org/apache/cassandra/SchemaLoader.java   |    6 +-
 .../cassandra/thrift/ThriftValidationTest.java     |   28 ++--
 17 files changed, 314 insertions(+), 384 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 4c4a41b..43ee218 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -11,6 +11,7 @@
    non-Windows platforms (CASSANDRA-4110)
  * fix terminination of the stress.java when errors were encountered
    (CASSANDRA-4128)
+ * Move CfDef and KsDef validation out of thrift (CASSANDRA-4037)
 Merged from 1.0:
  * allow short snitch names (CASSANDRA-4130)
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/config/CFMetaData.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java
index 403ed2b..6447a84 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -38,11 +38,13 @@ import org.apache.cassandra.cql3.QueryProcessor;
 import org.apache.cassandra.cql3.UntypedResultSet;
 import org.apache.cassandra.db.*;
 import org.apache.cassandra.db.compaction.AbstractCompactionStrategy;
+import org.apache.cassandra.db.index.SecondaryIndex;
 import org.apache.cassandra.db.marshal.*;
 import org.apache.cassandra.io.IColumnSerializer;
 import org.apache.cassandra.io.compress.CompressionParameters;
 import org.apache.cassandra.io.compress.SnappyCompressor;
 import org.apache.cassandra.thrift.CfDef;
+import org.apache.cassandra.thrift.IndexType;
 import org.apache.cassandra.thrift.InvalidRequestException;
 import org.apache.cassandra.utils.ByteBufferUtil;
 import org.apache.cassandra.utils.FBUtilities;
@@ -360,6 +362,7 @@ public final class CFMetaData
                       .replicateOnWrite(oldCFMD.replicateOnWrite)
                       .gcGraceSeconds(oldCFMD.gcGraceSeconds)
                       .defaultValidator(oldCFMD.defaultValidator)
+                      .keyValidator(oldCFMD.keyValidator)
                       .minCompactionThreshold(oldCFMD.minCompactionThreshold)
                       .maxCompactionThreshold(oldCFMD.maxCompactionThreshold)
                       .columnMetadata(oldCFMD.column_metadata)
@@ -626,8 +629,7 @@ public final class CFMetaData
                       .defaultValidator(TypeParser.parse(cf_def.default_validation_class))
                       .keyValidator(TypeParser.parse(cf_def.key_validation_class))
                       .columnMetadata(ColumnDefinition.fromThrift(cf_def.column_metadata))
-                      .compressionParameters(cp)
-                      .validate();
+                      .compressionParameters(cp);
     }
 
     public void reload() throws IOException
@@ -825,26 +827,13 @@ public final class CFMetaData
 
     /**
      * Convert a null index_name to appropriate default name according to column status
-     * @param cf_def Thrift ColumnFamily Definition
      */
-    public static void addDefaultIndexNames(org.apache.cassandra.thrift.CfDef cf_def) throws InvalidRequestException
+    public void addDefaultIndexNames() throws ConfigurationException
     {
-        if (cf_def.column_metadata == null)
-            return;
-
-        try
-        {
-            AbstractType<?> comparator = TypeParser.parse(cf_def.comparator_type);
-
-            for (org.apache.cassandra.thrift.ColumnDef column : cf_def.column_metadata)
-            {
-                if (column.index_type != null && column.index_name == null)
-                    column.index_name = getDefaultIndexName(cf_def.name, comparator, column.name);
-            }
-        }
-        catch (ConfigurationException e)
+        for (ColumnDefinition column : column_metadata.values())
         {
-            throw new InvalidRequestException(e.getMessage());
+            if (column.getIndexType() != null && column.getIndexName() == null)
+                column.setIndexName(getDefaultIndexName(cfName, comparator, column.name));
         }
     }
 
@@ -860,8 +849,38 @@ public final class CFMetaData
         return SuperColumn.serializer(subcolumnComparator);
     }
 
+    public static boolean isNameValid(String name)
+    {
+        return name != null && !name.isEmpty() && name.length() <= 32 && name.matches("\\w+");
+    }
+
+    public static boolean isIndexNameValid(String name)
+    {
+        return name != null && !name.isEmpty() && name.matches("\\w+");
+    }
+
     public CFMetaData validate() throws ConfigurationException
     {
+        if (!isNameValid(ksName))
+            throw new ConfigurationException(String.format("Invalid keyspace name: shouldn't be empty nor more than 32 character long (got \"%s\")", ksName));
+        if (!isNameValid(cfName))
+            throw new ConfigurationException(String.format("Invalid keyspace name: shouldn't be empty nor more than 32 character long (got \"%s\")", cfName));
+
+        if (cfType == null)
+            throw new ConfigurationException(String.format("Invalid column family type for %s", cfName));
+
+        if (cfType == ColumnFamilyType.Super)
+        {
+            if (subcolumnComparator == null)
+                throw new ConfigurationException(String.format("Missing subcolumn comparator for super column family %s", cfName));
+        }
+        else
+        {
+            if (subcolumnComparator != null)
+                throw new ConfigurationException(String.format("Subcolumn comparator (%s) is invalid for standard column family %s", subcolumnComparator, cfName));
+        }
+
+
         if (comparator instanceof CounterColumnType)
             throw new ConfigurationException("CounterColumnType is not a valid comparator");
         if (subcolumnComparator instanceof CounterColumnType)
@@ -883,9 +902,110 @@ public final class CFMetaData
                     throw new ConfigurationException("Cannot add a counter column (" + comparator.getString(def.name) + ") in a non counter column family");
         }
 
+        // check if any of the columns has name equal to the cf.key_alias
+        for (ColumnDefinition columndef : column_metadata.values())
+        {
+            if (keyAlias != null && keyAlias.equals(columndef.name))
+                throw new ConfigurationException("Cannot have key alias equals to a column name: " + UTF8Type.instance.compose(keyAlias));
+
+            for (ByteBuffer alias : columnAliases)
+                if (alias.equals(columndef.name))
+                    throw new ConfigurationException("Cannot have column alias equals to a column name: " + UTF8Type.instance.compose(alias));
+
+            if (valueAlias != null && valueAlias.equals(columndef.name))
+                throw new ConfigurationException("Cannot have value alias equals to a column name: " + UTF8Type.instance.compose(valueAlias));
+        }
+
+        validateAlias(keyAlias, "Key");
+        for (ByteBuffer alias : columnAliases)
+            validateAlias(alias, "Column");
+        validateAlias(valueAlias, "Value");
+
+        // initialize a set of names NOT in the CF under consideration
+        Set<String> indexNames = new HashSet<String>();
+        for (ColumnFamilyStore cfs : ColumnFamilyStore.all())
+        {
+            if (!cfs.getColumnFamilyName().equals(cfName))
+                for (ColumnDefinition cd : cfs.metadata.getColumn_metadata().values())
+                    indexNames.add(cd.getIndexName());
+        }
+
+        AbstractType<?> comparator = getColumnDefinitionComparator();
+
+        for (ColumnDefinition c : column_metadata.values())
+        {
+            try
+            {
+                comparator.validate(c.name);
+            }
+            catch (MarshalException e)
+            {
+                throw new ConfigurationException(String.format("Column name %s is not valid for comparator %s",
+                                                               ByteBufferUtil.bytesToHex(c.name), comparator));
+            }
+
+            if (c.getIndexType() == null)
+            {
+                if (c.getIndexName() != null)
+                    throw new ConfigurationException("Index name cannot be set without index type");
+            }
+            else
+            {
+                if (cfType == ColumnFamilyType.Super)
+                    throw new ConfigurationException("Secondary indexes are not supported on super column families");
+                if (!isIndexNameValid(c.getIndexName()))
+                    throw new ConfigurationException("Illegal index name " + c.getIndexName());
+                // check index names against this CF _and_ globally
+                if (indexNames.contains(c.getIndexName()))
+                    throw new ConfigurationException("Duplicate index name " + c.getIndexName());
+                indexNames.add(c.getIndexName());
+
+                if (c.getIndexType() == IndexType.CUSTOM)
+                {
+                    if (c.getIndexOptions() == null || !c.getIndexOptions().containsKey(SecondaryIndex.CUSTOM_INDEX_OPTION_NAME))
+                        throw new ConfigurationException("Required index option missing: " + SecondaryIndex.CUSTOM_INDEX_OPTION_NAME);
+                }
+
+                // This method validates the column metadata but does not intialize the index
+                SecondaryIndex.createInstance(null, c);
+            }
+        }
+
+        validateCompactionThresholds();
+
         return this;
     }
 
+    private static void validateAlias(ByteBuffer alias, String msg) throws ConfigurationException
+    {
+        if (alias != null)
+        {
+            if (!alias.hasRemaining())
+                throw new ConfigurationException(msg + " alias may not be empty");
+            try
+            {
+                UTF8Type.instance.validate(alias);
+            }
+            catch (MarshalException e)
+            {
+                throw new ConfigurationException(msg + " alias must be UTF8");
+            }
+        }
+    }
+
+    private void validateCompactionThresholds() throws ConfigurationException
+    {
+        if (maxCompactionThreshold == 0)
+            return;
+
+        if (minCompactionThreshold <= 1)
+            throw new ConfigurationException(String.format("Min compaction threshold cannot be less than 2 (got %d).", minCompactionThreshold));
+
+        if (minCompactionThreshold > maxCompactionThreshold)
+            throw new ConfigurationException(String.format("Min compaction threshold (got %d) cannot be greater than max compaction threshold (got %d)",
+                                                            minCompactionThreshold, maxCompactionThreshold));
+    }
+
     /**
      * Create schema mutations to update this metadata to provided new state.
      *
@@ -1159,11 +1279,6 @@ public final class CFMetaData
         return cqlCfDef;
     }
 
-    public static boolean isNameValid(String name)
-    {
-        return name.matches("\\w+");
-    }
-
     @Override
     public String toString()
     {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/config/ColumnDefinition.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/ColumnDefinition.java b/src/java/org/apache/cassandra/config/ColumnDefinition.java
index d8ac960..f6d8209 100644
--- a/src/java/org/apache/cassandra/config/ColumnDefinition.java
+++ b/src/java/org/apache/cassandra/config/ColumnDefinition.java
@@ -185,9 +185,16 @@ public class ColumnDefinition
 
     public void apply(ColumnDefinition def, AbstractType<?> comparator)  throws ConfigurationException
     {
-        // If an index is set (and not drop by this update), the validator shouldn't be change to a non-compatible one
-        if (getIndexType() != null && def.getIndexType() != null && !def.validator.isCompatibleWith(validator))
-            throw new ConfigurationException(String.format("Cannot modify validator to a non-compatible one for column %s since an index is set", comparator.getString(name)));
+        if (getIndexType() != null && def.getIndexType() != null)
+        {
+            // If an index is set (and not drop by this update), the validator shouldn't be change to a non-compatible one
+            if (!def.getValidator().isCompatibleWith(getValidator()))
+                throw new ConfigurationException(String.format("Cannot modify validator to a non-compatible one for column %s since an index is set", comparator.getString(name)));
+
+            assert getIndexName() != null;
+            if (!getIndexName().equals(def.getIndexName()))
+                throw new ConfigurationException("Cannot modify index name");
+        }
 
         setValidator(def.getValidator());
         setIndexType(def.getIndexType(), def.getIndexOptions());

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/config/KSMetaData.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/KSMetaData.java b/src/java/org/apache/cassandra/config/KSMetaData.java
index c10a94f..71230ec 100644
--- a/src/java/org/apache/cassandra/config/KSMetaData.java
+++ b/src/java/org/apache/cassandra/config/KSMetaData.java
@@ -31,9 +31,8 @@ import org.apache.cassandra.db.*;
 import org.apache.cassandra.db.filter.QueryPath;
 import org.apache.cassandra.db.marshal.AbstractType;
 import org.apache.cassandra.db.marshal.AsciiType;
-import org.apache.cassandra.locator.AbstractReplicationStrategy;
-import org.apache.cassandra.locator.LocalStrategy;
-import org.apache.cassandra.locator.NetworkTopologyStrategy;
+import org.apache.cassandra.locator.*;
+import org.apache.cassandra.service.StorageService;
 import org.apache.cassandra.thrift.CfDef;
 import org.apache.cassandra.thrift.KsDef;
 import org.apache.cassandra.thrift.ColumnDef;
@@ -60,6 +59,16 @@ public final class KSMetaData
         this.durableWrites = durableWrites;
     }
 
+    // For new user created keyspaces (through CQL)
+    public static KSMetaData newKeyspace(String name, String strategyName, Map<String, String> options) throws ConfigurationException
+    {
+        Class<? extends AbstractReplicationStrategy> cls = AbstractReplicationStrategy.getClass(strategyName);
+        if (cls.equals(LocalStrategy.class))
+            throw new ConfigurationException("Unable to use given strategy class: LocalStrategy is reserved for internal use.");
+
+        return new KSMetaData(name, cls, options, true, Collections.<CFMetaData>emptyList());
+    }
+
     public static KSMetaData cloneWith(KSMetaData ksm, Iterable<CFMetaData> cfDefs)
     {
         return new KSMetaData(ksm.name, ksm.strategyClass, ksm.strategyOptions, ksm.durableWrites, cfDefs);
@@ -141,8 +150,12 @@ public final class KSMetaData
 
     public static KSMetaData fromThrift(KsDef ksd, CFMetaData... cfDefs) throws ConfigurationException
     {
+        Class<? extends AbstractReplicationStrategy> cls = AbstractReplicationStrategy.getClass(ksd.strategy_class);
+        if (cls.equals(LocalStrategy.class))
+            throw new ConfigurationException("Unable to use given strategy class: LocalStrategy is reserved for internal use.");
+
         return new KSMetaData(ksd.name,
-                              AbstractReplicationStrategy.getClass(ksd.strategy_class),
+                              cls,
                               ksd.strategy_options == null ? Collections.<String, String>emptyMap() : ksd.strategy_options,
                               ksd.durable_writes,
                               Arrays.asList(cfDefs));
@@ -165,6 +178,23 @@ public final class KSMetaData
         return newState.toSchema(modificationTimestamp);
     }
 
+    public KSMetaData validate() throws ConfigurationException
+    {
+        if (!CFMetaData.isNameValid(name))
+            throw new ConfigurationException(String.format("Invalid keyspace name: shouldn't be empty nor more than 32 character long (got \"%s\")", name));
+
+        // Attempt to instantiate the ARS, which will throw a ConfigException if the strategy_options aren't fully formed
+        TokenMetadata tmd = StorageService.instance.getTokenMetadata();
+        IEndpointSnitch eps = DatabaseDescriptor.getEndpointSnitch();
+        AbstractReplicationStrategy.createReplicationStrategy(name, strategyClass, tmd, eps, strategyOptions);
+
+        for (CFMetaData cfm : cfMetaData.values())
+            cfm.validate();
+
+        return this;
+    }
+
+
     public KSMetaData reloadAttributes() throws IOException
     {
         Row ksDefRow = SystemTable.readSchemaRow(name);
@@ -271,9 +301,4 @@ public final class KSMetaData
 
         return cfms;
     }
-
-    public KSMetaData validate() throws ConfigurationException
-    {
-        return this;
-    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql/AlterTableStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/AlterTableStatement.java b/src/java/org/apache/cassandra/cql/AlterTableStatement.java
index b8b2e6f..46493b9 100644
--- a/src/java/org/apache/cassandra/cql/AlterTableStatement.java
+++ b/src/java/org/apache/cassandra/cql/AlterTableStatement.java
@@ -22,6 +22,7 @@ package org.apache.cassandra.cql;
 
 import org.apache.cassandra.config.*;
 import org.apache.cassandra.db.marshal.TypeParser;
+import org.apache.cassandra.io.compress.CompressionParameters;
 import org.apache.cassandra.thrift.CfDef;
 import org.apache.cassandra.thrift.ColumnDef;
 import org.apache.cassandra.thrift.InvalidRequestException;
@@ -67,11 +68,10 @@ public class AlterTableStatement
         }
     }
 
-    public CfDef getCfDef(String keyspace) throws ConfigurationException, InvalidRequestException
+    public CFMetaData getCFMetaData(String keyspace) throws ConfigurationException, InvalidRequestException
     {
         CFMetaData meta = Schema.instance.getCFMetaData(keyspace, columnFamily);
-
-        CfDef cfDef = meta.toThrift();
+        CFMetaData cfm = meta.clone();
 
         ByteBuffer columnName = this.oType == OperationType.OPTS ? null
                                                                  : meta.comparator.fromString(this.columnName);
@@ -79,28 +79,28 @@ public class AlterTableStatement
         switch (oType)
         {
             case ADD:
-                if (cfDef.key_alias != null && cfDef.key_alias.equals(columnName))
+                if (cfm.getKeyAlias() != null && cfm.getKeyAlias().equals(columnName))
                     throw new InvalidRequestException("Invalid column name: "
                                                       + this.columnName
                                                       + ", because it equals to key_alias.");
 
-                cfDef.column_metadata.add(new ColumnDefinition(columnName,
-                                                               TypeParser.parse(validator),
-                                                               null,
-                                                               null,
-                                                               null).toThrift());
+                cfm.addColumnDefinition(new ColumnDefinition(columnName,
+                                                             TypeParser.parse(validator),
+                                                             null,
+                                                             null,
+                                                             null));
                 break;
 
             case ALTER:
-                if (cfDef.key_alias != null && cfDef.key_alias.equals(columnName))
+                if (cfm.getKeyAlias() != null && cfm.getKeyAlias().equals(columnName))
                 {
-                    cfDef.setKey_validation_class(TypeParser.parse(validator).toString());
+                    cfm.keyValidator(TypeParser.parse(validator));
                 }
                 else
                 {
-                    ColumnDef toUpdate = null;
+                    ColumnDefinition toUpdate = null;
 
-                    for (ColumnDef columnDef : cfDef.column_metadata)
+                    for (ColumnDefinition columnDef : cfm.getColumn_metadata().values())
                     {
                         if (columnDef.name.equals(columnName))
                         {
@@ -114,14 +114,14 @@ public class AlterTableStatement
                                     this.columnName,
                                     columnFamily));
 
-                    toUpdate.setValidation_class(TypeParser.parse(validator).toString());
+                    toUpdate.setValidator(TypeParser.parse(validator));
                 }
                 break;
 
             case DROP:
-                ColumnDef toDelete = null;
+                ColumnDefinition toDelete = null;
 
-                for (ColumnDef columnDef : cfDef.column_metadata)
+                for (ColumnDefinition columnDef : cfm.getColumn_metadata().values())
                 {
                     if (columnDef.name.equals(columnName))
                     {
@@ -134,7 +134,7 @@ public class AlterTableStatement
                                                                     this.columnName,
                                                                     columnFamily));
 
-                cfDef.column_metadata.remove(toDelete);
+                cfm.removeColumnDefinition(toDelete);
                 break;
 
             case OPTS:
@@ -142,11 +142,11 @@ public class AlterTableStatement
                     throw new InvalidRequestException(String.format("ALTER COLUMNFAMILY WITH invoked, but no parameters found"));
 
                 cfProps.validate();
-                applyPropertiesToCfDef(cfDef, cfProps);
+                applyPropertiesToCFMetadata(cfm, cfProps);
                 break;
         }
 
-        return cfDef;
+        return cfm;
     }
 
     public String toString()
@@ -158,7 +158,7 @@ public class AlterTableStatement
                              validator);
     }
 
-    public static void applyPropertiesToCfDef(CfDef cfDef, CFPropDefs cfProps) throws InvalidRequestException
+    public static void applyPropertiesToCFMetadata(CFMetaData cfm, CFPropDefs cfProps) throws InvalidRequestException, ConfigurationException
     {
         if (cfProps.hasProperty(CFPropDefs.KW_COMPARATOR))
         {
@@ -166,13 +166,13 @@ public class AlterTableStatement
         }
         if (cfProps.hasProperty(CFPropDefs.KW_COMMENT))
         {
-            cfDef.comment = cfProps.getProperty(CFPropDefs.KW_COMMENT);
+            cfm.comment(cfProps.getProperty(CFPropDefs.KW_COMMENT));
         }
         if (cfProps.hasProperty(CFPropDefs.KW_DEFAULTVALIDATION))
         {
             try
             {
-                cfDef.default_validation_class = cfProps.getValidator().toString();
+                cfm.defaultValidator(cfProps.getValidator());
             }
             catch (ConfigurationException e)
             {
@@ -181,27 +181,25 @@ public class AlterTableStatement
             }
         }
 
-        cfDef.read_repair_chance = cfProps.getPropertyDouble(CFPropDefs.KW_READREPAIRCHANCE, cfDef.read_repair_chance);
-        cfDef.dclocal_read_repair_chance = cfProps.getPropertyDouble(CFPropDefs.KW_DCLOCALREADREPAIRCHANCE, cfDef.dclocal_read_repair_chance);
-        cfDef.gc_grace_seconds = cfProps.getPropertyInt(CFPropDefs.KW_GCGRACESECONDS, cfDef.gc_grace_seconds);
-        cfDef.replicate_on_write = cfProps.getPropertyBoolean(CFPropDefs.KW_REPLICATEONWRITE, cfDef.replicate_on_write);
-        cfDef.min_compaction_threshold = cfProps.getPropertyInt(CFPropDefs.KW_MINCOMPACTIONTHRESHOLD, cfDef.min_compaction_threshold);
-        cfDef.max_compaction_threshold = cfProps.getPropertyInt(CFPropDefs.KW_MAXCOMPACTIONTHRESHOLD, cfDef.max_compaction_threshold);
-        cfDef.caching = cfProps.getPropertyString(CFPropDefs.KW_CACHING, cfDef.caching);
-        cfDef.bloom_filter_fp_chance = cfProps.getPropertyDouble(CFPropDefs.KW_BF_FP_CHANCE, cfDef.bloom_filter_fp_chance);
+        cfm.readRepairChance(cfProps.getPropertyDouble(CFPropDefs.KW_READREPAIRCHANCE, cfm.getReadRepairChance()));
+        cfm.dcLocalReadRepairChance(cfProps.getPropertyDouble(CFPropDefs.KW_DCLOCALREADREPAIRCHANCE, cfm.getDcLocalReadRepair()));
+        cfm.gcGraceSeconds(cfProps.getPropertyInt(CFPropDefs.KW_GCGRACESECONDS, cfm.getGcGraceSeconds()));
+        cfm.replicateOnWrite(cfProps.getPropertyBoolean(CFPropDefs.KW_REPLICATEONWRITE, cfm.getReplicateOnWrite()));
+        cfm.minCompactionThreshold(cfProps.getPropertyInt(CFPropDefs.KW_MINCOMPACTIONTHRESHOLD, cfm.getMinCompactionThreshold()));
+        cfm.maxCompactionThreshold(cfProps.getPropertyInt(CFPropDefs.KW_MAXCOMPACTIONTHRESHOLD, cfm.getMaxCompactionThreshold()));
+        cfm.caching(CFMetaData.Caching.fromString(cfProps.getPropertyString(CFPropDefs.KW_CACHING, cfm.getCaching().toString())));
+        cfm.bloomFilterFpChance(cfProps.getPropertyDouble(CFPropDefs.KW_BF_FP_CHANCE, cfm.getBloomFilterFpChance()));
 
         if (!cfProps.compactionStrategyOptions.isEmpty())
         {
-            cfDef.compaction_strategy_options = new HashMap<String, String>();
+            cfm.compactionStrategyOptions(new HashMap<String, String>());
             for (Map.Entry<String, String> entry : cfProps.compactionStrategyOptions.entrySet())
-                cfDef.compaction_strategy_options.put(entry.getKey(), entry.getValue());
+                cfm.compactionStrategyOptions.put(entry.getKey(), entry.getValue());
         }
 
         if (!cfProps.compressionParameters.isEmpty())
         {
-            cfDef.compression_options = new HashMap<String, String>();
-            for (Map.Entry<String, String> entry : cfProps.compressionParameters.entrySet())
-                cfDef.compression_options.put(entry.getKey(), entry.getValue());
+            cfm.compressionParameters(CompressionParameters.create(cfProps.compressionParameters));
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java b/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
index 85d5ee4..edeb260 100644
--- a/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
+++ b/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
@@ -55,12 +55,6 @@ public class CreateColumnFamilyStatement
     {
         cfProps.validate();
 
-        // Column family name
-        if (!name.matches("\\w+"))
-            throw new InvalidRequestException(String.format("\"%s\" is not a valid column family name", name));
-        if (name.length() > 32)
-            throw new InvalidRequestException(String.format("Column family names shouldn't be more than 32 character long (got \"%s\")", name));
-
         // Ensure that exactly one key has been specified.
         if (keyValidator.size() < 1)
             throw new InvalidRequestException("You must specify a PRIMARY KEY");
@@ -190,8 +184,7 @@ public class CreateColumnFamilyStatement
                    .compactionStrategyOptions(cfProps.compactionStrategyOptions)
                    .compressionParameters(CompressionParameters.create(cfProps.compressionParameters))
                    .caching(CFMetaData.Caching.fromString(getPropertyString(CFPropDefs.KW_CACHING, CFMetaData.DEFAULT_CACHING_STRATEGY.toString())))
-                   .bloomFilterFpChance(getPropertyDouble(CFPropDefs.KW_BF_FP_CHANCE, CFMetaData.DEFAULT_BF_FP_CHANCE))
-                   .validate();
+                   .bloomFilterFpChance(getPropertyDouble(CFPropDefs.KW_BF_FP_CHANCE, CFMetaData.DEFAULT_BF_FP_CHANCE));
         }
         catch (ConfigurationException e)
         {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql/CreateKeyspaceStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/CreateKeyspaceStatement.java b/src/java/org/apache/cassandra/cql/CreateKeyspaceStatement.java
index a63ba7d..ce38b70 100644
--- a/src/java/org/apache/cassandra/cql/CreateKeyspaceStatement.java
+++ b/src/java/org/apache/cassandra/cql/CreateKeyspaceStatement.java
@@ -60,12 +60,6 @@ public class CreateKeyspaceStatement
      */
     public void validate() throws InvalidRequestException
     {
-        // keyspace name
-        if (!name.matches("\\w+"))
-            throw new InvalidRequestException(String.format("\"%s\" is not a valid keyspace name", name));
-        if (name.length() > 32)
-            throw new InvalidRequestException(String.format("Keyspace names shouldn't be more than 32 character long (got \"%s\")", name));
-
         // required
         if (!attrs.containsKey("strategy_class"))
             throw new InvalidRequestException("missing required argument \"strategy_class\"");
@@ -75,20 +69,6 @@ public class CreateKeyspaceStatement
         for (String key : attrs.keySet())
             if ((key.contains(":")) && (key.startsWith("strategy_options")))
                 strategyOptions.put(key.split(":")[1], attrs.get(key));
-
-        // trial run to let ARS validate class + per-class options
-        try
-        {
-            AbstractReplicationStrategy.createReplicationStrategy(name,
-                                                                  AbstractReplicationStrategy.getClass(strategyClass),
-                                                                  StorageService.instance.getTokenMetadata(),
-                                                                  DatabaseDescriptor.getEndpointSnitch(),
-                                                                  strategyOptions);
-        }
-        catch (ConfigurationException e)
-        {
-            throw new InvalidRequestException(e.getMessage());
-        }
     }
 
     public String getName()

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql/QueryProcessor.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/QueryProcessor.java b/src/java/org/apache/cassandra/cql/QueryProcessor.java
index 28ee353..0ea87bd 100644
--- a/src/java/org/apache/cassandra/cql/QueryProcessor.java
+++ b/src/java/org/apache/cassandra/cql/QueryProcessor.java
@@ -694,13 +694,11 @@ public class QueryProcessor
 
                 try
                 {
-                    KsDef ksd = new KsDef(create.getName(),
-                                          create.getStrategyClass(),
-                                          Collections.<CfDef>emptyList())
-                                .setStrategy_options(create.getStrategyOptions());
-                    ThriftValidation.validateKsDef(ksd);
-                    ThriftValidation.validateKeyspaceNotYetExisting(create.getName());
-                    MigrationManager.announceNewKeyspace(KSMetaData.fromThrift(ksd));
+                    KSMetaData ksm = KSMetaData.newKeyspace(create.getName(),
+                                                            create.getStrategyClass(),
+                                                            create.getStrategyOptions());
+                    ThriftValidation.validateKeyspaceNotYetExisting(ksm.name);
+                    MigrationManager.announceNewKeyspace(ksm);
                     validateSchemaIsSettled();
                 }
                 catch (ConfigurationException e)
@@ -717,12 +715,10 @@ public class QueryProcessor
                 CreateColumnFamilyStatement createCf = (CreateColumnFamilyStatement)statement.statement;
                 clientState.hasColumnFamilySchemaAccess(Permission.WRITE);
                 validateSchemaAgreement();
-                CFMetaData cfmd = createCf.getCFMetaData(keyspace, variables);
-                ThriftValidation.validateCfDef(cfmd.toThrift(), null);
 
                 try
                 {
-                    MigrationManager.announceNewColumnFamily(cfmd);
+                    MigrationManager.announceNewColumnFamily(createCf.getCFMetaData(keyspace, variables));
                     validateSchemaIsSettled();
                 }
                 catch (ConfigurationException e)
@@ -745,19 +741,18 @@ public class QueryProcessor
 
                 boolean columnExists = false;
                 ByteBuffer columnName = createIdx.getColumnName().getByteBuffer();
-                // mutating oldCfm directly would be bad, but mutating a Thrift copy is fine.  This also
-                // sets us up to use validateCfDef to check for index name collisions.
-                CfDef cf_def = oldCfm.toThrift();
-                for (ColumnDef cd : cf_def.column_metadata)
+                // mutating oldCfm directly would be bad, but mutating a copy is fine.
+                CFMetaData cfm = oldCfm.clone();
+                for (ColumnDefinition cd : cfm.getColumn_metadata().values())
                 {
                     if (cd.name.equals(columnName))
                     {
-                        if (cd.index_type != null)
+                        if (cd.getIndexType() != null)
                             throw new InvalidRequestException("Index already exists");
                         if (logger.isDebugEnabled())
-                            logger.debug("Updating column {} definition for index {}", oldCfm.comparator.getString(columnName), createIdx.getIndexName());
-                        cd.setIndex_type(IndexType.KEYS);
-                        cd.setIndex_name(createIdx.getIndexName());
+                            logger.debug("Updating column {} definition for index {}", cfm.comparator.getString(columnName), createIdx.getIndexName());
+                        cd.setIndexType(IndexType.KEYS, Collections.<String, String>emptyMap());
+                        cd.setIndexName(createIdx.getIndexName());
                         columnExists = true;
                         break;
                     }
@@ -765,11 +760,10 @@ public class QueryProcessor
                 if (!columnExists)
                     throw new InvalidRequestException("No column definition found for column " + oldCfm.comparator.getString(columnName));
 
-                CFMetaData.addDefaultIndexNames(cf_def);
-                ThriftValidation.validateCfDef(cf_def, oldCfm);
                 try
                 {
-                    MigrationManager.announceColumnFamilyUpdate(CFMetaData.fromThrift(cf_def));
+                    cfm.addDefaultIndexNames();
+                    MigrationManager.announceColumnFamilyUpdate(cfm);
                     validateSchemaIsSettled();
                 }
                 catch (ConfigurationException e)
@@ -858,7 +852,7 @@ public class QueryProcessor
 
                 try
                 {
-                    MigrationManager.announceColumnFamilyUpdate(CFMetaData.fromThrift(alterTable.getCfDef(keyspace)));
+                    MigrationManager.announceColumnFamilyUpdate(alterTable.getCFMetaData(keyspace));
                     validateSchemaIsSettled();
                 }
                 catch (ConfigurationException e)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
index 5fd9786..8dc19a8 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
@@ -23,6 +23,7 @@ import java.util.*;
 
 import org.apache.cassandra.cql3.*;
 import org.apache.cassandra.config.*;
+import org.apache.cassandra.io.compress.CompressionParameters;
 import org.apache.cassandra.service.MigrationManager;
 import org.apache.cassandra.thrift.CfDef;
 import org.apache.cassandra.thrift.ColumnDef;
@@ -53,7 +54,7 @@ public class AlterTableStatement extends SchemaAlteringStatement
     public void announceMigration() throws InvalidRequestException, ConfigurationException
     {
         CFMetaData meta = validateColumnFamily(keyspace(), columnFamily());
-        CfDef thriftDef = meta.toThrift();
+        CFMetaData cfm = meta.clone();
 
         CFDefinition cfDef = meta.getCfDef();
         CFDefinition.Name name = this.oType == Type.OPTS ? null : cfDef.get(columnName);
@@ -73,11 +74,11 @@ public class AlterTableStatement extends SchemaAlteringStatement
                             throw new InvalidRequestException(String.format("Invalid column name %s because it conflicts with an existing column", columnName));
                     }
                 }
-                thriftDef.column_metadata.add(new ColumnDefinition(columnName.key,
-                            CFPropDefs.parseType(validator),
-                            null,
-                            null,
-                            null).toThrift());
+                cfm.addColumnDefinition(new ColumnDefinition(columnName.key,
+                                                             CFPropDefs.parseType(validator),
+                                                             null,
+                                                             null,
+                                                             null));
                 break;
 
             case ALTER:
@@ -87,17 +88,17 @@ public class AlterTableStatement extends SchemaAlteringStatement
                 switch (name.kind)
                 {
                     case KEY_ALIAS:
-                        thriftDef.key_validation_class = CFPropDefs.parseType(validator).toString();
+                        cfm.keyValidator(CFPropDefs.parseType(validator));
                         break;
                     case COLUMN_ALIAS:
                         throw new InvalidRequestException(String.format("Cannot alter PRIMARY KEY part %s", columnName));
                     case VALUE_ALIAS:
-                        thriftDef.default_validation_class = CFPropDefs.parseType(validator).toString();
+                        cfm.defaultValidator(CFPropDefs.parseType(validator));
                         break;
                     case COLUMN_METADATA:
                         ColumnDefinition column = meta.getColumnDefinition(columnName.key);
                         column.setValidator(CFPropDefs.parseType(validator));
-                        thriftDef.column_metadata.add(column.toThrift());
+                        cfm.addColumnDefinition(column);
                         break;
                 }
                 break;
@@ -114,14 +115,14 @@ public class AlterTableStatement extends SchemaAlteringStatement
                     case COLUMN_ALIAS:
                         throw new InvalidRequestException(String.format("Cannot drop PRIMARY KEY part %s", columnName));
                     case COLUMN_METADATA:
-                        ColumnDef toDelete = null;
-                        for (ColumnDef columnDef : thriftDef.column_metadata)
+                        ColumnDefinition toDelete = null;
+                        for (ColumnDefinition columnDef : cfm.getColumn_metadata().values())
                         {
                             if (columnDef.name.equals(columnName.key))
                                 toDelete = columnDef;
                         }
                         assert toDelete != null;
-                        thriftDef.column_metadata.remove(toDelete);
+                        cfm.removeColumnDefinition(toDelete);
                         break;
                 }
                 break;
@@ -130,37 +131,37 @@ public class AlterTableStatement extends SchemaAlteringStatement
                     throw new InvalidRequestException(String.format("ALTER COLUMNFAMILY WITH invoked, but no parameters found"));
 
                 cfProps.validate();
-                applyPropertiesToCfDef(thriftDef, cfProps);
+                applyPropertiesToCFMetadata(cfm, cfProps);
                 break;
         }
 
-        MigrationManager.announceColumnFamilyUpdate(CFMetaData.fromThrift(thriftDef));
+        MigrationManager.announceColumnFamilyUpdate(cfm);
     }
 
-    public static void applyPropertiesToCfDef(CfDef cfDef, CFPropDefs cfProps) throws InvalidRequestException
+    public static void applyPropertiesToCFMetadata(CFMetaData cfm, CFPropDefs cfProps) throws InvalidRequestException, ConfigurationException
     {
         if (cfProps.hasProperty(CFPropDefs.KW_COMMENT))
         {
-            cfDef.comment = cfProps.get(CFPropDefs.KW_COMMENT);
+            cfm.comment(cfProps.get(CFPropDefs.KW_COMMENT));
         }
 
-        cfDef.read_repair_chance = cfProps.getDouble(CFPropDefs.KW_READREPAIRCHANCE, cfDef.read_repair_chance);
-        cfDef.dclocal_read_repair_chance = cfProps.getDouble(CFPropDefs.KW_DCLOCALREADREPAIRCHANCE, cfDef.dclocal_read_repair_chance);
-        cfDef.gc_grace_seconds = cfProps.getInt(CFPropDefs.KW_GCGRACESECONDS, cfDef.gc_grace_seconds);
-        cfDef.replicate_on_write = cfProps.getBoolean(CFPropDefs.KW_REPLICATEONWRITE, cfDef.replicate_on_write);
-        cfDef.min_compaction_threshold = cfProps.getInt(CFPropDefs.KW_MINCOMPACTIONTHRESHOLD, cfDef.min_compaction_threshold);
-        cfDef.max_compaction_threshold = cfProps.getInt(CFPropDefs.KW_MAXCOMPACTIONTHRESHOLD, cfDef.max_compaction_threshold);
-        cfDef.caching = cfProps.getString(CFPropDefs.KW_CACHING, cfDef.caching);
-        cfDef.bloom_filter_fp_chance = cfProps.getDouble(CFPropDefs.KW_BF_FP_CHANCE, cfDef.bloom_filter_fp_chance);
+        cfm.readRepairChance(cfProps.getDouble(CFPropDefs.KW_READREPAIRCHANCE, cfm.getReadRepairChance()));
+        cfm.dcLocalReadRepairChance(cfProps.getDouble(CFPropDefs.KW_DCLOCALREADREPAIRCHANCE, cfm.getDcLocalReadRepair()));
+        cfm.gcGraceSeconds(cfProps.getInt(CFPropDefs.KW_GCGRACESECONDS, cfm.getGcGraceSeconds()));
+        cfm.replicateOnWrite(cfProps.getBoolean(CFPropDefs.KW_REPLICATEONWRITE, cfm.getReplicateOnWrite()));
+        cfm.minCompactionThreshold(cfProps.getInt(CFPropDefs.KW_MINCOMPACTIONTHRESHOLD, cfm.getMinCompactionThreshold()));
+        cfm.maxCompactionThreshold(cfProps.getInt(CFPropDefs.KW_MAXCOMPACTIONTHRESHOLD, cfm.getMaxCompactionThreshold()));
+        cfm.caching(CFMetaData.Caching.fromString(cfProps.getString(CFPropDefs.KW_CACHING, cfm.getCaching().toString())));
+        cfm.bloomFilterFpChance(cfProps.getDouble(CFPropDefs.KW_BF_FP_CHANCE, cfm.getBloomFilterFpChance()));
 
         if (!cfProps.compactionStrategyOptions.isEmpty())
         {
-            cfDef.compaction_strategy_options = new HashMap<String, String>(cfProps.compactionStrategyOptions);
+            cfm.compactionStrategyOptions(new HashMap<String, String>(cfProps.compactionStrategyOptions));
         }
 
         if (!cfProps.compressionParameters.isEmpty())
         {
-            cfDef.compression_options = new HashMap<String, String>(cfProps.compressionParameters);
+            cfm.compressionParameters(CompressionParameters.create(cfProps.compressionParameters));
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java
index da9f4da..23bafd1 100644
--- a/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java
@@ -79,9 +79,7 @@ public class CreateColumnFamilyStatement extends SchemaAlteringStatement
 
     public void announceMigration() throws InvalidRequestException, ConfigurationException
     {
-        CFMetaData cfmd = getCFMetaData();
-        ThriftValidation.validateCfDef(cfmd.toThrift(), null);
-        MigrationManager.announceNewColumnFamily(cfmd);
+        MigrationManager.announceNewColumnFamily(getCFMetaData());
     }
 
     /**
@@ -118,8 +116,7 @@ public class CreateColumnFamilyStatement extends SchemaAlteringStatement
                    .compactionStrategyOptions(properties.compactionStrategyOptions)
                    .compressionParameters(CompressionParameters.create(properties.compressionParameters))
                    .caching(CFMetaData.Caching.fromString(properties.getString(CFPropDefs.KW_CACHING, CFMetaData.DEFAULT_CACHING_STRATEGY.toString())))
-                   .bloomFilterFpChance(properties.getDouble(CFPropDefs.KW_BF_FP_CHANCE, CFMetaData.DEFAULT_BF_FP_CHANCE))
-                   .validate();
+                   .bloomFilterFpChance(properties.getDouble(CFPropDefs.KW_BF_FP_CHANCE, CFMetaData.DEFAULT_BF_FP_CHANCE));
         }
         catch (ConfigurationException e)
         {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
index c42a854..9fe6f5b 100644
--- a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
@@ -18,10 +18,13 @@
  */
 package org.apache.cassandra.cql3.statements;
 
+import java.util.Collections;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.config.ColumnDefinition;
 import org.apache.cassandra.config.ConfigurationException;
 import org.apache.cassandra.cql3.*;
 import org.apache.cassandra.service.MigrationManager;
@@ -50,19 +53,18 @@ public class CreateIndexStatement extends SchemaAlteringStatement
     {
         CFMetaData oldCfm = ThriftValidation.validateColumnFamily(keyspace(), columnFamily());
         boolean columnExists = false;
-        // mutating oldCfm directly would be bad, but mutating a Thrift copy is fine.  This also
-        // sets us up to use validateCfDef to check for index name collisions.
-        CfDef cf_def = oldCfm.toThrift();
-        for (ColumnDef cd : cf_def.column_metadata)
+        // Mutating oldCfm directly would be bad so cloning.
+        CFMetaData cfm = oldCfm.clone();
+        for (ColumnDefinition cd : cfm.getColumn_metadata().values())
         {
             if (cd.name.equals(columnName.key))
             {
-                if (cd.index_type != null)
+                if (cd.getIndexType() != null)
                     throw new InvalidRequestException("Index already exists");
                 if (logger.isDebugEnabled())
                     logger.debug("Updating column {} definition for index {}", columnName, indexName);
-                cd.setIndex_type(IndexType.KEYS);
-                cd.setIndex_name(indexName);
+                cd.setIndexType(IndexType.KEYS, Collections.<String, String>emptyMap());
+                cd.setIndexName(indexName);
                 columnExists = true;
                 break;
             }
@@ -85,8 +87,7 @@ public class CreateIndexStatement extends SchemaAlteringStatement
             throw new InvalidRequestException("No column definition found for column " + columnName);
         }
 
-        CFMetaData.addDefaultIndexNames(cf_def);
-        ThriftValidation.validateCfDef(cf_def, oldCfm);
-        MigrationManager.announceColumnFamilyUpdate(CFMetaData.fromThrift(cf_def));
+        cfm.addDefaultIndexNames();
+        MigrationManager.announceColumnFamilyUpdate(cfm);
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
index c5de9d6..c1cbca2 100644
--- a/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/CreateKeyspaceStatement.java
@@ -104,10 +104,8 @@ public class CreateKeyspaceStatement extends SchemaAlteringStatement
 
     public void announceMigration() throws InvalidRequestException, ConfigurationException
     {
-        KsDef ksd = new KsDef(name, strategyClass, Collections.<CfDef>emptyList());
-        ksd.setStrategy_options(strategyOptions);
-        ThriftValidation.validateKsDef(ksd);
+        KSMetaData ksm = KSMetaData.newKeyspace(name, strategyClass, strategyOptions);
         ThriftValidation.validateKeyspaceNotYetExisting(name);
-        MigrationManager.announceNewKeyspace(KSMetaData.fromThrift(ksd));
+        MigrationManager.announceNewKeyspace(ksm);
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/cql3/statements/DropIndexStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/DropIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/DropIndexStatement.java
index 4959e81..ab47100 100644
--- a/src/java/org/apache/cassandra/cql3/statements/DropIndexStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/DropIndexStatement.java
@@ -39,32 +39,32 @@ public class DropIndexStatement extends SchemaAlteringStatement
 
     public void announceMigration() throws InvalidRequestException, ConfigurationException
     {
-        CfDef cfDef = null;
+        CFMetaData updatedCfm = null;
 
         KSMetaData ksm = Schema.instance.getTableDefinition(keyspace());
 
         for (CFMetaData cfm : ksm.cfMetaData().values())
         {
-            cfDef = getUpdatedCFDef(cfm.toThrift());
-            if (cfDef != null)
+            updatedCfm = getUpdatedCFMetadata(cfm);
+            if (updatedCfm != null)
                 break;
         }
 
-        if (cfDef == null)
+        if (updatedCfm == null)
             throw new InvalidRequestException("Index '" + index + "' could not be found in any of the column families of keyspace '" + keyspace() + "'");
 
-        MigrationManager.announceColumnFamilyUpdate(CFMetaData.fromThrift(cfDef));
+        MigrationManager.announceColumnFamilyUpdate(updatedCfm);
     }
 
-    private CfDef getUpdatedCFDef(CfDef cfDef) throws InvalidRequestException
+    private CFMetaData getUpdatedCFMetadata(CFMetaData cfm) throws InvalidRequestException
     {
-        for (ColumnDef column : cfDef.column_metadata)
+        for (ColumnDefinition column : cfm.getColumn_metadata().values())
         {
-            if (column.index_type != null && column.index_name != null && column.index_name.equals(index))
+            if (column.getIndexType() != null && column.getIndexName() != null && column.getIndexName().equals(index))
             {
-                column.index_name = null;
-                column.index_type = null;
-                return cfDef;
+                column.setIndexName(null);
+                column.setIndexType(null, null);
+                return cfm;
             }
         }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/thrift/CassandraServer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/CassandraServer.java b/src/java/org/apache/cassandra/thrift/CassandraServer.java
index 1611c2b..61a3233 100644
--- a/src/java/org/apache/cassandra/thrift/CassandraServer.java
+++ b/src/java/org/apache/cassandra/thrift/CassandraServer.java
@@ -912,14 +912,15 @@ public class CassandraServer implements Cassandra.Iface
     {
         logger.debug("add_column_family");
         state().hasColumnFamilySchemaAccess(Permission.WRITE);
-        CFMetaData.addDefaultIndexNames(cf_def);
-        ThriftValidation.validateCfDef(cf_def, null);
+
         validateSchemaAgreement();
 
         try
         {
             cf_def.unsetId(); // explicitly ignore any id set by client (Hector likes to set zero)
-            MigrationManager.announceNewColumnFamily(CFMetaData.fromThrift(cf_def));
+            CFMetaData cfm = CFMetaData.fromThrift(cf_def);
+            cfm.addDefaultIndexNames();
+            MigrationManager.announceNewColumnFamily(cfm);
             return Schema.instance.getVersion().toString();
         }
         catch (ConfigurationException e)
@@ -976,12 +977,10 @@ public class CassandraServer implements Cassandra.Iface
             for (CfDef cf_def : ks_def.cf_defs)
             {
                 cf_def.unsetId(); // explicitly ignore any id set by client (same as system_add_column_family)
-                CFMetaData.addDefaultIndexNames(cf_def);
-                ThriftValidation.validateCfDef(cf_def, null);
-                cfDefs.add(CFMetaData.fromThrift(cf_def));
+                CFMetaData cfm = CFMetaData.fromThrift(cf_def);
+                cfm.addDefaultIndexNames();
+                cfDefs.add(cfm);
             }
-
-            ThriftValidation.validateKsDef(ks_def);
             MigrationManager.announceNewKeyspace(KSMetaData.fromThrift(ks_def, cfDefs.toArray(new CFMetaData[cfDefs.size()])));
             return Schema.instance.getVersion().toString();
         }
@@ -1030,7 +1029,6 @@ public class CassandraServer implements Cassandra.Iface
 
         try
         {
-            ThriftValidation.validateKsDef(ks_def);
             MigrationManager.announceKeyspaceUpdate(KSMetaData.fromThrift(ks_def));
             return Schema.instance.getVersion().toString();
         }
@@ -1052,14 +1050,14 @@ public class CassandraServer implements Cassandra.Iface
         CFMetaData oldCfm = Schema.instance.getCFMetaData(cf_def.keyspace, cf_def.name);
         if (oldCfm == null)
             throw new InvalidRequestException("Could not find column family definition to modify.");
-        CFMetaData.addDefaultIndexNames(cf_def);
-        ThriftValidation.validateCfDef(cf_def, oldCfm);
         validateSchemaAgreement();
 
         try
         {
             CFMetaData.applyImplicitDefaults(cf_def);
-            MigrationManager.announceColumnFamilyUpdate(CFMetaData.fromThrift(cf_def));
+            CFMetaData cfm = CFMetaData.fromThrift(cf_def);
+            cfm.addDefaultIndexNames();
+            MigrationManager.announceColumnFamilyUpdate(cfm);
             return Schema.instance.getVersion().toString();
         }
         catch (ConfigurationException e)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/src/java/org/apache/cassandra/thrift/ThriftValidation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/ThriftValidation.java b/src/java/org/apache/cassandra/thrift/ThriftValidation.java
index eaaf61a..25c751c 100644
--- a/src/java/org/apache/cassandra/thrift/ThriftValidation.java
+++ b/src/java/org/apache/cassandra/thrift/ThriftValidation.java
@@ -28,7 +28,6 @@ import org.slf4j.LoggerFactory;
 
 import org.apache.cassandra.config.*;
 import org.apache.cassandra.db.*;
-import org.apache.cassandra.db.index.SecondaryIndex;
 import org.apache.cassandra.db.marshal.AbstractType;
 import org.apache.cassandra.db.marshal.AsciiType;
 import org.apache.cassandra.db.marshal.MarshalException;
@@ -580,130 +579,6 @@ public class ThriftValidation
         return isIndexed;
     }
 
-    public static void validateCfDef(CfDef cf_def, CFMetaData old) throws InvalidRequestException
-    {
-        try
-        {
-            if (cf_def.name.length() > 32)
-                throw new InvalidRequestException(String.format("Column family names shouldn't be more than 32 character long (got \"%s\")", cf_def.name));
-            if (!CFMetaData.isNameValid(cf_def.name))
-                throw new ConfigurationException(String.format("Invalid column family name. Should be only alphanumerical characters (got \"%s\")", cf_def.name));
-            if (cf_def.key_alias != null)
-            {
-                if (!cf_def.key_alias.hasRemaining())
-                    throw new InvalidRequestException("key_alias may not be empty");
-                try
-                {
-                    // it's hard to use a key in a select statement if we can't type it.
-                    // for now let's keep it simple and require ascii.
-                    AsciiType.instance.validate(cf_def.key_alias);
-                }
-                catch (MarshalException e)
-                {
-                    throw new InvalidRequestException("Key aliases must be ascii");
-                }
-            }
-
-            ColumnFamilyType cfType = ColumnFamilyType.create(cf_def.column_type);
-            if (cfType == null)
-                throw new InvalidRequestException("invalid column type " + cf_def.column_type);
-
-            TypeParser.parse(cf_def.key_validation_class);
-            TypeParser.parse(cf_def.comparator_type);
-            TypeParser.parse(cf_def.subcomparator_type);
-            TypeParser.parse(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;
-
-            if (cf_def.key_alias != null)
-            {
-                // check if any of the columns has name equal to the cf.key_alias
-                for (ColumnDef columnDef : cf_def.column_metadata)
-                {
-                    if (cf_def.key_alias.equals(columnDef.name))
-                        throw new InvalidRequestException("Invalid column name: "
-                                                          + AsciiType.instance.compose(cf_def.key_alias)
-                                                          + ", because it equals the key_alias");
-                }
-            }
-
-            // initialize a set of names NOT in the CF under consideration
-            Set<String> indexNames = new HashSet<String>();
-            for (ColumnFamilyStore cfs : ColumnFamilyStore.all())
-            {
-                if (!cfs.getColumnFamilyName().equals(cf_def.name))
-                    for (ColumnDefinition cd : cfs.metadata.getColumn_metadata().values())
-                        indexNames.add(cd.getIndexName());
-            }
-
-            AbstractType<?> comparator = CFMetaData.getColumnDefinitionComparator(cf_def);
-
-            for (ColumnDef c : cf_def.column_metadata)
-            {
-                TypeParser.parse(c.validation_class);
-
-                try
-                {
-                    comparator.validate(c.name);
-                }
-                catch (MarshalException e)
-                {
-                    throw new InvalidRequestException(String.format("Column name %s is not valid for comparator %s",
-                                                                    ByteBufferUtil.bytesToHex(c.name), comparator));
-                }
-
-                if (c.index_type == null)
-                {
-                    if (c.index_name != null)
-                        throw new ConfigurationException("index_name cannot be set without index_type");
-                }
-                else
-                {
-                    if (cfType == ColumnFamilyType.Super)
-                        throw new InvalidRequestException("Secondary indexes are not supported on supercolumns");
-                    assert c.index_name != null; // should have a default set by now if none was provided
-                    if (!CFMetaData.isNameValid(c.index_name))
-                        throw new InvalidRequestException("Illegal index name " + c.index_name);
-                    // check index names against this CF _and_ globally
-                    if (indexNames.contains(c.index_name))
-                        throw new InvalidRequestException("Duplicate index name " + c.index_name);
-                    indexNames.add(c.index_name);
-
-                    ColumnDefinition oldCd = old == null ? null : old.getColumnDefinition(c.name);
-                    if (oldCd != null && oldCd.getIndexType() != null)
-                    {
-                        assert oldCd.getIndexName() != null;
-                        if (!oldCd.getIndexName().equals(c.index_name))
-                            throw new InvalidRequestException("Cannot modify index name");
-                    }
-
-                    if (c.index_type == IndexType.CUSTOM)
-                    {
-                        if (c.index_options == null || !c.index_options.containsKey(SecondaryIndex.CUSTOM_INDEX_OPTION_NAME))
-                            throw new InvalidRequestException("Required index option missing: " + SecondaryIndex.CUSTOM_INDEX_OPTION_NAME);
-                    }
-
-                    // Create the index type and validate the options
-                    ColumnDefinition cdef = ColumnDefinition.fromThrift(c);
-
-                    // This method validates the column metadata but does not intialize the index
-                    SecondaryIndex.createInstance(null, cdef);
-                }
-            }
-            validateMinMaxCompactionThresholds(cf_def);
-
-            // validates compression parameters
-            CompressionParameters.create(cf_def.compression_options);
-        }
-        catch (ConfigurationException e)
-        {
-            throw new InvalidRequestException(e.getMessage());
-        }
-    }
-
     public static void validateCommutativeForWrite(CFMetaData metadata, ConsistencyLevel consistency) throws InvalidRequestException
     {
         if (consistency == ConsistencyLevel.ANY)
@@ -716,59 +591,6 @@ public class ThriftValidation
         }
     }
 
-    public static void validateKsDef(KsDef ks_def) throws ConfigurationException
-    {
-        if (ks_def.name.length() > 32)
-            throw new ConfigurationException(String.format("Keyspace names shouldn't be more than 32 character long (got \"%s\")", ks_def.name));
-        if (!CFMetaData.isNameValid(ks_def.name))
-            throw new ConfigurationException(String.format("Invalid keyspace name. Should be only alphanumerical characters (got \"%s\")", ks_def.name));
-
-        // Attempt to instantiate the ARS, which will throw a ConfigException if
-        //  the strategy_options aren't fully formed or if the ARS Classname is invalid.
-        Map<String, String> options = ks_def.strategy_options == null ? Collections.<String, String>emptyMap() : ks_def.strategy_options;
-        TokenMetadata tmd = StorageService.instance.getTokenMetadata();
-        IEndpointSnitch eps = DatabaseDescriptor.getEndpointSnitch();
-        Class<? extends AbstractReplicationStrategy> cls = AbstractReplicationStrategy.getClass(ks_def.strategy_class);
-
-        if (cls.equals(LocalStrategy.class))
-            throw new ConfigurationException("Unable to use given strategy class: LocalStrategy is reserved for internal use.");
-
-        AbstractReplicationStrategy.createReplicationStrategy(ks_def.name, cls, tmd, eps, options);
-    }
-
-    public static void validateMinMaxCompactionThresholds(org.apache.cassandra.thrift.CfDef cf_def) throws ConfigurationException
-    {
-        if (cf_def.isSetMin_compaction_threshold() && cf_def.isSetMax_compaction_threshold())
-        {
-            validateMinCompactionThreshold(cf_def.min_compaction_threshold, cf_def.max_compaction_threshold);
-        }
-        else if (cf_def.isSetMin_compaction_threshold())
-        {
-            validateMinCompactionThreshold(cf_def.min_compaction_threshold, CFMetaData.DEFAULT_MAX_COMPACTION_THRESHOLD);
-        }
-        else if (cf_def.isSetMax_compaction_threshold())
-        {
-            if (cf_def.max_compaction_threshold < CFMetaData.DEFAULT_MIN_COMPACTION_THRESHOLD && cf_def.max_compaction_threshold != 0)
-            {
-                throw new ConfigurationException("max_compaction_threshold cannot be less than min_compaction_threshold");
-            }
-        }
-        else
-        {
-            //Defaults are valid.
-        }
-    }
-
-    public static void validateMinCompactionThreshold(int min_compaction_threshold, int max_compaction_threshold) throws ConfigurationException
-    {
-        if (min_compaction_threshold <= 1)
-            throw new ConfigurationException("min_compaction_threshold cannot be less than 2.");
-
-        if (min_compaction_threshold > max_compaction_threshold && max_compaction_threshold != 0)
-            throw new ConfigurationException(String.format("min_compaction_threshold cannot be greater than max_compaction_threshold %d",
-                                                            max_compaction_threshold));
-    }
-
     public static void validateKeyspaceNotYetExisting(String newKsName) throws InvalidRequestException
     {
         // keyspace names must be unique case-insensitively because the keyspace name becomes the directory

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/test/unit/org/apache/cassandra/SchemaLoader.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/SchemaLoader.java b/test/unit/org/apache/cassandra/SchemaLoader.java
index e1d86d4..4edd8b0 100644
--- a/test/unit/org/apache/cassandra/SchemaLoader.java
+++ b/test/unit/org/apache/cassandra/SchemaLoader.java
@@ -129,14 +129,14 @@ public class SchemaLoader
             UTF8Type.instance,
             null,
             null,
-            "Column42"));
+            null));
         Map<ByteBuffer, ColumnDefinition> utf8Column = new HashMap<ByteBuffer, ColumnDefinition>();
         utf8Column.put(UTF8Type.instance.fromString("fortytwo"), new ColumnDefinition(
             UTF8Type.instance.fromString("fortytwo"),
             IntegerType.instance,
             null,
             null,
-            "Column42"));
+            null));
 
         // Keyspace 1
         schema.add(KSMetaData.testMetadata(ks1,
@@ -307,7 +307,7 @@ public class SchemaLoader
                    {{
                         ByteBuffer cName = ByteBuffer.wrap("birthdate".getBytes(Charsets.UTF_8));
                         IndexType keys = withIdxType ? IndexType.KEYS : null;
-                        put(cName, new ColumnDefinition(cName, LongType.instance, keys, null, ByteBufferUtil.bytesToHex(cName)));
+                        put(cName, new ColumnDefinition(cName, LongType.instance, keys, null, withIdxType ? ByteBufferUtil.bytesToHex(cName) : null));
                     }});
     }
     private static CFMetaData jdbcCFMD(String ksName, String cfName, AbstractType comp)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/522730a0/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java b/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
index 2a87d35..0166e4f 100644
--- a/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
+++ b/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
@@ -28,6 +28,8 @@ import org.junit.Test;
 import org.apache.cassandra.SchemaLoader;
 import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ConfigurationException;
+import org.apache.cassandra.config.ColumnDefinition;
+import org.apache.cassandra.config.KSMetaData;
 import org.apache.cassandra.config.Schema;
 import org.apache.cassandra.db.marshal.AsciiType;
 import org.apache.cassandra.db.marshal.UTF8Type;
@@ -102,42 +104,40 @@ public class ThriftValidationTest extends SchemaLoader
     public void testColumnNameEqualToKeyAlias()
     {
         CFMetaData metaData = Schema.instance.getCFMetaData("Keyspace1", "Standard1");
-        CfDef newMetadata = metaData.toThrift();
+        CFMetaData newMetadata = metaData.clone();
 
         boolean gotException = false;
 
         // add a key_alias = "id"
-        newMetadata.setKey_alias(AsciiType.instance.decompose("id"));
+        newMetadata.keyAlias(AsciiType.instance.decompose("id"));
 
         // should not throw IRE here
         try
         {
-            ThriftValidation.validateCfDef(newMetadata, metaData);
+            newMetadata.validate();
         }
-        catch (InvalidRequestException e)
+        catch (ConfigurationException e)
         {
             gotException = true;
         }
 
-        assert !gotException : "got unexpected InvalidRequestException";
+        assert !gotException : "got unexpected ConfigurationException";
 
         // add a column with name = "id"
-        newMetadata.addToColumn_metadata(new ColumnDef(UTF8Type.instance.decompose("id"),
-                                                       "org.apache.cassandra.db.marshal.UTF8Type"));
-
+        newMetadata.addColumnDefinition(ColumnDefinition.utf8("id"));
 
         gotException = false;
 
         try
         {
-            ThriftValidation.validateCfDef(newMetadata, metaData);
+            newMetadata.validate();
         }
-        catch (InvalidRequestException e)
+        catch (ConfigurationException e)
         {
             gotException = true;
         }
 
-        assert gotException : "expected InvalidRequestException but not received.";
+        assert gotException : "expected ConfigurationException but not received.";
     }
 
     @Test
@@ -152,7 +152,7 @@ public class ThriftValidationTest extends SchemaLoader
 
         try
         {
-            ThriftValidation.validateKsDef(ks_def);
+            KSMetaData.fromThrift(ks_def).validate();
         }
         catch (ConfigurationException e)
         {
@@ -167,7 +167,7 @@ public class ThriftValidationTest extends SchemaLoader
 
         try
         {
-            ThriftValidation.validateKsDef(ks_def);
+            KSMetaData.fromThrift(ks_def).validate();
         }
         catch (ConfigurationException e)
         {
@@ -182,7 +182,7 @@ public class ThriftValidationTest extends SchemaLoader
 
         try
         {
-            ThriftValidation.validateKsDef(ks_def);
+            KSMetaData.fromThrift(ks_def).validate();
         }
         catch (ConfigurationException e)
         {


Mime
View raw message