diff --git a/CHANGES.txt b/CHANGES.txt index a96137164e..062811b158 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -5,7 +5,7 @@ Merged from 3.11: * Moved jflex from runtime to build dependencies (CASSANDRA-18664) Merged from 3.0: -3.0.30 + * Backport of CASSANDRA-16905 Further restrict schema column drop/recreate conversions (CASSANDRA-18760) * CQLSH emits a warning when the server version doesn't match (CASSANDRA-18745) * Fix missing speculative retries in tablestats (CASSANDRA-18767) * Fix Requires for Java for RPM package (CASSANDRA-18751) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java index f361b643eb..c8cf04561d 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java @@ -206,7 +206,7 @@ public abstract class AlterTableStatement extends AlterSchemaStatement { // After #8099, not safe to re-add columns of incompatible types - until *maybe* deser logic with dropped // columns is pushed deeper down the line. The latter would still be problematic in cases of schema races. - if (!type.isValueCompatibleWith(droppedColumn.type)) + if (!type.isSerializationCompatibleWith(droppedColumn.type)) { throw ire("Cannot re-add previously dropped column '%s' of type %s, incompatible with previous type %s", name, diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractType.java b/src/java/org/apache/cassandra/db/marshal/AbstractType.java index 19cf849dba..892a5330b5 100644 --- a/src/java/org/apache/cassandra/db/marshal/AbstractType.java +++ b/src/java/org/apache/cassandra/db/marshal/AbstractType.java @@ -32,6 +32,7 @@ import org.apache.cassandra.cql3.AssignmentTestable; import org.apache.cassandra.cql3.CQL3Type; import org.apache.cassandra.cql3.ColumnSpecification; import org.apache.cassandra.cql3.Term; +import org.apache.cassandra.db.rows.Cell; import org.apache.cassandra.exceptions.SyntaxException; import org.apache.cassandra.io.util.DataInputPlus; import org.apache.cassandra.io.util.DataOutputPlus; @@ -313,9 +314,11 @@ public abstract class AbstractType implements Comparator, Assignm * * Note that a type should be compatible with at least itself. */ - public boolean isValueCompatibleWith(AbstractType otherType) + public boolean isValueCompatibleWith(AbstractType previous) { - return isValueCompatibleWithInternal((otherType instanceof ReversedType) ? ((ReversedType) otherType).baseType : otherType); + AbstractType thisType = isReversed() ? ((ReversedType) this).baseType : this; + AbstractType thatType = previous.isReversed() ? ((ReversedType) previous).baseType : previous; + return thisType.isValueCompatibleWithInternal(thatType); } /** @@ -327,6 +330,18 @@ public abstract class AbstractType implements Comparator, Assignm return isCompatibleWith(otherType); } + /** + * Similar to {@link #isValueCompatibleWith(AbstractType)}, but takes into account {@link Cell} encoding. + * In particular, this method doesn't consider two types serialization compatible if one of them has fixed + * length (overrides {@link #valueLengthIfFixed()}, and the other one doesn't. + */ + public boolean isSerializationCompatibleWith(AbstractType previous) + { + return isValueCompatibleWith(previous) + && valueLengthIfFixed() == previous.valueLengthIfFixed() + && isMultiCell() == previous.isMultiCell(); + } + /** * An alternative comparison function used by CollectionsType in conjunction with CompositeType. * diff --git a/src/java/org/apache/cassandra/db/marshal/CollectionType.java b/src/java/org/apache/cassandra/db/marshal/CollectionType.java index 0d627a5cbf..c52cddc07f 100644 --- a/src/java/org/apache/cassandra/db/marshal/CollectionType.java +++ b/src/java/org/apache/cassandra/db/marshal/CollectionType.java @@ -175,7 +175,7 @@ public abstract class CollectionType extends AbstractType return false; // the value comparator is only used for Cell values, so sorting doesn't matter - return this.valueComparator().isValueCompatibleWith(tprev.valueComparator()); + return this.valueComparator().isSerializationCompatibleWith(tprev.valueComparator()); } @Override @@ -199,6 +199,15 @@ public abstract class CollectionType extends AbstractType return isValueCompatibleWithFrozen(tprev); } + @Override + public boolean isSerializationCompatibleWith(AbstractType previous) + { + if (!isValueCompatibleWith(previous)) + return false; + + return valueComparator().isSerializationCompatibleWith(((CollectionType)previous).valueComparator()); + } + /** A version of isCompatibleWith() to deal with non-multicell (frozen) collections */ protected abstract boolean isCompatibleWithFrozen(CollectionType previous); diff --git a/src/java/org/apache/cassandra/db/marshal/ReversedType.java b/src/java/org/apache/cassandra/db/marshal/ReversedType.java index 8a4b58dca2..ceea84a39f 100644 --- a/src/java/org/apache/cassandra/db/marshal/ReversedType.java +++ b/src/java/org/apache/cassandra/db/marshal/ReversedType.java @@ -105,12 +105,6 @@ public class ReversedType extends AbstractType return this.baseType.isCompatibleWith(((ReversedType) otherType).baseType); } - @Override - public boolean isValueCompatibleWith(AbstractType otherType) - { - return this.baseType.isValueCompatibleWith(otherType); - } - @Override public CQL3Type asCQL3Type() { diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java index 2741f941ca..0e9ce81a80 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java @@ -27,6 +27,7 @@ import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.dht.OrderPreservingPartitioner; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.exceptions.SyntaxException; import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.locator.TokenMetadata; @@ -431,6 +432,30 @@ public class AlterTest extends CQLTester alterTable("alter table %s add v1 int"); } + @Test(expected = InvalidRequestException.class) + public void testDropFixedAddVariable() throws Throwable + { + createTable("create table %s (k int, c int, v int, PRIMARY KEY (k, c))"); + execute("alter table %s drop v"); + execute("alter table %s add v varint"); + } + + @Test(expected = InvalidRequestException.class) + public void testDropFixedCollectionAddVariableCollection() throws Throwable + { + createTable("create table %s (k int, c int, v list, PRIMARY KEY (k, c))"); + execute("alter table %s drop v"); + execute("alter table %s add v list"); + } + + @Test(expected = InvalidRequestException.class) + public void testDropSimpleAddComplex() throws Throwable + { + createTable("create table %s (k int, c int, v set, PRIMARY KEY (k, c))"); + execute("alter table %s drop v"); + execute("alter table %s add v blob"); + } + @Test // tests CASSANDRA-9565 public void testDoubleWith() throws Throwable