Updated Branches: refs/heads/trunk 8c062d807 -> 67435b528
Fix ALTER RENAME post-5125 patch by slebresne; reviewed by iamaleksey for CASSANDRA-5702 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/67435b52 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/67435b52 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/67435b52 Branch: refs/heads/trunk Commit: 67435b528dd474bd25fc90eaace6e6786f75ce04 Parents: 8c062d8 Author: Sylvain Lebresne <sylv...@datastax.com> Authored: Wed Jun 26 19:34:27 2013 +0200 Committer: Sylvain Lebresne <sylv...@datastax.com> Committed: Wed Jun 26 19:34:27 2013 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/config/CFMetaData.java | 237 ++++++++++++------- .../cassandra/config/ColumnDefinition.java | 10 +- .../org/apache/cassandra/config/KSMetaData.java | 1 + .../cassandra/cql/AlterTableStatement.java | 2 +- .../apache/cassandra/cql/QueryProcessor.java | 29 +-- .../org/apache/cassandra/cql/WhereClause.java | 11 +- .../org/apache/cassandra/cql3/CFDefinition.java | 33 +-- .../cql3/statements/AlterTableStatement.java | 26 -- src/java/org/apache/cassandra/db/DefsTable.java | 6 + .../apache/cassandra/config/CFMetaDataTest.java | 28 ++- .../org/apache/cassandra/config/DefsTest.java | 2 +- 12 files changed, 191 insertions(+), 195 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index c0bafb3..4973987 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -64,6 +64,7 @@ * Conditional create/drop ks/table/index statements in CQL3 (CASSANDRA-2737) * more pre-table creation property validation (CASSANDRA-5693) * Redesign repair messages (CASSANDRA-5426) + * Fix ALTER RENAME post-5125 (CASSANDRA-5702) 1.2.7 http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/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 15713bb..43d7297 100644 --- a/src/java/org/apache/cassandra/config/CFMetaData.java +++ b/src/java/org/apache/cassandra/config/CFMetaData.java @@ -22,6 +22,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; import java.util.*; import java.util.Map.Entry; @@ -80,7 +81,6 @@ public final class CFMetaData public final static int DEFAULT_MIN_COMPACTION_THRESHOLD = 4; public final static int DEFAULT_MAX_COMPACTION_THRESHOLD = 32; public final static Class<? extends AbstractCompactionStrategy> DEFAULT_COMPACTION_STRATEGY_CLASS = SizeTieredCompactionStrategy.class; - public final static ByteBuffer DEFAULT_KEY_NAME = ByteBufferUtil.bytes("KEY"); public final static Caching DEFAULT_CACHING_STRATEGY = Caching.KEYS_ONLY; public final static int DEFAULT_DEFAULT_TIME_TO_LIVE = 0; public final static SpeculativeRetry DEFAULT_SPECULATIVE_RETRY = new SpeculativeRetry(SpeculativeRetry.RetryType.NONE, 0); @@ -381,6 +381,10 @@ public final class CFMetaData * clustering key ones, those list are ordered by the "component index" of the * elements. */ + public static final String DEFAULT_KEY_ALIAS = "key"; + public static final String DEFAULT_COLUMN_ALIAS = "column"; + public static final String DEFAULT_VALUE_ALIAS = "value"; + private volatile Map<ByteBuffer, ColumnDefinition> column_metadata = new HashMap<ByteBuffer,ColumnDefinition>(); private volatile List<ColumnDefinition> partitionKeyColumns; // Always of size keyValidator.componentsCount, null padded if necessary private volatile List<ColumnDefinition> clusteringKeyColumns; // Of size comparator.componentsCount or comparator.componentsCount -1, null padded if necessary @@ -402,11 +406,11 @@ public final class CFMetaData public CFMetaData dcLocalReadRepairChance(double prop) {dcLocalReadRepairChance = prop; return this;} public CFMetaData replicateOnWrite(boolean prop) {replicateOnWrite = prop; return this;} public CFMetaData gcGraceSeconds(int prop) {gcGraceSeconds = prop; return this;} - public CFMetaData defaultValidator(AbstractType<?> prop) {defaultValidator = prop; updateCfDef(); return this;} - public CFMetaData keyValidator(AbstractType<?> prop) {keyValidator = prop; updateCfDef(); return this;} + public CFMetaData defaultValidator(AbstractType<?> prop) {defaultValidator = prop; return this;} + public CFMetaData keyValidator(AbstractType<?> prop) {keyValidator = prop; return this;} public CFMetaData minCompactionThreshold(int prop) {minCompactionThreshold = prop; return this;} public CFMetaData maxCompactionThreshold(int prop) {maxCompactionThreshold = prop; return this;} - public CFMetaData columnMetadata(Map<ByteBuffer,ColumnDefinition> prop) {column_metadata = prop; updateCfDef(); return this;} + public CFMetaData columnMetadata(Map<ByteBuffer,ColumnDefinition> prop) {column_metadata = prop; return this;} public CFMetaData compactionStrategyClass(Class<? extends AbstractCompactionStrategy> prop) {compactionStrategyClass = prop; return this;} public CFMetaData compactionStrategyOptions(Map<String, String> prop) {compactionStrategyOptions = prop; return this;} public CFMetaData compressionParameters(CompressionParameters prop) {compressionParameters = prop; return this;} @@ -439,8 +443,6 @@ public final class CFMetaData cfType = type; comparator = comp; cfId = id; - - updateCfDef(); // init cqlCfDef } public Map<String, Map<String, String>> getTriggers() @@ -460,7 +462,7 @@ public final class CFMetaData CreateColumnFamilyStatement statement = (CreateColumnFamilyStatement) QueryProcessor.parseStatement(cql).prepare().statement; CFMetaData cfmd = newSystemMetadata(keyspace, statement.columnFamily(), "", statement.comparator, null); statement.applyPropertiesTo(cfmd); - return cfmd; + return cfmd.rebuild(); } catch (RequestValidationException e) { @@ -524,7 +526,8 @@ public final class CFMetaData .triggers(parent.triggers) .compactionStrategyClass(parent.compactionStrategyClass) .compactionStrategyOptions(parent.compactionStrategyOptions) - .reloadSecondaryIndexMetadata(parent); + .reloadSecondaryIndexMetadata(parent) + .rebuild(); } public CFMetaData reloadSecondaryIndexMetadata(CFMetaData parent) @@ -577,7 +580,8 @@ public final class CFMetaData .memtableFlushPeriod(oldCFMD.memtableFlushPeriod) .populateIoCacheOnFlush(oldCFMD.populateIoCacheOnFlush) .droppedColumns(oldCFMD.droppedColumns) - .triggers(oldCFMD.getTriggers()); + .triggers(oldCFMD.getTriggers()) + .rebuild(); } /** @@ -662,12 +666,21 @@ public final class CFMetaData } // Used by CQL2 only. - public ByteBuffer getKeyName() + public String getCQL2KeyName() { if (partitionKeyColumns.size() > 1) throw new IllegalStateException("Cannot acces column family with composite key from CQL < 3.0.0"); - return partitionKeyColumns.get(0) == null ? DEFAULT_KEY_NAME : partitionKeyColumns.get(0).name; + try + { + // For compatibility sake, we uppercase if it's the default alias as we used to return it that way in resultsets. + String str = ByteBufferUtil.string(partitionKeyColumns.get(0).name); + return str.equalsIgnoreCase(DEFAULT_KEY_ALIAS) ? str.toUpperCase() : str; + } + catch (CharacterCodingException e) + { + throw new RuntimeException(e.getMessage(), e); + } } public CompressionParameters compressionParameters() @@ -921,7 +934,7 @@ public final class CFMetaData .defaultValidator(TypeParser.parse(cf_def.default_validation_class)) .columnMetadata(ColumnDefinition.fromThrift(cf_def.column_metadata, newCFMD.isSuper())) .compressionParameters(cp) - .updateCfDef(); + .rebuild(); } catch (SyntaxException e) { @@ -1012,7 +1025,7 @@ public final class CFMetaData compressionParameters = cfm.compressionParameters(); - updateCfDef(); + rebuild(); logger.debug("application result is {}", this); } @@ -1130,7 +1143,7 @@ public final class CFMetaData def.setMin_compaction_threshold(minCompactionThreshold); def.setMax_compaction_threshold(maxCompactionThreshold); // We only return the alias if only one is set since thrift don't know about multiple key aliases - if (partitionKeyColumns.size() == 1 && partitionKeyColumns.get(0) != null) + if (partitionKeyColumns.size() == 1) def.setKey_alias(partitionKeyColumns.get(0).name); List<org.apache.cassandra.thrift.ColumnDef> column_meta = new ArrayList<org.apache.cassandra.thrift.ColumnDef>(column_metadata.size()); for (ColumnDefinition cd : column_metadata.values()) @@ -1273,6 +1286,8 @@ public final class CFMetaData public CFMetaData validate() throws ConfigurationException { + rebuild(); + if (!isNameValid(ksName)) throw new ConfigurationException(String.format("Keyspace name must not be empty, more than %s characters long, or contain non-alphanumeric-underscore characters (got \"%s\")", Schema.NAME_LENGTH, ksName)); if (!isNameValid(cfName)) @@ -1372,16 +1387,13 @@ public final class CFMetaData private static void validateAlias(ColumnDefinition alias, String msg) throws ConfigurationException { - if (alias != null) + try { - try - { - UTF8Type.instance.validate(alias.name); - } - catch (MarshalException e) - { - throw new ConfigurationException(msg + " alias must be UTF8"); - } + UTF8Type.instance.validate(alias.name); + } + catch (MarshalException e) + { + throw new ConfigurationException(msg + " alias must be UTF8"); } } @@ -1632,7 +1644,6 @@ public final class CFMetaData if (!aliases.isEmpty() && aliases.get(0) != null) column_metadata.put(aliases.get(0), new ColumnDefinition(aliases.get(0), comparator, null, type)); } - updateCfDef(); } /** @@ -1661,7 +1672,7 @@ public final class CFMetaData { List<String> aliases = new ArrayList<String>(rawAliases.size()); for (ColumnDefinition rawAlias : rawAliases) - aliases.add(rawAlias == null ? null : UTF8Type.instance.compose(rawAlias.name)); + aliases.add(UTF8Type.instance.compose(rawAlias.name)); return json(aliases); } @@ -1669,7 +1680,7 @@ public final class CFMetaData { List<ByteBuffer> rawAliases = new ArrayList<ByteBuffer>(aliases.size()); for (String alias : aliases) - rawAliases.add(alias == null ? null : UTF8Type.instance.decompose(alias)); + rawAliases.add(UTF8Type.instance.decompose(alias)); return rawAliases; } @@ -1743,7 +1754,7 @@ public final class CFMetaData { for (ColumnDefinition cd : ColumnDefinition.fromSchema(serializedColumnDefinitions, cfDef)) cfDef.column_metadata.put(cd.name, cd); - return cfDef.updateCfDef(); + return cfDef.rebuild(); } public void addColumnDefinition(ColumnDefinition def) throws ConfigurationException @@ -1759,13 +1770,11 @@ public final class CFMetaData public void addOrReplaceColumnDefinition(ColumnDefinition def) { column_metadata.put(def.name, def); - updateCfDef(); } public boolean removeColumnDefinition(ColumnDefinition def) { boolean removed = column_metadata.remove(def.name) != null; - updateCfDef(); return removed; } @@ -1794,7 +1803,7 @@ public final class CFMetaData column_metadata.remove(def.name); } - private CFMetaData updateCfDef() + public CFMetaData rebuild() { /* * TODO: There is definitively some repetition between the CQL3 metadata stored in this @@ -1816,7 +1825,8 @@ public final class CFMetaData private void rebuildCQL3Metadata() { List<ColumnDefinition> pkCols = nullInitializedList(keyValidator.componentsCount()); - int nbCkCols = isDense(comparator, column_metadata.values()) + boolean isDense = isDense(comparator, column_metadata.values()); + int nbCkCols = isDense ? comparator.componentsCount() : comparator.componentsCount() - (hasCollection() ? 2 : 1); List<ColumnDefinition> ckCols = nullInitializedList(nbCkCols); @@ -1839,17 +1849,81 @@ public final class CFMetaData regCols.add(def); break; case COMPACT_VALUE: - assert compactCol == null : "There shouldn't be more than one compact value defined"; + assert compactCol == null : "There shouldn't be more than one compact value defined: got " + compactCol + " and " + def; compactCol = def; break; } } // Now actually assign the correct value. This is not atomic, but then again, updating CFMetaData is never atomic anyway. - partitionKeyColumns = pkCols; - clusteringKeyColumns = ckCols; + partitionKeyColumns = addDefaultKeyAliases(pkCols); + clusteringKeyColumns = addDefaultColumnAliases(ckCols); regularColumns = regCols; - compactValueColumn = compactCol; + compactValueColumn = addDefaultValueAlias(compactCol, isDense); + } + + private List<ColumnDefinition> addDefaultKeyAliases(List<ColumnDefinition> pkCols) + { + for (int i = 0; i < pkCols.size(); i++) + { + if (pkCols.get(i) == null) + { + Integer idx = null; + AbstractType<?> type = keyValidator; + if (keyValidator instanceof CompositeType) + { + idx = i; + type = ((CompositeType)keyValidator).types.get(i); + } + // For compatibility sake, we call the first alias 'key' rather than 'key1'. This + // is inconsistent with column alias, but it's probably not worth risking breaking compatibility now. + ByteBuffer name = ByteBufferUtil.bytes(i == 0 ? DEFAULT_KEY_ALIAS : DEFAULT_KEY_ALIAS + (i + 1)); + ColumnDefinition newDef = ColumnDefinition.partitionKeyDef(name, type, idx); + column_metadata.put(newDef.name, newDef); + pkCols.set(i, newDef); + } + } + return pkCols; + } + + private List<ColumnDefinition> addDefaultColumnAliases(List<ColumnDefinition> ckCols) + { + for (int i = 0; i < ckCols.size(); i++) + { + if (ckCols.get(i) == null) + { + Integer idx = null; + AbstractType<?> type = comparator; + if (comparator instanceof CompositeType) + { + idx = i; + type = ((CompositeType)comparator).types.get(i); + } + ByteBuffer name = ByteBufferUtil.bytes(DEFAULT_COLUMN_ALIAS + (i + 1)); + ColumnDefinition newDef = ColumnDefinition.clusteringKeyDef(name, type, idx); + column_metadata.put(newDef.name, newDef); + ckCols.set(i, newDef); + } + } + return ckCols; + } + + private ColumnDefinition addDefaultValueAlias(ColumnDefinition compactValueDef, boolean isDense) + { + if (isDense) + { + if (compactValueDef != null) + return compactValueDef; + + ColumnDefinition newDef = ColumnDefinition.compactValueDef(ByteBufferUtil.bytes(DEFAULT_VALUE_ALIAS), defaultValidator); + column_metadata.put(newDef.name, newDef); + return newDef; + } + else + { + assert compactValueDef == null; + return null; + } } private boolean hasCollection() @@ -1871,74 +1945,53 @@ public final class CFMetaData private static boolean isDense(AbstractType<?> comparator, Collection<ColumnDefinition> defs) { /* - * This is a bit subtle to compute because of thrift upgrades. A CQL3 - * CF will have all it's column metadata set up from creation, so - * checking isDense should just be looking the ColumnDefinition of - * type CLUSTERING_KEY having the biggest componentIndex and comparing that - * to comparator.componentsCount. - * However, thrift CF will have no or only some (through ALTER RENAME) - * metadata set and we still need to make our best effort at finding whether - * it is intended as a dense CF or not. + * As said above, this method is only here because we need to deal with thrift upgrades. + * Once a CF has been "upgraded", i.e. we've rebuilt and save its CQL3 metadata at least once, + * then checking for isDense amounts to looking whether the maximum componentIndex for the + * CLUSTERING_KEY ColumnDefinitions is equal to comparator.componentsCount() - 1 or not. + * + * But non-upgraded thrift CF will have no such CLUSTERING_KEY column definitions, so we need + * to infer that information without relying on them in that case. And for the most part this is + * easy, a CF that has at least one REGULAR definition is not dense. But the subtlety is that not + * having a REGULAR definition may not mean dense because of CQL3 definitions that have only the + * PRIMARY KEY defined. + * + * So we need to recognize those special case CQL3 table with only a primary key. If we have some + * clustering columns, we're fine as said above. So the only problem is that we cannot decide for + * sure if a CF without REGULAR columns nor CLUSTERING_KEY definition is meant to be dense, or if it + * has been created in CQL3 by say: + * CREATE TABLE test (k int PRIMARY KEY) + * in which case it should not be dense. However, we can limit our margin of error by assuming we are + * in the latter case only if the comparator is exactly CompositeType(UTF8Type). */ - - // First, we compute the number of clustering columns metadata actually defined (and - // whether there is some "hole" in the metadata) - boolean[] definedClusteringKeys = new boolean[comparator.componentsCount()]; boolean hasRegular = false; + int maxClusteringIdx = -1; for (ColumnDefinition def : defs) { switch (def.type) { case CLUSTERING_KEY: - definedClusteringKeys[def.componentIndex == null ? 0 : def.componentIndex] = true; + maxClusteringIdx = Math.max(maxClusteringIdx, def.componentIndex == null ? 0 : def.componentIndex); break; case REGULAR: hasRegular = true; break; } } - boolean hasNulls = false; - int maxIdx = -1; - for (int i = definedClusteringKeys.length - 1; i >= 0; i--) - { - if (maxIdx == -1) - { - if (definedClusteringKeys[i]) - maxIdx = i; - } - else - { - if (!definedClusteringKeys[i]) - hasNulls = true; - } - } - if (comparator instanceof CompositeType) - { - List<AbstractType<?>> types = ((CompositeType)comparator).types; - /* - * There was no real way to define a non-dense composite CF in thrift (the ColumnDefinition.componentIndex - * is not exposed), so consider dense anything that don't look like a CQL3 created CF. - * - * Note that this is not perfect: if someone upgrading from thrift "renames" all but - * the last column alias, the cf will be considered "sparse" and he will be stuck with - * that even though that might not be what he wants. But the simple workaround is - * for that user to rename all the aliases at the same time in the first place. - */ - AbstractType<?> lastType = types.get(types.size() - 1); - if (lastType instanceof ColumnToCollectionType) - return false; + return maxClusteringIdx >= 0 + ? maxClusteringIdx == comparator.componentsCount() - 1 + : !hasRegular && !isCQL3OnlyPKComparator(comparator); - return !(maxIdx == types.size() - 2 && lastType instanceof UTF8Type && !hasNulls); - } - else - { - /* - * For non-composite, we only need to "detect" case where the CF is clearly used as static. - * For that, just check if we have regular columns metadata sets up and no defined clustering key. - */ - return !(hasRegular && maxIdx == -1); - } + } + + private static boolean isCQL3OnlyPKComparator(AbstractType<?> comparator) + { + if (!(comparator instanceof CompositeType)) + return false; + + CompositeType ct = (CompositeType)comparator; + return ct.types.size() == 1 && ct.types.get(0) instanceof UTF8Type; } private static <T> List<T> nullInitializedList(int size) @@ -1950,9 +2003,7 @@ public final class CFMetaData } /** - * Returns whether this CFMetaData can be fully translated to a thrift - * definition, i.e. if it doesn't store information that have an equivalent - * in thrift CfDef. + * Returns whether this CFMetaData can be returned to thrift. */ public boolean isThriftCompatible() { @@ -1963,7 +2014,9 @@ public final class CFMetaData for (ColumnDefinition def : column_metadata.values()) { - if (!def.isThriftCompatible()) + // Non-REGULAR ColumnDefinition are not "thrift compatible" per-se, but it's ok because they hold metadata + // this is only of use to CQL3, so we will just skip them in toThrift. + if (def.type == ColumnDefinition.Type.REGULAR && !def.isThriftCompatible()) return false; } return true; http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/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 470e7d7..d63e2ce 100644 --- a/src/java/org/apache/cassandra/config/ColumnDefinition.java +++ b/src/java/org/apache/cassandra/config/ColumnDefinition.java @@ -51,7 +51,6 @@ public class ColumnDefinition * Note that thrift/CQL2 only know about definitions of type REGULAR (and * the ones whose componentIndex == null). */ - public enum Type { PARTITION_KEY, @@ -265,6 +264,10 @@ public class ColumnDefinition String index_name = null; Integer componentIndex = null; + Type type = result.has("type") + ? Enum.valueOf(Type.class, result.getString("type").toUpperCase()) + : Type.REGULAR; + if (result.has("index_type")) index_type = IndexType.valueOf(result.getString("index_type")); if (result.has("index_options")) @@ -274,12 +277,9 @@ public class ColumnDefinition if (result.has("component_index")) componentIndex = result.getInt("component_index"); // A ColumnDefinition for super columns applies to the column component - else if (cfm.isSuper()) + else if (type == Type.CLUSTERING_KEY && cfm.isSuper()) componentIndex = 1; - Type type = result.has("type") - ? Enum.valueOf(Type.class, result.getString("type").toUpperCase()) - : Type.REGULAR; cds.add(new ColumnDefinition(cfm.getComponentComparator(componentIndex, type).fromString(result.getString("column_name")), TypeParser.parse(result.getString("validator")), http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/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 f188134..1988a9f 100644 --- a/src/java/org/apache/cassandra/config/KSMetaData.java +++ b/src/java/org/apache/cassandra/config/KSMetaData.java @@ -315,6 +315,7 @@ public final class KSMetaData // value aliases. But that's what we want (see CFMetaData.fromSchemaNoColumns). cfm.addOrReplaceColumnDefinition(cd); } + cfm.rebuild(); } return cfms; http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/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 662d889..48e64c8 100644 --- a/src/java/org/apache/cassandra/cql/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql/AlterTableStatement.java @@ -80,7 +80,7 @@ public class AlterTableStatement case ALTER: // We only look for the first key alias which is ok for CQL2 ColumnDefinition partionKeyDef = cfm.partitionKeyColumns().get(0); - if (partionKeyDef != null && partionKeyDef.name.equals(columnName)) + if (partionKeyDef.name.equals(columnName)) { cfm.keyValidator(TypeParser.parse(validator)); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/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 a40bfea..ea179b4 100644 --- a/src/java/org/apache/cassandra/cql/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql/QueryProcessor.java @@ -69,7 +69,7 @@ public class QueryProcessor private static final Logger logger = LoggerFactory.getLogger(QueryProcessor.class); - public static final String DEFAULT_KEY_NAME = bufferToString(CFMetaData.DEFAULT_KEY_NAME); + public static final String DEFAULT_KEY_NAME = CFMetaData.DEFAULT_KEY_ALIAS.toUpperCase(); private static List<org.apache.cassandra.db.Row> getSlice(CFMetaData metadata, SelectStatement select, List<ByteBuffer> variables, long now) throws InvalidRequestException, ReadTimeoutException, UnavailableException, IsBootstrappingException, WriteTimeoutException @@ -117,7 +117,7 @@ public class QueryProcessor private static SortedSet<ByteBuffer> getColumnNames(SelectStatement select, CFMetaData metadata, List<ByteBuffer> variables) throws InvalidRequestException { - String keyString = getKeyString(metadata); + String keyString = metadata.getCQL2KeyName(); List<Term> selectColumnNames = select.getColumnNames(); SortedSet<ByteBuffer> columnNames = new TreeSet<ByteBuffer>(metadata.comparator); for (Term column : selectColumnNames) @@ -270,7 +270,7 @@ public class QueryProcessor public static void validateKeyAlias(CFMetaData cfm, String key) throws InvalidRequestException { assert key.toUpperCase().equals(key); // should always be uppercased by caller - String realKeyAlias = bufferToString(cfm.getKeyName()).toUpperCase(); + String realKeyAlias = cfm.getCQL2KeyName(); if (!realKeyAlias.equals(key)) throw new InvalidRequestException(String.format("Expected key '%s' to be present in WHERE clause for '%s'", realKeyAlias, cfm.cfName)); } @@ -422,9 +422,10 @@ public class QueryProcessor if (select.isFullWildcard()) { // prepend key - thriftColumns.add(new Column(metadata.getKeyName()).setValue(row.key.key).setTimestamp(-1)); - result.schema.name_types.put(metadata.getKeyName(), TypeParser.getShortName(AsciiType.instance)); - result.schema.value_types.put(metadata.getKeyName(), TypeParser.getShortName(metadata.getKeyValidator())); + ByteBuffer keyName = ByteBufferUtil.bytes(metadata.getCQL2KeyName()); + thriftColumns.add(new Column(keyName).setValue(row.key.key).setTimestamp(-1)); + result.schema.name_types.put(keyName, TypeParser.getShortName(AsciiType.instance)); + result.schema.value_types.put(keyName, TypeParser.getShortName(metadata.getKeyValidator())); } // preserve comparator order @@ -445,7 +446,7 @@ public class QueryProcessor } else { - String keyString = getKeyString(metadata); + String keyString = metadata.getCQL2KeyName(); // order columns in the order they were asked for for (Term term : select.getColumnNames()) @@ -816,20 +817,6 @@ public class QueryProcessor return new Column(c.name()).setValue(value).setTimestamp(c.timestamp()); } - private static String getKeyString(CFMetaData metadata) - { - String keyString; - try - { - keyString = ByteBufferUtil.string(metadata.getKeyName()); - } - catch (CharacterCodingException e) - { - throw new AssertionError(e); - } - return keyString; - } - private static CQLStatement getStatement(String queryStr) throws SyntaxException { try http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/src/java/org/apache/cassandra/cql/WhereClause.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql/WhereClause.java b/src/java/org/apache/cassandra/cql/WhereClause.java index d9b0f96..bba8338 100644 --- a/src/java/org/apache/cassandra/cql/WhereClause.java +++ b/src/java/org/apache/cassandra/cql/WhereClause.java @@ -139,16 +139,7 @@ public class WhereClause public void extractKeysFromColumns(CFMetaData cfm) { - String realKeyAlias = null; - try - { - // ThriftValidation ensures that key_alias is ascii - realKeyAlias = ByteBufferUtil.string(cfm.getKeyName()).toUpperCase(); - } - catch (CharacterCodingException e) - { - throw new RuntimeException(e); - } + String realKeyAlias = cfm.getCQL2KeyName(); if (!keys.isEmpty()) return; // we already have key(s) set (<key> IN (.., ...) construction used) http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/src/java/org/apache/cassandra/cql3/CFDefinition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/CFDefinition.java b/src/java/org/apache/cassandra/cql3/CFDefinition.java index 2375962..f92b50a 100644 --- a/src/java/org/apache/cassandra/cql3/CFDefinition.java +++ b/src/java/org/apache/cassandra/cql3/CFDefinition.java @@ -39,9 +39,6 @@ public class CFDefinition implements Iterable<CFDefinition.Name> { public static final AbstractType<?> definitionType = UTF8Type.instance; - private static final String DEFAULT_KEY_ALIAS = "key"; - private static final String DEFAULT_COLUMN_ALIAS = "column"; - private static final String DEFAULT_VALUE_ALIAS = "value"; public final CFMetaData cfm; // LinkedHashMap because the order does matter (it is the order in the composite type) @@ -67,7 +64,7 @@ public class CFDefinition implements Iterable<CFDefinition.Name> this.hasCompositeKey = cfm.getKeyValidator() instanceof CompositeType; for (int i = 0; i < cfm.partitionKeyColumns().size(); ++i) { - ColumnIdentifier id = getKeyId(cfm, i); + ColumnIdentifier id = new ColumnIdentifier(cfm.partitionKeyColumns().get(i).name, definitionType); this.keys.put(id, new Name(cfm.ksName, cfm.cfName, id, Name.Kind.KEY_ALIAS, i, cfm.getKeyValidator().getComponents().get(i))); } @@ -76,7 +73,7 @@ public class CFDefinition implements Iterable<CFDefinition.Name> this.isCompact = cfm.clusteringKeyColumns().size() == cfm.comparator.componentsCount(); for (int i = 0; i < cfm.clusteringKeyColumns().size(); ++i) { - ColumnIdentifier id = getColumnId(cfm, i); + ColumnIdentifier id = new ColumnIdentifier(cfm.clusteringKeyColumns().get(i).name, definitionType); this.columns.put(id, new Name(cfm.ksName, cfm.cfName, id, Name.Kind.COLUMN_ALIAS, i, cfm.comparator.getComponents().get(i))); } @@ -95,30 +92,6 @@ public class CFDefinition implements Iterable<CFDefinition.Name> } } - private static ColumnIdentifier getKeyId(CFMetaData cfm, int i) - { - List<ColumnDefinition> definedNames = cfm.partitionKeyColumns(); - // For compatibility sake, non-composite key default alias is 'key', not 'key1'. - return definedNames == null || i >= definedNames.size() || definedNames.get(i) == null - ? new ColumnIdentifier(i == 0 ? DEFAULT_KEY_ALIAS : DEFAULT_KEY_ALIAS + (i + 1), false) - : new ColumnIdentifier(definedNames.get(i).name, definitionType); - } - - private static ColumnIdentifier getColumnId(CFMetaData cfm, int i) - { - List<ColumnDefinition> definedNames = cfm.clusteringKeyColumns(); - return definedNames == null || i >= definedNames.size() || definedNames.get(i) == null - ? new ColumnIdentifier(DEFAULT_COLUMN_ALIAS + (i + 1), false) - : new ColumnIdentifier(definedNames.get(i).name, definitionType); - } - - private static ColumnIdentifier getValueId(CFMetaData cfm) - { - return cfm.compactValueColumn() == null - ? new ColumnIdentifier(DEFAULT_VALUE_ALIAS, false) - : new ColumnIdentifier(cfm.compactValueColumn().name, definitionType); - } - public ColumnToCollectionType getCollectionType() { if (!hasCollections) @@ -130,7 +103,7 @@ public class CFDefinition implements Iterable<CFDefinition.Name> private static Name createValue(CFMetaData cfm) { - ColumnIdentifier alias = getValueId(cfm); + ColumnIdentifier alias = new ColumnIdentifier(cfm.compactValueColumn().name, definitionType); // That's how we distinguish between 'no value alias because coming from thrift' and 'I explicitely did not // define a value' (see CreateColumnFamilyStatement) return alias.key.equals(ByteBufferUtil.EMPTY_BYTE_BUFFER) http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/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 03e1b4b..80835a6 100644 --- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java @@ -24,9 +24,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import com.google.common.base.Predicate; -import com.google.common.collect.Sets; - import org.apache.cassandra.auth.Permission; import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.config.ColumnDefinition; @@ -190,11 +187,6 @@ public class AlterTableStatement extends SchemaAlteringStatement cfProps.applyToCFMetadata(cfm); break; case RENAME: - if (cfm.partitionKeyColumns().size() < cfDef.keys.size() && !renamesAllAliases(cfDef, renames.keySet(), CFDefinition.Name.Kind.KEY_ALIAS, cfDef.keys.size())) - throw new InvalidRequestException("When upgrading from Thrift, all the columns of the (composite) partition key must be renamed together."); - if (cfm.clusteringKeyColumns().size() < cfDef.columns.size() && !renamesAllAliases(cfDef, renames.keySet(), CFDefinition.Name.Kind.COLUMN_ALIAS, cfDef.columns.size())) - throw new InvalidRequestException("When upgrading from Thrift, all the columns of the (composite) clustering key must be renamed together."); - for (Map.Entry<ColumnIdentifier, ColumnIdentifier> entry : renames.entrySet()) { ColumnIdentifier from = entry.getKey(); @@ -207,24 +199,6 @@ public class AlterTableStatement extends SchemaAlteringStatement MigrationManager.announceColumnFamilyUpdate(cfm, false); } - private static boolean renamesAllAliases(CFDefinition cfDef, Set<ColumnIdentifier> names, CFDefinition.Name.Kind kind, int expected) - { - int renamed = Sets.filter(names, isA(cfDef, kind)).size(); - return renamed == 0 || renamed == expected; - } - - private static Predicate<ColumnIdentifier> isA(final CFDefinition cfDef, final CFDefinition.Name.Kind kind) - { - return new Predicate<ColumnIdentifier>() - { - public boolean apply(ColumnIdentifier input) - { - CFDefinition.Name name = cfDef.get(input); - return name != null && name.kind == kind; - } - }; - } - public String toString() { return String.format("AlterTableStatement(name=%s, type=%s, column=%s, validator=%s)", http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/src/java/org/apache/cassandra/db/DefsTable.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/DefsTable.java b/src/java/org/apache/cassandra/db/DefsTable.java index 4b6c1c2..6008e75 100644 --- a/src/java/org/apache/cassandra/db/DefsTable.java +++ b/src/java/org/apache/cassandra/db/DefsTable.java @@ -24,6 +24,8 @@ import java.util.*; import com.google.common.collect.Iterables; import com.google.common.collect.MapDifference; import com.google.common.collect.Maps; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.cassandra.config.*; import org.apache.cassandra.db.compaction.CompactionManager; @@ -105,6 +107,8 @@ import org.apache.cassandra.utils.FBUtilities; */ public class DefsTable { + private static final Logger logger = LoggerFactory.getLogger(DefsTable.class); + /* saves keyspace definitions to system schema columnfamilies */ public static synchronized void save(Collection<KSMetaData> keyspaces) { @@ -342,6 +346,8 @@ public class DefsTable KSMetaData ksm = Schema.instance.getKSMetaData(cfm.ksName); ksm = KSMetaData.cloneWith(ksm, Iterables.concat(ksm.cfMetaData().values(), Collections.singleton(cfm))); + logger.info("Loading " + cfm); + Schema.instance.load(cfm); // make sure it's init-ed w/ the old definitions first, http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/test/unit/org/apache/cassandra/config/CFMetaDataTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/config/CFMetaDataTest.java b/test/unit/org/apache/cassandra/config/CFMetaDataTest.java index 3627cd8..73ba0a6 100644 --- a/test/unit/org/apache/cassandra/config/CFMetaDataTest.java +++ b/test/unit/org/apache/cassandra/config/CFMetaDataTest.java @@ -19,6 +19,7 @@ package org.apache.cassandra.config; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.HashMap; @@ -118,23 +119,32 @@ public class CFMetaDataTest extends SchemaLoader } } - private void checkInverses(CFMetaData cfm) throws Exception + private static CFMetaData withoutThriftIncompatible(CFMetaData cfm) { - DecoratedKey k = StorageService.getPartitioner().decorateKey(ByteBufferUtil.bytes(cfm.ksName)); + CFMetaData result = cfm.clone(); - // This is a nasty hack to work around the fact that non-null componentIndex - // are only used by CQL (so far) so we don't expose them through thrift - // There is a CFM with componentIndex defined in Keyspace2 which is used by - // ColumnFamilyStoreTest to verify index repair (CASSANDRA-2897) - for (ColumnDefinition def: cfm.allColumns()) + // This is a nasty hack to work around the fact that in thrift we exposes: + // - neither definition with a non-nulll componentIndex + // - nor non REGULAR definitions. + Iterator<ColumnDefinition> iter = result.allColumns().iterator(); + while (iter.hasNext()) { + ColumnDefinition def = iter.next(); // Remove what we know is not thrift compatible if (!def.isThriftCompatible()) - cfm.removeColumnDefinition(def); + iter.remove(); } + return result; + } + + private void checkInverses(CFMetaData cfm) throws Exception + { + DecoratedKey k = StorageService.getPartitioner().decorateKey(ByteBufferUtil.bytes(cfm.ksName)); // Test thrift conversion - assert cfm.equals(CFMetaData.fromThrift(cfm.toThrift())) : String.format("\n%s\n!=\n%s", cfm, CFMetaData.fromThrift(cfm.toThrift())); + CFMetaData before = withoutThriftIncompatible(cfm); + CFMetaData after = withoutThriftIncompatible(CFMetaData.fromThrift(before.toThrift())); + assert before.equals(after) : String.format("\n%s\n!=\n%s", before, after); // Test schema conversion RowMutation rm = cfm.toSchema(System.currentTimeMillis()); http://git-wip-us.apache.org/repos/asf/cassandra/blob/67435b52/test/unit/org/apache/cassandra/config/DefsTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/config/DefsTest.java b/test/unit/org/apache/cassandra/config/DefsTest.java index 52c4d36..9864398 100644 --- a/test/unit/org/apache/cassandra/config/DefsTest.java +++ b/test/unit/org/apache/cassandra/config/DefsTest.java @@ -515,7 +515,7 @@ public class DefsTest extends SchemaLoader CFMetaData meta = cfs.metadata.clone(); ColumnDefinition cdOld = meta.regularColumns().iterator().next(); ColumnDefinition cdNew = ColumnDefinition.regularDef(cdOld.name, cdOld.getValidator(), null); - meta.columnMetadata(Collections.singletonMap(cdOld.name, cdNew)); + meta.addOrReplaceColumnDefinition(cdNew); MigrationManager.announceColumnFamilyUpdate(meta, false); // check