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 2014/02/24 10:05:49 UTC
git commit: Fix assertion error in ALTER TYPE RENAME
Repository: cassandra
Updated Branches:
refs/heads/cassandra-2.1 0510d4e5b -> e30f11143
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/cassandra-2.1
Commit: e30f11143abfe3b03fe43aadf99a0804a62f3091
Parents: 0510d4e
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Feb 24 10:04:48 2014 +0100
Committer: Sylvain Lebresne <sy...@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;