Merge branch 'cassandra-3.11' into cassandra-4.0

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)

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,

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<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);
     }
 
     /**
@@ -327,6 +330,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

@@ -175,7 +175,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
@@ -199,6 +199,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

@@ -105,12 +105,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

@@ -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<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