diff --git a/CHANGES.txt b/CHANGES.txt index 6e3ae4f67f..8954614de2 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -3,6 +3,7 @@ * ReadCommandController should close fast to avoid deadlock when building secondary index (CASSANDRA-19564) * Redact security-sensitive information in system_views.settings (CASSANDRA-20856) Merged from 4.0: + * Restrict BytesType compatibility to scalar types only (CASSANDRA-20982) * Backport fix to nodetool gcstats output for direct memory (CASSANDRA-21037) * ArrayIndexOutOfBoundsException with repaired data tracking and counters (CASSANDRA-20871) * Fix cleanup of old incremental repair sessions in case of owned token range changes or a table deleting (CASSANDRA-20877) 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 357ef45615..531b8c5279 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java @@ -240,9 +240,10 @@ public abstract class AlterTableStatement extends AlterSchemaStatement // columns is pushed deeper down the line. The latter would still be problematic in cases of schema races. if (!type.isSerializationCompatibleWith(droppedColumn.type)) { - throw ire("Cannot re-add previously dropped column '%s' of type %s, incompatible with previous type %s", + throw ire("Cannot add a column '%s' of type %s, incompatible with previously dropped column '%s' of type %s", name, type.asCQL3Type(), + name, droppedColumn.type.asCQL3Type()); } diff --git a/src/java/org/apache/cassandra/db/marshal/BytesType.java b/src/java/org/apache/cassandra/db/marshal/BytesType.java index cabd007b09..ccd144123a 100644 --- a/src/java/org/apache/cassandra/db/marshal/BytesType.java +++ b/src/java/org/apache/cassandra/db/marshal/BytesType.java @@ -85,6 +85,14 @@ public class BytesType extends AbstractType return true; } + @Override + public boolean isSerializationCompatibleWith(AbstractType previous) + { + // BytesType should only be compatible with simple scalar types, not with collections or UDTs + // because converting a collection or UDT to raw bytes is nonsensical + return !previous.isCollection() && !previous.isUDT() && super.isSerializationCompatibleWith(previous); + } + public CQL3Type asCQL3Type() { return CQL3Type.Native.BLOB; 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 f0440bf167..c4569fb826 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java @@ -59,7 +59,7 @@ public class AlterTest extends CQLTester { createTable("CREATE TABLE %s (a int, b " + type + ", PRIMARY KEY (a));"); alterTable("ALTER TABLE %s DROP b;"); - assertInvalidMessage("Cannot re-add previously dropped column 'b' of type blob, incompatible with previous type " + type, + assertInvalidMessage("Cannot add a column 'b' of type blob, incompatible with previously dropped column 'b' of type " + type, "ALTER TABLE %s ADD b blob;"); } @@ -67,13 +67,13 @@ public class AlterTest extends CQLTester { createTable("CREATE TABLE %s (a int, b blob, PRIMARY KEY (a));"); alterTable("ALTER TABLE %s DROP b;"); - assertInvalidMessage("Cannot re-add previously dropped column 'b' of type " + type + ", incompatible with previous type blob", + assertInvalidMessage("Cannot add a column 'b' of type " + type + ", incompatible with previously dropped column 'b' of type blob", "ALTER TABLE %s ADD b " + type + ';'); } } @Test - public void testFrozenCollectionsAreCompatibleWithBlob() + public void testFrozenCollectionsAreNotCompatibleWithBlob() throws Throwable { String[] collectionTypes = new String[] {"frozen>", "frozen>", "frozen>"}; @@ -81,7 +81,8 @@ public class AlterTest extends CQLTester { createTable("CREATE TABLE %s (a int, b " + type + ", PRIMARY KEY (a));"); alterTable("ALTER TABLE %s DROP b;"); - alterTable("ALTER TABLE %s ADD b blob;"); + assertInvalidMessage("Cannot add a column 'b' of type blob, incompatible with previously dropped column 'b' of type " + type, + "ALTER TABLE %s ADD b blob;"); } } @@ -561,8 +562,8 @@ public class AlterTest extends CQLTester { createTable("create table %s (k int, c int, v " + typePair[0] + ", PRIMARY KEY (k, c))"); execute("alter table %s drop v"); - assertInvalidMessage("Cannot re-add previously dropped column 'v' of type " - + typePair[1] + ", incompatible with previous type " + typePair[0], + assertInvalidMessage("Cannot add a column 'v' of type " + + typePair[1] + ", incompatible with previously dropped column 'v' of type " + typePair[0], "alter table %s add v " + typePair[1]); } } diff --git a/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java b/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java index 789794044a..0e8a5fe924 100644 --- a/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java +++ b/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java @@ -19,9 +19,16 @@ */ package org.apache.cassandra.db.marshal; +import java.util.Arrays; + +import org.apache.cassandra.cql3.FieldIdentifier; import org.apache.cassandra.serializers.MarshalException; +import org.apache.cassandra.utils.ByteBufferUtil; import org.junit.Test; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + public class BytesTypeTest { private static final String INVALID_HEX = "33AG45F"; // Invalid (has a G) @@ -38,4 +45,68 @@ public class BytesTypeTest { BytesType.instance.fromString(VALID_HEX); } + + @Test + public void testValueCompatibilityWithScalarTypes() + { + // BytesType should be value-compatible with simple scalar types + assertTrue("BytesType should be value-compatible with AsciiType", + BytesType.instance.isValueCompatibleWith(AsciiType.instance)); + assertTrue("BytesType should be value-compatible with UTF8Type", + BytesType.instance.isValueCompatibleWith(UTF8Type.instance)); + assertTrue("BytesType should be value-compatible with Int32Type", + BytesType.instance.isValueCompatibleWith(Int32Type.instance)); + assertTrue("BytesType should be value-compatible with LongType", + BytesType.instance.isValueCompatibleWith(LongType.instance)); + assertTrue("BytesType should be value-compatible with UUIDType", + BytesType.instance.isValueCompatibleWith(UUIDType.instance)); + assertTrue("BytesType should be value-compatible with BooleanType", + BytesType.instance.isValueCompatibleWith(BooleanType.instance)); + assertTrue("BytesType should be value-compatible with itself", + BytesType.instance.isValueCompatibleWith(BytesType.instance)); + } + + @Test + public void testSerializationIncompatibilityWithCollections() + { + // BytesType should NOT be serialization-compatible with collections (even frozen ones) + // because converting a collection to raw bytes for schema changes is nonsensical + ListType frozenList = ListType.getInstance(Int32Type.instance, false); + SetType frozenSet = SetType.getInstance(Int32Type.instance, false); + MapType frozenMap = MapType.getInstance(Int32Type.instance, UTF8Type.instance, false); + + assertFalse("BytesType should NOT be serialization-compatible with frozen", + BytesType.instance.isSerializationCompatibleWith(frozenList)); + assertFalse("BytesType should NOT be serialization-compatible with frozen", + BytesType.instance.isSerializationCompatibleWith(frozenSet)); + assertFalse("BytesType should NOT be serialization-compatible with frozen", + BytesType.instance.isSerializationCompatibleWith(frozenMap)); + + // Also test with multi-cell collections + ListType multiCellList = ListType.getInstance(Int32Type.instance, true); + SetType multiCellSet = SetType.getInstance(Int32Type.instance, true); + MapType multiCellMap = MapType.getInstance(Int32Type.instance, UTF8Type.instance, true); + + assertFalse("BytesType should NOT be serialization-compatible with list", + BytesType.instance.isSerializationCompatibleWith(multiCellList)); + assertFalse("BytesType should NOT be serialization-compatible with set", + BytesType.instance.isSerializationCompatibleWith(multiCellSet)); + assertFalse("BytesType should NOT be serialization-compatible with map", + BytesType.instance.isSerializationCompatibleWith(multiCellMap)); + } + + @Test + public void testSerializationIncompatibilityWithUDT() + { + // BytesType should NOT be serialization-compatible with User Defined Types + // because converting a UDT to raw bytes for schema changes is nonsensical + UserType udt = new UserType("ks", + ByteBufferUtil.bytes("myType"), + Arrays.asList(FieldIdentifier.forQuoted("field1"), FieldIdentifier.forQuoted("field2")), + Arrays.asList(Int32Type.instance, UTF8Type.instance), + true); + + assertFalse("BytesType should NOT be serialization-compatible with UDT", + BytesType.instance.isSerializationCompatibleWith(udt)); + } }