Merge branch 'cassandra-3.0' into cassandra-3.11

CHANGES.txt

@@ -1,6 +1,6 @@
 3.11.17
 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)
 

src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java

@@ -205,7 +205,7 @@ public class AlterTableStatement extends SchemaAlteringStatement
                         }
                         // 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))
                         {
                             String message =
                                 String.format("Cannot re-add previously dropped column '%s' of type %s, incompatible with previous type %s",

src/java/org/apache/cassandra/db/marshal/AbstractType.java

@@ -36,6 +36,7 @@ import org.apache.cassandra.cql3.CQL3Type;
 import org.apache.cassandra.cql3.ColumnSpecification;
 import org.apache.cassandra.cql3.Term;
 import org.apache.cassandra.db.TypeSizes;
+import org.apache.cassandra.db.rows.Cell;
 import org.apache.cassandra.exceptions.SyntaxException;
 import org.apache.cassandra.serializers.TypeSerializer;
 import org.apache.cassandra.serializers.MarshalException;
@@ -282,9 +283,11 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, 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);
     }
 
     /**
@@ -296,6 +299,18 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>, 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.
      *

src/java/org/apache/cassandra/db/marshal/CollectionType.java

@@ -174,7 +174,7 @@ public abstract class CollectionType<T> extends AbstractType<T>
             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
@@ -198,6 +198,15 @@ public abstract class CollectionType<T> extends AbstractType<T>
         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);
 

src/java/org/apache/cassandra/db/marshal/ReversedType.java

@@ -109,12 +109,6 @@ public class ReversedType<T> extends AbstractType<T>
         return this.baseType.isCompatibleWith(((ReversedType) otherType).baseType);
     }
 
-    @Override
-    public boolean isValueCompatibleWith(AbstractType<?> otherType)
-    {
-        return this.baseType.isValueCompatibleWith(otherType);
-    }
-
     @Override
     public CQL3Type asCQL3Type()
     {

test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java

@@ -414,6 +414,30 @@ public class AlterTest extends CQLTester
         execute("ALTER TABLE %s RENAME c1 TO \"\"");
     }
 
+    @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<int>, PRIMARY KEY (k, c))");
+        execute("alter table %s drop v");
+        execute("alter table %s add v list<varint>");
+    }
+
+    @Test(expected = InvalidRequestException.class)
+    public void testDropSimpleAddComplex() throws Throwable
+    {
+        createTable("create table %s (k int, c int, v set<text>, 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