Updated Branches: refs/heads/trunk 10777f2cc -> a801aae13
CQL3: Allow renaming PK columns patch by slebresne; reviewed by jbellis for CASSANDRA-4822 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/a801aae1 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/a801aae1 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/a801aae1 Branch: refs/heads/trunk Commit: a801aae13b00221ff724218c2cbd4e7f09124b9e Parents: ff817cf Author: Sylvain Lebresne <[email protected]> Authored: Wed Oct 17 10:33:36 2012 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Thu Oct 25 17:58:55 2012 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/config/CFMetaData.java | 15 +--- .../org/apache/cassandra/cql3/CFDefinition.java | 45 +++++++---- src/java/org/apache/cassandra/cql3/Cql.g | 8 ++- .../cql3/statements/AlterTableStatement.java | 58 +++++++++++++- 5 files changed, 92 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/a801aae1/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 8325a93..5761dc1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -37,6 +37,7 @@ * Fix Assertion error in cql3 select (CASSANDRA-4783) * Fix list prepend logic (CQL3) (CASSANDRA-4835) * Add booleans as literals in CQL3 (CASSANDRA-4776) + * Allow renaming PK columns in CQL3 (CASSANDRA-4822) Merged from 1.1: * fix get_paged_slice to wrap to next row correctly (CASSANDRA-4816) * fix indexing empty column values (CASSANDRA-4832) http://git-wip-us.apache.org/repos/asf/cassandra/blob/a801aae1/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 670e86b..1092da2 100644 --- a/src/java/org/apache/cassandra/config/CFMetaData.java +++ b/src/java/org/apache/cassandra/config/CFMetaData.java @@ -248,6 +248,7 @@ public final class CFMetaData private volatile AbstractType<?> keyValidator; // default BytesType (no-op), use comparator types private volatile int minCompactionThreshold; // default 4 private volatile int maxCompactionThreshold; // default 32 + // Both those aliases list can be null padded if only some of the position have been given an alias through ALTER TABLE .. RENAME private volatile List<ByteBuffer> keyAliases = new ArrayList<ByteBuffer>(); private volatile List<ByteBuffer> columnAliases = new ArrayList<ByteBuffer>(); private volatile ByteBuffer valueAlias; // default NULL @@ -795,23 +796,13 @@ public final class CFMetaData maxCompactionThreshold = cfm.maxCompactionThreshold; /* - * We don't allow changing the number of aliases (removal would be plain wrong and we've decided to no support addition since it would - * only make sense in very few cases). - * However, since thrift doesn't know about aliases (expect for the key aliases, but even then it doesn't support composite ones), we - * don't want to reject update that don't set the aliases at all. + * Because thrift updates don't know about aliases, we should ignore + * the case where the new aliases are empty. */ if (!cfm.keyAliases.isEmpty()) - { - if (keyAliases.size() != cfm.keyAliases.size()) - throw new ConfigurationException("Cannot change the number of key aliases"); keyAliases = cfm.keyAliases; - } if (!cfm.columnAliases.isEmpty()) - { - if (columnAliases.size() != cfm.columnAliases.size()) - throw new ConfigurationException("Cannot change the number of column aliases"); columnAliases = cfm.columnAliases; - } if (cfm.valueAlias != null) valueAlias = cfm.valueAlias; http://git-wip-us.apache.org/repos/asf/cassandra/blob/a801aae1/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 8e91ade..980fb68 100644 --- a/src/java/org/apache/cassandra/cql3/CFDefinition.java +++ b/src/java/org/apache/cassandra/cql3/CFDefinition.java @@ -86,27 +86,26 @@ public class CFDefinition implements Iterable<CFDefinition.Name> { this.isComposite = true; CompositeType composite = (CompositeType)cfm.comparator; - if (cfm.getColumnAliases().size() == composite.types.size()) - { - // "dense" composite - this.isCompact = true; - this.hasCollections = false; - for (int i = 0; i < composite.types.size(); i++) - { - ColumnIdentifier id = getColumnId(cfm, i); - this.columns.put(id, new Name(cfm.ksName, cfm.cfName, id, Name.Kind.COLUMN_ALIAS, i, composite.types.get(i))); - } - this.value = createValue(cfm); - } - else + /* + * We are a "sparse" composite, i.e. a non-compact one, if either: + * - the last type of the composite is a ColumnToCollectionType + * - or we have one less alias than of composite types and the last type is UTF8Type. + * + * 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. + */ + int last = composite.types.size() - 1; + AbstractType<?> lastType = composite.types.get(last); + if (lastType instanceof ColumnToCollectionType + || (cfm.getColumnAliases().size() == last && lastType instanceof UTF8Type)) { // "sparse" composite this.isCompact = false; this.value = null; assert cfm.getValueAlias() == null; // check for collection type - int last = composite.types.size() - 1; - AbstractType<?> lastType = composite.types.get(last); if (lastType instanceof ColumnToCollectionType) { --last; @@ -129,6 +128,18 @@ public class CFDefinition implements Iterable<CFDefinition.Name> this.metadata.put(id, new Name(cfm.ksName, cfm.cfName, id, Name.Kind.COLUMN_METADATA, def.getValue().getValidator())); } } + else + { + // "dense" composite + this.isCompact = true; + this.hasCollections = false; + for (int i = 0; i < composite.types.size(); i++) + { + ColumnIdentifier id = getColumnId(cfm, i); + this.columns.put(id, new Name(cfm.ksName, cfm.cfName, id, Name.Kind.COLUMN_ALIAS, i, composite.types.get(i))); + } + this.value = createValue(cfm); + } } else { @@ -164,7 +175,7 @@ public class CFDefinition implements Iterable<CFDefinition.Name> { List<ByteBuffer> definedNames = cfm.getKeyAliases(); // For compatibility sake, non-composite key default alias is 'key', not 'key1'. - return definedNames == null || i >= definedNames.size() + return definedNames == null || i >= definedNames.size() || cfm.getKeyAliases().get(i) == null ? new ColumnIdentifier(i == 0 ? DEFAULT_KEY_ALIAS : DEFAULT_KEY_ALIAS + (i + 1), false) : new ColumnIdentifier(cfm.getKeyAliases().get(i), definitionType); } @@ -172,7 +183,7 @@ public class CFDefinition implements Iterable<CFDefinition.Name> private static ColumnIdentifier getColumnId(CFMetaData cfm, int i) { List<ByteBuffer> definedNames = cfm.getColumnAliases(); - return definedNames == null || i >= definedNames.size() + return definedNames == null || i >= definedNames.size() || cfm.getColumnAliases().get(i) == null ? new ColumnIdentifier(DEFAULT_COLUMN_ALIAS + (i + 1), false) : new ColumnIdentifier(cfm.getColumnAliases().get(i), definitionType); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/a801aae1/src/java/org/apache/cassandra/cql3/Cql.g ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Cql.g b/src/java/org/apache/cassandra/cql3/Cql.g index 0f7668e..347bc1e 100644 --- a/src/java/org/apache/cassandra/cql3/Cql.g +++ b/src/java/org/apache/cassandra/cql3/Cql.g @@ -437,20 +437,25 @@ alterKeyspaceStatement returns [AlterKeyspaceStatement expr] * ALTER COLUMN FAMILY <CF> ADD <column> <newtype>; * ALTER COLUMN FAMILY <CF> DROP <column>; * ALTER COLUMN FAMILY <CF> WITH <property> = <value>; + * ALTER COLUMN FAMILY <CF> RENAME <column> TO <column>; */ alterTableStatement returns [AlterTableStatement expr] @init { AlterTableStatement.Type type = null; CFPropDefs props = new CFPropDefs(); + Map<ColumnIdentifier, ColumnIdentifier> renames = new HashMap<ColumnIdentifier, ColumnIdentifier>(); } : K_ALTER K_COLUMNFAMILY cf=columnFamilyName ( K_ALTER id=cident K_TYPE v=comparatorType { type = AlterTableStatement.Type.ALTER; } | K_ADD id=cident v=comparatorType { type = AlterTableStatement.Type.ADD; } | K_DROP id=cident { type = AlterTableStatement.Type.DROP; } | K_WITH properties[props] { type = AlterTableStatement.Type.OPTS; } + | K_RENAME { type = AlterTableStatement.Type.RENAME; } + id1=cident K_TO toId1=cident { renames.put(id1, toId1); } + ( K_AND idn=cident K_TO toIdn=cident { renames.put(idn, toIdn); } )* ) { - $expr = new AlterTableStatement(cf, type, id, v, props); + $expr = new AlterTableStatement(cf, type, id, v, props, renames); } ; @@ -783,6 +788,7 @@ K_VALUES: V A L U E S; K_TIMESTAMP: T I M E S T A M P; K_TTL: T T L; K_ALTER: A L T E R; +K_RENAME: R E N A M E; K_ADD: A D D; K_TYPE: T Y P E; K_COMPACT: C O M P A C T; http://git-wip-us.apache.org/repos/asf/cassandra/blob/a801aae1/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 40eb8f8..631ed75 100644 --- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java @@ -39,21 +39,23 @@ public class AlterTableStatement extends SchemaAlteringStatement { public static enum Type { - ADD, ALTER, DROP, OPTS + ADD, ALTER, DROP, OPTS, RENAME } public final Type oType; public final ParsedType validator; public final ColumnIdentifier columnName; private final CFPropDefs cfProps; + private final Map<ColumnIdentifier, ColumnIdentifier> renames; - public AlterTableStatement(CFName name, Type type, ColumnIdentifier columnName, ParsedType validator, CFPropDefs cfProps) + public AlterTableStatement(CFName name, Type type, ColumnIdentifier columnName, ParsedType validator, CFPropDefs cfProps, Map<ColumnIdentifier, ColumnIdentifier> renames) { super(name); this.oType = type; this.columnName = columnName; this.validator = validator; // used only for ADD/ALTER commands this.cfProps = cfProps; + this.renames = renames; } public void checkAccess(ClientState state) throws UnauthorizedException, InvalidRequestException @@ -67,7 +69,7 @@ public class AlterTableStatement extends SchemaAlteringStatement CFMetaData cfm = meta.clone(); CFDefinition cfDef = meta.getCfDef(); - CFDefinition.Name name = this.oType == Type.OPTS ? null : cfDef.get(columnName); + CFDefinition.Name name = columnName == null ? null : cfDef.get(columnName); switch (oType) { case ADD: @@ -116,7 +118,7 @@ public class AlterTableStatement extends SchemaAlteringStatement case ALTER: if (name == null) - throw new InvalidRequestException(String.format("Column %s was not found in CF %s", columnName, columnFamily())); + throw new InvalidRequestException(String.format("Column %s was not found in table %s", columnName, columnFamily())); switch (name.kind) { @@ -156,7 +158,7 @@ public class AlterTableStatement extends SchemaAlteringStatement if (cfDef.isCompact) throw new InvalidRequestException("Cannot drop columns from a compact CF"); if (name == null) - throw new InvalidRequestException(String.format("Column %s was not found in CF %s", columnName, columnFamily())); + throw new InvalidRequestException(String.format("Column %s was not found in table %s", columnName, columnFamily())); switch (name.kind) { @@ -182,11 +184,57 @@ public class AlterTableStatement extends SchemaAlteringStatement cfProps.validate(); cfProps.applyToCFMetadata(cfm); break; + case RENAME: + for (Map.Entry<ColumnIdentifier, ColumnIdentifier> entry : renames.entrySet()) + { + CFDefinition.Name from = cfDef.get(entry.getKey()); + ColumnIdentifier to = entry.getValue(); + if (from == null) + throw new InvalidRequestException(String.format("Column %s was not found in table %s", entry.getKey(), columnFamily())); + + CFDefinition.Name exists = cfDef.get(to); + if (exists != null) + throw new InvalidRequestException(String.format("Cannot rename column %s in table %s to %s; another column of that name already exist", from, columnFamily(), to)); + + switch (from.kind) + { + case KEY_ALIAS: + cfm.keyAliases(rename(from.position, to, cfm.getKeyAliases())); + break; + case COLUMN_ALIAS: + cfm.columnAliases(rename(from.position, to, cfm.getColumnAliases())); + break; + case VALUE_ALIAS: + cfm.valueAlias(to.key); + break; + case COLUMN_METADATA: + throw new InvalidRequestException(String.format("Cannot rename non PRIMARY KEY part %s", from)); + } + } + break; } MigrationManager.announceColumnFamilyUpdate(cfm); } + private static List<ByteBuffer> rename(int pos, ColumnIdentifier newName, List<ByteBuffer> aliases) + { + if (pos < aliases.size()) + { + List<ByteBuffer> newList = new ArrayList<ByteBuffer>(aliases); + newList.set(pos, newName.key); + return newList; + } + else + { + List<ByteBuffer> newList = new ArrayList<ByteBuffer>(pos + 1); + for (int i = 0; i < pos; ++i) + newList.add(i < aliases.size() ? aliases.get(i) : null); + newList.add(newName.key); + return newList; + } + } + public String toString() { return String.format("AlterTableStatement(name=%s, type=%s, column=%s, validator=%s)",
