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 <[email protected]> Authored: Mon Feb 24 10:04:48 2014 +0100 Committer: Sylvain Lebresne <[email protected]> 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;
