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: Fix assertion error in ALTER TYPE RENAME
Date Mon, 24 Feb 2014 09:05:59 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk 462082323 -> 3b45d004c


Fix assertion error in ALTER TYPE RENAME

patch by mstepura & slebresn; reviewed by mstepura & slebresne for CASSANDRA-6705


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/e30f1114
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/e30f1114
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/e30f1114

Branch: refs/heads/trunk
Commit: e30f11143abfe3b03fe43aadf99a0804a62f3091
Parents: 0510d4e
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Mon Feb 24 10:04:48 2014 +0100
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Mon Feb 24 10:04:48 2014 +0100

----------------------------------------------------------------------
 CHANGES.txt                                         |  1 +
 .../cql3/statements/AlterTableStatement.java        |  9 ++++++++-
 .../cql3/statements/AlterTypeStatement.java         |  4 ++++
 .../db/composites/AbstractCellNameType.java         |  2 +-
 .../cassandra/db/composites/CellNameType.java       |  4 ++--
 .../db/composites/CompoundSparseCellNameType.java   |  4 ++--
 .../apache/cassandra/db/marshal/CollectionType.java | 16 ++++++++++++++++
 .../db/marshal/ColumnToCollectionType.java          |  3 ++-
 8 files changed, 36 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index b1fedd2..be2925a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -3,6 +3,7 @@
  * Fix ABTC NPE (CASSANDRA-6692)
  * Allow nodetool to use a file or prompt for password (CASSANDRA-6660)
  * Fix AIOOBE when concurrently accessing ABSC (CASSANDRA-6742)
+ * Fix assertion error in ALTER TYPE RENAME (CASSANDRA-6705)
 
 
 2.1.0-beta1

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/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 9c097a3..51b2865 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
@@ -109,7 +109,7 @@ public class AlterTableStatement extends SchemaAlteringStatement
                     if (cfm.isSuper())
                         throw new InvalidRequestException("Cannot use collection types with
Super column family");
 
-                    cfm.comparator = cfm.comparator.addCollection(columnName, (CollectionType)type);
+                    cfm.comparator = cfm.comparator.addOrUpdateCollection(columnName, (CollectionType)type);
                 }
 
                 Integer componentIndex = cfm.comparator.isCompound() ? cfm.comparator.clusteringPrefixSize()
: null;
@@ -186,6 +186,13 @@ public class AlterTableStatement extends SchemaAlteringStatement
                                                                            def.type.asCQL3Type(),
                                                                            validator));
 
+                        // For collections, if we alter the type, we need to update the comparator
too since it includes
+                        // the type too (note that isValueCompatibleWith above has validated
that the need type don't really
+                        // change the underlying sorting order, but we still don't want to
have a discrepancy between the type
+                        // in the comparator and the one in the ColumnDefinition as that
would be dodgy).
+                        if (validator.getType() instanceof CollectionType)
+                            cfm.comparator = cfm.comparator.addOrUpdateCollection(def.name,
(CollectionType)validator.getType());
+
                         break;
                 }
                 // In any case, we update the column definition

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
index 61a4e35..4ce9283 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
@@ -167,6 +167,10 @@ public abstract class AlterTypeStatement extends SchemaAlteringStatement
             case CLUSTERING_COLUMN:
                 cfm.comparator = CellNames.fromAbstractType(updateWith(cfm.comparator.asAbstractType(),
keyspace, toReplace, updated), cfm.comparator.isDense());
                 break;
+            default:
+                // If it's a collection, we still want to modify the comparator because the
collection is aliased in it
+                if (def.type instanceof CollectionType)
+                    cfm.comparator = CellNames.fromAbstractType(updateWith(cfm.comparator.asAbstractType(),
keyspace, toReplace, updated), cfm.comparator.isDense());
         }
         return true;
     }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/src/java/org/apache/cassandra/db/composites/AbstractCellNameType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/composites/AbstractCellNameType.java b/src/java/org/apache/cassandra/db/composites/AbstractCellNameType.java
index 6d4ee12..22aba09 100644
--- a/src/java/org/apache/cassandra/db/composites/AbstractCellNameType.java
+++ b/src/java/org/apache/cassandra/db/composites/AbstractCellNameType.java
@@ -198,7 +198,7 @@ public abstract class AbstractCellNameType extends AbstractCType implements
Cell
         throw new UnsupportedOperationException();
     }
 
-    public CellNameType addCollection(ColumnIdentifier columnName, CollectionType newCollection)
+    public CellNameType addOrUpdateCollection(ColumnIdentifier columnName, CollectionType
newCollection)
     {
         throw new UnsupportedOperationException();
     }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/src/java/org/apache/cassandra/db/composites/CellNameType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/composites/CellNameType.java b/src/java/org/apache/cassandra/db/composites/CellNameType.java
index 7128c91..6e8744a 100644
--- a/src/java/org/apache/cassandra/db/composites/CellNameType.java
+++ b/src/java/org/apache/cassandra/db/composites/CellNameType.java
@@ -99,10 +99,10 @@ public interface CellNameType extends CType
     public ColumnToCollectionType collectionType();
 
     /**
-     * Return the new type obtained by adding the new collection type for the provided column
name
+     * Return the new type obtained by adding/updating to the new collection type for the
provided column name
      * to this type.
      */
-    public CellNameType addCollection(ColumnIdentifier columnName, CollectionType newCollection);
+    public CellNameType addOrUpdateCollection(ColumnIdentifier columnName, CollectionType
newCollection);
 
     /**
      * Returns a new CellNameType that is equivalent to this one but with one

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/src/java/org/apache/cassandra/db/composites/CompoundSparseCellNameType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/composites/CompoundSparseCellNameType.java b/src/java/org/apache/cassandra/db/composites/CompoundSparseCellNameType.java
index e0cbc0f..44acf21 100644
--- a/src/java/org/apache/cassandra/db/composites/CompoundSparseCellNameType.java
+++ b/src/java/org/apache/cassandra/db/composites/CompoundSparseCellNameType.java
@@ -122,7 +122,7 @@ public class CompoundSparseCellNameType extends AbstractCompoundCellNameType
     }
 
     @Override
-    public CellNameType addCollection(ColumnIdentifier columnName, CollectionType newCollection)
+    public CellNameType addOrUpdateCollection(ColumnIdentifier columnName, CollectionType
newCollection)
     {
         return new WithCollection(clusteringType, ColumnToCollectionType.getInstance(Collections.singletonMap(columnName.bytes,
newCollection)), internedIds);
     }
@@ -244,7 +244,7 @@ public class CompoundSparseCellNameType extends AbstractCompoundCellNameType
         }
 
         @Override
-        public CellNameType addCollection(ColumnIdentifier columnName, CollectionType newCollection)
+        public CellNameType addOrUpdateCollection(ColumnIdentifier columnName, CollectionType
newCollection)
         {
             Map<ByteBuffer, CollectionType> newMap = new HashMap<>(collectionType.defined);
             newMap.put(columnName.bytes, newCollection);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/src/java/org/apache/cassandra/db/marshal/CollectionType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/CollectionType.java b/src/java/org/apache/cassandra/db/marshal/CollectionType.java
index b9816a6..fe672e4 100644
--- a/src/java/org/apache/cassandra/db/marshal/CollectionType.java
+++ b/src/java/org/apache/cassandra/db/marshal/CollectionType.java
@@ -94,6 +94,22 @@ public abstract class CollectionType<T> extends AbstractType<T>
         valueComparator().validate(bytes);
     }
 
+    @Override
+    public boolean isCompatibleWith(AbstractType<?> previous)
+    {
+        if (this == previous)
+            return true;
+
+        if (!getClass().equals(previous.getClass()))
+            return false;
+
+        CollectionType tprev = (CollectionType) previous;
+        // The name is part of the Cell name, so we need sorting compatibility, i.e. isCompatibleWith().
+        // But value is the Cell value, so isValueCompatibleWith() is enough
+        return this.nameComparator().isCompatibleWith(tprev.nameComparator())
+            && this.valueComparator().isValueCompatibleWith(tprev.valueComparator());
+    }
+
     public boolean isCollection()
     {
         return true;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e30f1114/src/java/org/apache/cassandra/db/marshal/ColumnToCollectionType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/ColumnToCollectionType.java b/src/java/org/apache/cassandra/db/marshal/ColumnToCollectionType.java
index a4f7857..a28b874 100644
--- a/src/java/org/apache/cassandra/db/marshal/ColumnToCollectionType.java
+++ b/src/java/org/apache/cassandra/db/marshal/ColumnToCollectionType.java
@@ -121,7 +121,8 @@ public class ColumnToCollectionType extends AbstractType<ByteBuffer>
         // We are compatible if we have all the definitions previous have (but we can have
more).
         for (Map.Entry<ByteBuffer, CollectionType> entry : prev.defined.entrySet())
         {
-            if (!entry.getValue().isCompatibleWith(defined.get(entry.getKey())))
+            CollectionType newType = defined.get(entry.getKey());
+            if (newType == null || !newType.isCompatibleWith(entry.getValue()))
                 return false;
         }
         return true;


Mime
View raw message