Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id E35799860 for ; Thu, 22 Mar 2012 12:50:08 +0000 (UTC) Received: (qmail 82927 invoked by uid 500); 22 Mar 2012 12:50:08 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 82850 invoked by uid 500); 22 Mar 2012 12:50:08 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 82661 invoked by uid 99); 22 Mar 2012 12:50:08 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 22 Mar 2012 12:50:08 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id E4B5F86A2; Thu, 22 Mar 2012 12:50:07 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: slebresne@apache.org To: commits@cassandra.apache.org X-Mailer: ASF-Git Admin Mailer Subject: [1/4] git commit: Move CF and KS validation out of thrift. Message-Id: <20120322125007.E4B5F86A2@tyr.zones.apache.org> Date: Thu, 22 Mar 2012 12:50:07 +0000 (UTC) Updated Branches: refs/heads/cassandra-1.1 5787bb80e -> 86f5eaa9b Move CF and KS validation out of thrift. patch by slebresne; reviewed by jbellis for CASSANDRA-4037 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/86f5eaa9 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/86f5eaa9 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/86f5eaa9 Branch: refs/heads/cassandra-1.1 Commit: 86f5eaa9b68e3dda0052bb92658fb86ac5fddc48 Parents: 9f11058 Author: Sylvain Lebresne Authored: Fri Mar 16 09:30:34 2012 +0100 Committer: Sylvain Lebresne Committed: Thu Mar 22 13:49:23 2012 +0100 ---------------------------------------------------------------------- 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 | 64 +++--- .../cassandra/cql/CreateColumnFamilyStatement.java | 9 +- .../cassandra/cql/CreateKeyspaceStatement.java | 20 -- .../org/apache/cassandra/cql/QueryProcessor.java | 38 ++-- .../cql3/statements/AlterTableStatement.java | 49 ++-- .../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, 310 insertions(+), 380 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/86f5eaa9/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index cb7a9c6..7933e49 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,7 @@ 1.1.1-dev * optimize commitlog checksumming (CASSANDRA-3610) * identify and blacklist corrupted SSTables from future compactions (CASSANDRA-2261) + * Move CfDef and KsDef validation out of thrift (CASSANDRA-4037) 1.1-dev http://git-wip-us.apache.org/repos/asf/cassandra/blob/86f5eaa9/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 2c1df75..22b16d7 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; @@ -357,6 +359,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) @@ -622,8 +625,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 @@ -821,26 +823,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)); } } @@ -856,8 +845,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) @@ -879,9 +898,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 indexNames = new HashSet(); + 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. * @@ -1155,11 +1275,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/86f5eaa9/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/86f5eaa9/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 43c49db..0673115 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 options) throws ConfigurationException + { + Class 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.emptyList()); + } + public static KSMetaData cloneWith(KSMetaData ksm, Iterable 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 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.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/86f5eaa9/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 0557397..fcaf19c 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,25 +181,23 @@ 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); + 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())); if (!cfProps.compactionStrategyOptions.isEmpty()) { - cfDef.compaction_strategy_options = new HashMap(); + cfm.compactionStrategyOptions(new HashMap()); for (Map.Entry 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(); - for (Map.Entry 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/86f5eaa9/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 a5acd62..3fc4499 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"); @@ -188,8 +182,7 @@ public class CreateColumnFamilyStatement .keyAlias(keyAlias) .compactionStrategyClass(cfProps.compactionStrategyClass) .compactionStrategyOptions(cfProps.compactionStrategyOptions) - .compressionParameters(CompressionParameters.create(cfProps.compressionParameters)) - .validate(); + .compressionParameters(CompressionParameters.create(cfProps.compressionParameters)); } catch (ConfigurationException e) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/86f5eaa9/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/86f5eaa9/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 4fd59bb..1d1df83 100644 --- a/src/java/org/apache/cassandra/cql/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql/QueryProcessor.java @@ -680,13 +680,11 @@ public class QueryProcessor try { - KsDef ksd = new KsDef(create.getName(), - create.getStrategyClass(), - Collections.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) @@ -703,12 +701,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) @@ -731,19 +727,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.emptyMap()); + cd.setIndexName(createIdx.getIndexName()); columnExists = true; break; } @@ -751,11 +746,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) @@ -844,7 +838,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/86f5eaa9/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 9e08fbb..06129e0 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,35 +131,35 @@ 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); + 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())); if (!cfProps.compactionStrategyOptions.isEmpty()) { - cfDef.compaction_strategy_options = new HashMap(cfProps.compactionStrategyOptions); + cfm.compactionStrategyOptions(new HashMap(cfProps.compactionStrategyOptions)); } if (!cfProps.compressionParameters.isEmpty()) { - cfDef.compression_options = new HashMap(cfProps.compressionParameters); + cfm.compressionParameters(CompressionParameters.create(cfProps.compressionParameters)); } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/86f5eaa9/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 4b9b901..5ad532b 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()); } /** @@ -116,8 +114,7 @@ public class CreateColumnFamilyStatement extends SchemaAlteringStatement .columnAliases(columnAliases) .valueAlias(valueAlias) .compactionStrategyOptions(properties.compactionStrategyOptions) - .compressionParameters(CompressionParameters.create(properties.compressionParameters)) - .validate(); + .compressionParameters(CompressionParameters.create(properties.compressionParameters)); } catch (ConfigurationException e) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/86f5eaa9/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.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/86f5eaa9/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.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/86f5eaa9/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/86f5eaa9/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 5411568..5fd3a45 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/86f5eaa9/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 indexNames = new HashSet(); - 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 options = ks_def.strategy_options == null ? Collections.emptyMap() : ks_def.strategy_options; - TokenMetadata tmd = StorageService.instance.getTokenMetadata(); - IEndpointSnitch eps = DatabaseDescriptor.getEndpointSnitch(); - Class 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/86f5eaa9/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 utf8Column = new HashMap(); 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/86f5eaa9/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) {