cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [1/2] git commit: CQL3: Allow renaming PK columns
Date Thu, 25 Oct 2012 15:59:33 GMT
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 <sylvain@datastax.com>
Authored: Wed Oct 17 10:33:36 2012 +0200
Committer: Sylvain Lebresne <sylvain@datastax.com>
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)",


Mime
View raw message