You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2013/06/26 19:35:24 UTC
git commit: Fix ALTER RENAME post-5125
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 <sy...@datastax.com>
Authored: Wed Jun 26 19:34:27 2013 +0200
Committer: Sylvain Lebresne <sy...@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