diff --git a/CHANGES.txt b/CHANGES.txt index 2a1cf1736b..676c464003 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,6 +7,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 diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java index 9c097a33f1..51b28659d9 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 diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java index 61a4e35c7e..4ce92839ee 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; } diff --git a/src/java/org/apache/cassandra/db/composites/AbstractCellNameType.java b/src/java/org/apache/cassandra/db/composites/AbstractCellNameType.java index 6d4ee1282f..22aba09e9a 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(); } diff --git a/src/java/org/apache/cassandra/db/composites/CellNameType.java b/src/java/org/apache/cassandra/db/composites/CellNameType.java index e4fa0e2d9a..698e67076e 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 diff --git a/src/java/org/apache/cassandra/db/composites/CompoundSparseCellNameType.java b/src/java/org/apache/cassandra/db/composites/CompoundSparseCellNameType.java index e0cbc0f97b..44acf21a1b 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 newMap = new HashMap<>(collectionType.defined); newMap.put(columnName.bytes, newCollection); diff --git a/src/java/org/apache/cassandra/db/marshal/CollectionType.java b/src/java/org/apache/cassandra/db/marshal/CollectionType.java index b9816a638d..fe672e402d 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 extends AbstractType 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; diff --git a/src/java/org/apache/cassandra/db/marshal/ColumnToCollectionType.java b/src/java/org/apache/cassandra/db/marshal/ColumnToCollectionType.java index a4f7857b4c..a28b87414c 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 // We are compatible if we have all the definitions previous have (but we can have more). for (Map.Entry 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;