dcapwell commented on code in PR #3169:
URL: https://github.com/apache/cassandra/pull/3169#discussion_r1518175643


##########
test/unit/org/apache/cassandra/db/marshal/AbstractTypeTest.java:
##########
@@ -740,4 +801,927 @@ public String toString()
                    '}';
         }
     }
+
+    /**
+     * This test case checks the output of is*CompatibleWith methods for 
primitive types - whether the output is
+     * consistent with the assumed relation defined in this test class 
(expected compatibility entered manually).
+     */
+    @Test
+    public void testAssumedPrimitiveTypesCompatibility()
+    {
+        testAssumedPrimitiveTypesCompatibility(typesCompatibility);
+        if (typesCompatibility.prev != null)
+            testAssumedPrimitiveTypesCompatibility(typesCompatibility.prev);
+    }
+
+    public void testAssumedPrimitiveTypesCompatibility(TypesCompatibility 
typesCompatibility)
+    {
+        SoftAssertions assertions = new SoftAssertions();
+
+        // assumed compatibility
+        typesCompatibility.compatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isCompatibleWith(r)).describedAs(isCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.valueCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isValueCompatibleWith(r)).describedAs(isValueCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isSerializationCompatibleWith(r)).describedAs(isSerializationCompatibleWithDesc(l,
 r)).isTrue());
+
+        // assumed incompatibility
+        inverse(typesCompatibility.compatibleWith).forEach(p -> 
assertions.assertThat(p.left.isCompatibleWith(p.right)).describedAs(isCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.valueCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isValueCompatibleWith(p.right)).describedAs(isValueCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.serializationCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isSerializationCompatibleWith(p.right)).describedAs(isSerializationCompatibleWithDesc(p.left,
 p.right)).isFalse());
+
+        // it is implied that isSerializationCompatibleWith is a subset of 
isValueCompatibleWith
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(typesCompatibility.valueCompatibleWith.containsEntry(l, 
r)).describedAs(isValueCompatibleWithDesc(l, r)).isTrue());
+        assertions.assertAll();
+    }
+
+    /**
+     * Assuming that {@link #testAssumedPrimitiveTypesCompatibility()} passes, 
this test case verifies whether the types
+     * said to be compatible hold the assumed properties. In particular:
+     * <li>L {@link AbstractType#isValueCompatibleWith(AbstractType)} R - if 
it is possible to {@code L.compose(R.decompose(v))} for any v valid for type R, 
and the converted value of type L still makes sense</li>
+     * <li>L {@link AbstractType#isSerializationCompatibleWith(AbstractType)} 
R - if it is possible to read a cell written using R's type serializer with L's 
type serializer</li>
+     * <li>L {@link AbstractType#isCompatibleWith(AbstractType)} R - L 
isSerializationCompatibleWith R and it is possible to compare decomposed values 
of R's type using L's type comparator</li>
+     */
+    @Test
+    public void testPrimitiveTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        typesCompatibility.valueCompatibleWith.forEach((left, right) -> 
assertions.check(() -> verifyTypesCompatibility(left, right, 
getTypeSupport(right).valueGen)));
+        assertions.assertAll();
+    }
+
+    private static <L, R> void verifyTypesCompatibility(AbstractType left, 
AbstractType right, Gen rightGen)

Review Comment:
   ```suggestion
       private static <L, R> void verifyTypesCompatibility(AbstractType<L> 
left, AbstractType<R> right, Gen<R> rightGen)
   ```
   
   why have `<L, R>` if you use raw types? 



##########
test/unit/org/apache/cassandra/db/marshal/AbstractTypeTest.java:
##########
@@ -740,4 +801,927 @@ public String toString()
                    '}';
         }
     }
+
+    /**
+     * This test case checks the output of is*CompatibleWith methods for 
primitive types - whether the output is
+     * consistent with the assumed relation defined in this test class 
(expected compatibility entered manually).
+     */
+    @Test
+    public void testAssumedPrimitiveTypesCompatibility()
+    {
+        testAssumedPrimitiveTypesCompatibility(typesCompatibility);
+        if (typesCompatibility.prev != null)
+            testAssumedPrimitiveTypesCompatibility(typesCompatibility.prev);
+    }
+
+    public void testAssumedPrimitiveTypesCompatibility(TypesCompatibility 
typesCompatibility)
+    {
+        SoftAssertions assertions = new SoftAssertions();
+
+        // assumed compatibility
+        typesCompatibility.compatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isCompatibleWith(r)).describedAs(isCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.valueCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isValueCompatibleWith(r)).describedAs(isValueCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isSerializationCompatibleWith(r)).describedAs(isSerializationCompatibleWithDesc(l,
 r)).isTrue());
+
+        // assumed incompatibility
+        inverse(typesCompatibility.compatibleWith).forEach(p -> 
assertions.assertThat(p.left.isCompatibleWith(p.right)).describedAs(isCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.valueCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isValueCompatibleWith(p.right)).describedAs(isValueCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.serializationCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isSerializationCompatibleWith(p.right)).describedAs(isSerializationCompatibleWithDesc(p.left,
 p.right)).isFalse());
+
+        // it is implied that isSerializationCompatibleWith is a subset of 
isValueCompatibleWith
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(typesCompatibility.valueCompatibleWith.containsEntry(l, 
r)).describedAs(isValueCompatibleWithDesc(l, r)).isTrue());
+        assertions.assertAll();
+    }
+
+    /**
+     * Assuming that {@link #testAssumedPrimitiveTypesCompatibility()} passes, 
this test case verifies whether the types
+     * said to be compatible hold the assumed properties. In particular:
+     * <li>L {@link AbstractType#isValueCompatibleWith(AbstractType)} R - if 
it is possible to {@code L.compose(R.decompose(v))} for any v valid for type R, 
and the converted value of type L still makes sense</li>
+     * <li>L {@link AbstractType#isSerializationCompatibleWith(AbstractType)} 
R - if it is possible to read a cell written using R's type serializer with L's 
type serializer</li>
+     * <li>L {@link AbstractType#isCompatibleWith(AbstractType)} R - L 
isSerializationCompatibleWith R and it is possible to compare decomposed values 
of R's type using L's type comparator</li>
+     */
+    @Test
+    public void testPrimitiveTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        typesCompatibility.valueCompatibleWith.forEach((left, right) -> 
assertions.check(() -> verifyTypesCompatibility(left, right, 
getTypeSupport(right).valueGen)));
+        assertions.assertAll();
+    }
+
+    private static <L, R> void verifyTypesCompatibility(AbstractType left, 
AbstractType right, Gen rightGen)
+    {
+        if (left.equals(right))
+            return;
+
+        if (!left.isValueCompatibleWith(right))
+            return;
+
+        ColumnMetadata rightColumn = new ColumnMetadata("k", "t", 
ColumnIdentifier.getInterned("c", false), right, ColumnMetadata.NO_POSITION, 
ColumnMetadata.Kind.REGULAR, null);
+        ColumnMetadata leftColumn = new ColumnMetadata("k", "t", 
ColumnIdentifier.getInterned("c", false), left, ColumnMetadata.NO_POSITION, 
ColumnMetadata.Kind.REGULAR, null);
+
+        TableMetadata leftTable = TableMetadata.builder("k", 
"t").addPartitionKeyColumn("pk", 
EmptyType.instance).addColumn(leftColumn).build();
+        TableMetadata rightTable = TableMetadata.builder("k", 
"t").addPartitionKeyColumn("pk", 
EmptyType.instance).addColumn(rightColumn).build();
+
+        SerializationHeader leftHeader = new SerializationHeader(false, 
leftTable, leftTable.regularAndStaticColumns(), EncodingStats.NO_STATS);
+        SerializationHeader rightHeader = new SerializationHeader(false, 
rightTable, rightTable.regularAndStaticColumns(), EncodingStats.NO_STATS);
+
+        DeserializationHelper leftHelper = new 
DeserializationHelper(leftTable, MessagingService.current_version, 
DeserializationHelper.Flag.LOCAL, ColumnFilter.all(leftTable));
+        SerializationHelper rightHelper = new SerializationHelper(rightHeader);
+
+        qt().withExamples(10).forAll(rightGen).checkAssert(v -> {
+            
Assertions.assertThatNoException().describedAs(typeRelDesc(".decompose", left, 
right)).isThrownBy(() -> {
+
+                // value compatibility means that we can use left's type 
serializer to decompose a value of right's type
+                ByteBuffer rightDecomposed = right.decompose((R) v);
+                L leftComposed = (L) left.compose(rightDecomposed);
+                ByteBuffer leftDecomposed = left.decompose(leftComposed);
+                
assertThat(leftDecomposed.hasRemaining()).isEqualTo(rightDecomposed.hasRemaining());
+            });
+
+            
Assertions.assertThatNoException().describedAs(typeRelDesc(".deserialize", 
left, right)).isThrownBy(() -> {
+                // serialization compatibility means that we can read a cell 
written using right's type serializer with left's type serializer;
+                // this additinoally imposes the requirement for storing the 
buffer lenght in the serialized form if the value is of variable length
+                // as well as, either both types serialize into a single or 
multiple cells
+                if (left.isSerializationCompatibleWith(right))
+                {
+                    if (!left.isMultiCell() && !right.isMultiCell())
+                        verifySerializationCompatibilityForSimpleCells(left, 
right, v, rightTable, rightColumn, rightHelper, leftHeader, leftHelper, 
leftColumn);
+                    else
+                        verifySerializationCompatibilityForComplexCells(left, 
right, v, rightTable, rightColumn, rightHelper, leftHeader, leftHelper, 
leftColumn);
+                }
+            });
+        });
+
+        if (!left.isCompatibleWith(right) || right.comparisonType == 
AbstractType.ComparisonType.NOT_COMPARABLE || left.comparisonType == 
AbstractType.ComparisonType.NOT_COMPARABLE)
+            return;
+
+        // types compatibility means that we can compare values of right's 
type using left's type comparator additionally
+        // to types being serialization compatible
+        qt().withExamples(10).forAll(rightGen, rightGen).checkAssert((r1, r2) 
-> {
+            
Assertions.assertThatNoException().describedAs(typeRelDesc(".compare", left, 
right)).isThrownBy(() -> {
+                if (!left.isMultiCell() && !right.isMultiCell())
+                {
+                    // make sure that frozen<left> isCompatibleWith 
frozen<right> ==> left isCompatibleWith right
+                    
assertThat(left.unfreeze().isCompatibleWith(right.unfreeze())).isTrue();
+                }

Review Comment:
   can we pull this out of the property check?  the property check runs 10 
times, where as this condition is unchanged, so can run *before*



##########
test/unit/org/apache/cassandra/db/marshal/AbstractTypeTest.java:
##########
@@ -740,4 +801,927 @@ public String toString()
                    '}';
         }
     }
+
+    /**
+     * This test case checks the output of is*CompatibleWith methods for 
primitive types - whether the output is
+     * consistent with the assumed relation defined in this test class 
(expected compatibility entered manually).
+     */
+    @Test
+    public void testAssumedPrimitiveTypesCompatibility()
+    {
+        testAssumedPrimitiveTypesCompatibility(typesCompatibility);
+        if (typesCompatibility.prev != null)
+            testAssumedPrimitiveTypesCompatibility(typesCompatibility.prev);
+    }
+
+    public void testAssumedPrimitiveTypesCompatibility(TypesCompatibility 
typesCompatibility)
+    {
+        SoftAssertions assertions = new SoftAssertions();
+
+        // assumed compatibility
+        typesCompatibility.compatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isCompatibleWith(r)).describedAs(isCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.valueCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isValueCompatibleWith(r)).describedAs(isValueCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isSerializationCompatibleWith(r)).describedAs(isSerializationCompatibleWithDesc(l,
 r)).isTrue());
+
+        // assumed incompatibility
+        inverse(typesCompatibility.compatibleWith).forEach(p -> 
assertions.assertThat(p.left.isCompatibleWith(p.right)).describedAs(isCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.valueCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isValueCompatibleWith(p.right)).describedAs(isValueCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.serializationCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isSerializationCompatibleWith(p.right)).describedAs(isSerializationCompatibleWithDesc(p.left,
 p.right)).isFalse());
+
+        // it is implied that isSerializationCompatibleWith is a subset of 
isValueCompatibleWith
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(typesCompatibility.valueCompatibleWith.containsEntry(l, 
r)).describedAs(isValueCompatibleWithDesc(l, r)).isTrue());
+        assertions.assertAll();
+    }
+
+    /**
+     * Assuming that {@link #testAssumedPrimitiveTypesCompatibility()} passes, 
this test case verifies whether the types
+     * said to be compatible hold the assumed properties. In particular:
+     * <li>L {@link AbstractType#isValueCompatibleWith(AbstractType)} R - if 
it is possible to {@code L.compose(R.decompose(v))} for any v valid for type R, 
and the converted value of type L still makes sense</li>
+     * <li>L {@link AbstractType#isSerializationCompatibleWith(AbstractType)} 
R - if it is possible to read a cell written using R's type serializer with L's 
type serializer</li>
+     * <li>L {@link AbstractType#isCompatibleWith(AbstractType)} R - L 
isSerializationCompatibleWith R and it is possible to compare decomposed values 
of R's type using L's type comparator</li>
+     */
+    @Test
+    public void testPrimitiveTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        typesCompatibility.valueCompatibleWith.forEach((left, right) -> 
assertions.check(() -> verifyTypesCompatibility(left, right, 
getTypeSupport(right).valueGen)));
+        assertions.assertAll();
+    }
+
+    private static <L, R> void verifyTypesCompatibility(AbstractType left, 
AbstractType right, Gen rightGen)
+    {
+        if (left.equals(right))
+            return;
+
+        if (!left.isValueCompatibleWith(right))
+            return;
+
+        ColumnMetadata rightColumn = new ColumnMetadata("k", "t", 
ColumnIdentifier.getInterned("c", false), right, ColumnMetadata.NO_POSITION, 
ColumnMetadata.Kind.REGULAR, null);
+        ColumnMetadata leftColumn = new ColumnMetadata("k", "t", 
ColumnIdentifier.getInterned("c", false), left, ColumnMetadata.NO_POSITION, 
ColumnMetadata.Kind.REGULAR, null);
+
+        TableMetadata leftTable = TableMetadata.builder("k", 
"t").addPartitionKeyColumn("pk", 
EmptyType.instance).addColumn(leftColumn).build();
+        TableMetadata rightTable = TableMetadata.builder("k", 
"t").addPartitionKeyColumn("pk", 
EmptyType.instance).addColumn(rightColumn).build();
+
+        SerializationHeader leftHeader = new SerializationHeader(false, 
leftTable, leftTable.regularAndStaticColumns(), EncodingStats.NO_STATS);
+        SerializationHeader rightHeader = new SerializationHeader(false, 
rightTable, rightTable.regularAndStaticColumns(), EncodingStats.NO_STATS);
+
+        DeserializationHelper leftHelper = new 
DeserializationHelper(leftTable, MessagingService.current_version, 
DeserializationHelper.Flag.LOCAL, ColumnFilter.all(leftTable));
+        SerializationHelper rightHelper = new SerializationHelper(rightHeader);
+
+        qt().withExamples(10).forAll(rightGen).checkAssert(v -> {
+            
Assertions.assertThatNoException().describedAs(typeRelDesc(".decompose", left, 
right)).isThrownBy(() -> {
+
+                // value compatibility means that we can use left's type 
serializer to decompose a value of right's type
+                ByteBuffer rightDecomposed = right.decompose((R) v);
+                L leftComposed = (L) left.compose(rightDecomposed);

Review Comment:
   if you fix the method signature you don't need to cast



##########
test/unit/org/apache/cassandra/db/marshal/AbstractTypeTest.java:
##########
@@ -740,4 +801,927 @@ public String toString()
                    '}';
         }
     }
+
+    /**
+     * This test case checks the output of is*CompatibleWith methods for 
primitive types - whether the output is
+     * consistent with the assumed relation defined in this test class 
(expected compatibility entered manually).
+     */
+    @Test
+    public void testAssumedPrimitiveTypesCompatibility()
+    {
+        testAssumedPrimitiveTypesCompatibility(typesCompatibility);
+        if (typesCompatibility.prev != null)
+            testAssumedPrimitiveTypesCompatibility(typesCompatibility.prev);
+    }
+
+    public void testAssumedPrimitiveTypesCompatibility(TypesCompatibility 
typesCompatibility)
+    {
+        SoftAssertions assertions = new SoftAssertions();
+
+        // assumed compatibility
+        typesCompatibility.compatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isCompatibleWith(r)).describedAs(isCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.valueCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isValueCompatibleWith(r)).describedAs(isValueCompatibleWithDesc(l,
 r)).isTrue());
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(l.isSerializationCompatibleWith(r)).describedAs(isSerializationCompatibleWithDesc(l,
 r)).isTrue());
+
+        // assumed incompatibility
+        inverse(typesCompatibility.compatibleWith).forEach(p -> 
assertions.assertThat(p.left.isCompatibleWith(p.right)).describedAs(isCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.valueCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isValueCompatibleWith(p.right)).describedAs(isValueCompatibleWithDesc(p.left,
 p.right)).isFalse());
+        inverse(typesCompatibility.serializationCompatibleWith).forEach(p -> 
assertions.assertThat(p.left.isSerializationCompatibleWith(p.right)).describedAs(isSerializationCompatibleWithDesc(p.left,
 p.right)).isFalse());
+
+        // it is implied that isSerializationCompatibleWith is a subset of 
isValueCompatibleWith
+        typesCompatibility.serializationCompatibleWith.forEach((l, r) -> 
assertions.assertThat(typesCompatibility.valueCompatibleWith.containsEntry(l, 
r)).describedAs(isValueCompatibleWithDesc(l, r)).isTrue());
+        assertions.assertAll();
+    }
+
+    /**
+     * Assuming that {@link #testAssumedPrimitiveTypesCompatibility()} passes, 
this test case verifies whether the types
+     * said to be compatible hold the assumed properties. In particular:
+     * <li>L {@link AbstractType#isValueCompatibleWith(AbstractType)} R - if 
it is possible to {@code L.compose(R.decompose(v))} for any v valid for type R, 
and the converted value of type L still makes sense</li>
+     * <li>L {@link AbstractType#isSerializationCompatibleWith(AbstractType)} 
R - if it is possible to read a cell written using R's type serializer with L's 
type serializer</li>
+     * <li>L {@link AbstractType#isCompatibleWith(AbstractType)} R - L 
isSerializationCompatibleWith R and it is possible to compare decomposed values 
of R's type using L's type comparator</li>
+     */
+    @Test
+    public void testPrimitiveTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        typesCompatibility.valueCompatibleWith.forEach((left, right) -> 
assertions.check(() -> verifyTypesCompatibility(left, right, 
getTypeSupport(right).valueGen)));
+        assertions.assertAll();
+    }
+
+    private static <L, R> void verifyTypesCompatibility(AbstractType left, 
AbstractType right, Gen rightGen)
+    {
+        if (left.equals(right))
+            return;
+
+        if (!left.isValueCompatibleWith(right))
+            return;
+
+        ColumnMetadata rightColumn = new ColumnMetadata("k", "t", 
ColumnIdentifier.getInterned("c", false), right, ColumnMetadata.NO_POSITION, 
ColumnMetadata.Kind.REGULAR, null);
+        ColumnMetadata leftColumn = new ColumnMetadata("k", "t", 
ColumnIdentifier.getInterned("c", false), left, ColumnMetadata.NO_POSITION, 
ColumnMetadata.Kind.REGULAR, null);
+
+        TableMetadata leftTable = TableMetadata.builder("k", 
"t").addPartitionKeyColumn("pk", 
EmptyType.instance).addColumn(leftColumn).build();
+        TableMetadata rightTable = TableMetadata.builder("k", 
"t").addPartitionKeyColumn("pk", 
EmptyType.instance).addColumn(rightColumn).build();
+
+        SerializationHeader leftHeader = new SerializationHeader(false, 
leftTable, leftTable.regularAndStaticColumns(), EncodingStats.NO_STATS);
+        SerializationHeader rightHeader = new SerializationHeader(false, 
rightTable, rightTable.regularAndStaticColumns(), EncodingStats.NO_STATS);
+
+        DeserializationHelper leftHelper = new 
DeserializationHelper(leftTable, MessagingService.current_version, 
DeserializationHelper.Flag.LOCAL, ColumnFilter.all(leftTable));
+        SerializationHelper rightHelper = new SerializationHelper(rightHeader);
+
+        qt().withExamples(10).forAll(rightGen).checkAssert(v -> {
+            
Assertions.assertThatNoException().describedAs(typeRelDesc(".decompose", left, 
right)).isThrownBy(() -> {
+
+                // value compatibility means that we can use left's type 
serializer to decompose a value of right's type
+                ByteBuffer rightDecomposed = right.decompose((R) v);
+                L leftComposed = (L) left.compose(rightDecomposed);
+                ByteBuffer leftDecomposed = left.decompose(leftComposed);
+                
assertThat(leftDecomposed.hasRemaining()).isEqualTo(rightDecomposed.hasRemaining());
+            });
+
+            
Assertions.assertThatNoException().describedAs(typeRelDesc(".deserialize", 
left, right)).isThrownBy(() -> {
+                // serialization compatibility means that we can read a cell 
written using right's type serializer with left's type serializer;
+                // this additinoally imposes the requirement for storing the 
buffer lenght in the serialized form if the value is of variable length
+                // as well as, either both types serialize into a single or 
multiple cells
+                if (left.isSerializationCompatibleWith(right))
+                {
+                    if (!left.isMultiCell() && !right.isMultiCell())
+                        verifySerializationCompatibilityForSimpleCells(left, 
right, v, rightTable, rightColumn, rightHelper, leftHeader, leftHelper, 
leftColumn);
+                    else
+                        verifySerializationCompatibilityForComplexCells(left, 
right, v, rightTable, rightColumn, rightHelper, leftHeader, leftHelper, 
leftColumn);
+                }
+            });
+        });
+
+        if (!left.isCompatibleWith(right) || right.comparisonType == 
AbstractType.ComparisonType.NOT_COMPARABLE || left.comparisonType == 
AbstractType.ComparisonType.NOT_COMPARABLE)
+            return;
+
+        // types compatibility means that we can compare values of right's 
type using left's type comparator additionally
+        // to types being serialization compatible
+        qt().withExamples(10).forAll(rightGen, rightGen).checkAssert((r1, r2) 
-> {
+            
Assertions.assertThatNoException().describedAs(typeRelDesc(".compare", left, 
right)).isThrownBy(() -> {
+                if (!left.isMultiCell() && !right.isMultiCell())
+                {
+                    // make sure that frozen<left> isCompatibleWith 
frozen<right> ==> left isCompatibleWith right
+                    
assertThat(left.unfreeze().isCompatibleWith(right.unfreeze())).isTrue();
+                }
+
+                ByteBuffer rBuf1 = right.decompose(r1);
+                ByteBuffer rBuf2 = right.decompose(r2);
+                ByteBuffer lBuf1 = left.decompose(left.compose(rBuf1));
+                ByteBuffer lBuf2 = left.decompose(left.compose(rBuf2));
+
+                int c = right.compare(rBuf1, rBuf2);
+                // first just check that the comparison is antisymmetric
+                assertThat(copySign(1, right.compare(rBuf2, 
rBuf1))).isEqualTo(copySign(1, -c));
+
+                // then, check if we can compare buffers using left's 
comparator
+                assertThat(copySign(1, left.compare(lBuf1, 
lBuf2))).isEqualTo(copySign(1, c));
+                assertThat(copySign(1, left.compare(lBuf1, 
rBuf2))).isEqualTo(copySign(1, c));
+                assertThat(copySign(1, left.compare(rBuf1, 
lBuf2))).isEqualTo(copySign(1, c));
+                assertThat(copySign(1, left.compare(rBuf1, 
rBuf2))).isEqualTo(copySign(1, c));
+
+                assertThat(copySign(1, left.compare(lBuf2, 
lBuf1))).isEqualTo(copySign(1, -c));
+                assertThat(copySign(1, left.compare(lBuf2, 
rBuf1))).isEqualTo(copySign(1, -c));
+                assertThat(copySign(1, left.compare(rBuf2, 
lBuf1))).isEqualTo(copySign(1, -c));
+                assertThat(copySign(1, left.compare(rBuf2, 
rBuf1))).isEqualTo(copySign(1, -c));
+            });
+        });
+    }
+
+    private static void 
verifySerializationCompatibilityForSimpleCells(AbstractType left, AbstractType 
right, Object v,
+                                                                       
TableMetadata rightTable, ColumnMetadata rightColumn, SerializationHelper 
rightHelper,
+                                                                       
SerializationHeader leftHeader, DeserializationHelper leftHelper, 
ColumnMetadata leftColumn) throws IOException
+    {
+        Row rightRow = 
Rows.simpleBuilder(rightTable).noPrimaryKeyLivenessInfo().add(rightColumn.name.toString(),
 v).build();
+        try (DataOutputBuffer out = new DataOutputBuffer())
+        {
+            UnfilteredSerializer.serializer.serialize(rightRow, rightHelper, 
out, MessagingService.current_version);
+            try (DataInputBuffer in = new DataInputBuffer(out.getData()))
+            {
+                Row.Builder builder = BTreeRow.sortedBuilder();
+                
builder.addPrimaryKeyLivenessInfo(rightRow.primaryKeyLivenessInfo());
+                Row leftRow = (Row) 
UnfilteredSerializer.serializer.deserialize(in, leftHeader, leftHelper, 
builder);
+                Cell leftData = (Cell) leftRow.getColumnData(leftColumn);
+                Cell rightData = (Cell) rightRow.getColumnData(rightColumn);
+                
assertThat(leftData.buffer()).describedAs(typeRelDesc(".deserialize", left, 
right)).isEqualTo(rightData.buffer());
+            }
+        }
+    }
+
+    private static void 
verifySerializationCompatibilityForComplexCells(AbstractType left, AbstractType 
right, Object v,
+                                                                        
TableMetadata rightTable, ColumnMetadata rightColumn, SerializationHelper 
rightHelper,
+                                                                        
SerializationHeader leftHeader, DeserializationHelper leftHelper, 
ColumnMetadata leftColumn) throws IOException
+    {
+        Row rightRow = 
Rows.simpleBuilder(rightTable).noPrimaryKeyLivenessInfo().add(rightColumn.name.toString(),
 v).build();
+        try (DataOutputBuffer out = new DataOutputBuffer())
+        {
+            UnfilteredSerializer.serializer.serialize(rightRow, rightHelper, 
out, MessagingService.current_version);
+            try (DataInputBuffer in = new DataInputBuffer(out.getData()))
+            {
+                Row.Builder builder = BTreeRow.sortedBuilder();
+                
builder.addPrimaryKeyLivenessInfo(rightRow.primaryKeyLivenessInfo());
+                Row leftRow = (Row) 
UnfilteredSerializer.serializer.deserialize(in, leftHeader, leftHelper, 
builder);
+                ComplexColumnData leftData = 
leftRow.getComplexColumnData(leftColumn);
+                ComplexColumnData rightData = 
rightRow.getComplexColumnData(rightColumn);
+                
assertThat(leftData.cellsCount()).isEqualTo(rightData.cellsCount());
+                for (int i = 0; i < leftData.cellsCount(); i++)
+                {
+                    Cell leftCell = leftData.getCellByIndex(i);
+                    Cell rightCell = rightData.getCellByIndex(i);
+                    
assertThat(leftCell.buffer()).describedAs(bytesToHex(leftCell.buffer())).isEqualTo(rightCell.buffer()).describedAs(bytesToHex(rightCell.buffer()));
+                    
assertThat(leftCell.path().size()).isEqualTo(rightCell.path().size());
+                    for (int j = 0; j < leftCell.path().size(); j++)
+                        
assertThat(leftCell.path().get(j)).describedAs(bytesToHex(leftCell.path().get(j))).isEqualTo(rightCell.path().get(j)).describedAs(bytesToHex(rightCell.path().get(j)));
+                }
+            }
+        }
+    }
+
+    @Test
+    public void testMultiCellSupport()
+    {
+        if (typesCompatibility.prev != null)
+        {
+            // for compatibility, we ensure that this version can read values 
of all the types the previous version can write
+            
assertThat(typesCompatibility.multiCellSupportingTypesForReading).containsAll(typesCompatibility.prev.multiCellSupportingTypes);
+        }
+
+        // all primitive types should be freezing agnostic
+        primitiveTypes().forEach(type -> {
+            assertThat(type.freeze()).isSameAs(type);
+            assertThat(type.unfreeze()).isSameAs(type);
+        });
+
+        qt().forAll(genBuilder().withoutTypeKinds(PRIMITIVE)
+                                .withMaxDepth(1)
+                                .multiCellIfRelevant()
+                                .withUserTypeKeyspace("ks")
+                                .withUDTNames(arbitrary().constant("ut"))
+                                .build())
+            .checkAssert(type -> {
+                if 
(typesCompatibility.multiCellSupportingTypesForReading.contains(type.getClass()))
+                {
+                    assertThat(type.isMultiCell()).isTrue();
+                    assertThat(type.freeze()).isNotEqualTo(type);
+                    
assertThat(type.freeze().unfreeze()).isNotEqualTo(type.freeze());
+                }
+                else
+                {
+                    assertThat(type.isMultiCell()).isFalse();
+                    assertThat(type.freeze()).isSameAs(type);
+                    assertThat(type.unfreeze()).isSameAs(type);
+                }
+            });
+    }
+
+    @Test
+    public void testMapTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        SoftAssertions upgradeAssertions = new SoftAssertions();
+
+        primitiveTypePairs(t -> t.getClass() != 
EmptyType.class).forEach(keyPair -> { // key cannot be empty
+            primitiveTypePairs().forEach(valuePair -> {
+                MapType<?, ?> leftMap = MapType.getInstance(keyPair.left, 
valuePair.left, true);
+                MapType<?, ?> rightMap = MapType.getInstance(keyPair.right, 
valuePair.right, true);
+
+                // note that we can assume is*Compatible methods are correct 
for primitive types because they are verified in other tests
+                typesCompatibility.checkMapTypeCompatibility(leftMap, 
rightMap, assertions);
+                if (typesCompatibility.prev != null)
+                    typesCompatibility.prev.checkMapTypeCompatibility(leftMap, 
rightMap, upgradeAssertions);
+            });
+        });
+
+        assertions.assertAll();
+        upgradeAssertions.assertAll();
+    }
+
+    @Test
+    public void testSetTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        SoftAssertions upgradeAssertions = new SoftAssertions();
+
+        primitiveTypePairs().forEach(keyPair -> {
+            SetType<?> leftSet = SetType.getInstance(keyPair.left, true);
+            SetType<?> rightSet = SetType.getInstance(keyPair.right, true);
+            // note that we can assume is*Compatible methods are correct for 
primitive types because they are verified in other tests
+            typesCompatibility.checkSetTypeCompatibility(leftSet, rightSet, 
assertions);
+            if (typesCompatibility.prev != null)
+                typesCompatibility.prev.checkSetTypeCompatibility(leftSet, 
rightSet, upgradeAssertions);
+        });
+
+        assertions.assertAll();
+        upgradeAssertions.assertAll();
+    }
+
+    @Test
+    public void testListTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        SoftAssertions upgradeAssertions = new SoftAssertions();
+
+        primitiveTypePairs().forEach(valuePair -> {
+            ListType<?> leftList = ListType.getInstance(valuePair.left, true);
+            ListType<?> rightList = ListType.getInstance(valuePair.right, 
true);
+            // note that we can assume is*Compatible methods are correct for 
primitive types because they are verified in other tests
+            typesCompatibility.checkListTypeCompatibility(leftList, rightList, 
assertions);
+            if (typesCompatibility.prev != null)
+                typesCompatibility.prev.checkListTypeCompatibility(leftList, 
rightList, upgradeAssertions);
+        });
+
+        assertions.assertAll();
+        upgradeAssertions.assertAll();
+    }
+
+    @Test
+    public void testUserTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        SoftAssertions upgradeAssertions = new SoftAssertions();
+
+        String ks = "ks";
+        ByteBuffer t = ByteBufferUtil.bytes("t");
+        List<FieldIdentifier> names = Stream.of("a", 
"b").map(FieldIdentifier::forUnquoted).collect(Collectors.toUnmodifiableList());
+
+        primitiveTypePairs().forEach(elem1Pair -> {
+            primitiveTypePairs().forEach(elem2Pair -> {
+                UserType leftType = new UserType(ks, t, names, 
List.of(elem1Pair.left, elem2Pair.left), true);
+                UserType rightType = new UserType(ks, t, names, 
List.of(elem1Pair.right, elem2Pair.right), true);
+
+                // note that we can assume is*Compatible methods are correct 
for primitive types because they are verified in other tests
+                typesCompatibility.checkUserTypeCompatibility(leftType, 
rightType, assertions);
+                if (typesCompatibility.prev != null)
+                    
typesCompatibility.prev.checkUserTypeCompatibility(leftType, rightType, 
upgradeAssertions);
+            });
+        });
+
+        assertions.assertAll();
+        upgradeAssertions.assertAll();
+    }
+
+    @Test
+    public void testCompositeTypesCompatibility()
+    {
+        SoftAssertions assertions = new SoftAssertions();
+        SoftAssertions upgradeAssertions = new SoftAssertions();
+
+        primitiveTypePairs().forEach(elem1Pair -> {
+            primitiveTypePairs().forEach(elem2Pair -> {
+                DynamicCompositeType leftType = 
DynamicCompositeType.getInstance(Map.of((byte) 0, elem1Pair.left, (byte) 1, 
elem2Pair.left));
+                DynamicCompositeType rightType = 
DynamicCompositeType.getInstance(Map.of((byte) 0, elem1Pair.right, (byte) 1, 
elem2Pair.right));
+                // note that we can assume is*Compatible methods are correct 
for primitive types because they are verified in other tests
+                typesCompatibility.checkCompositeTypeCompatibility(leftType, 
rightType, assertions);
+                if (typesCompatibility.prev != null)
+                    
typesCompatibility.prev.checkCompositeTypeCompatibility(leftType, rightType, 
upgradeAssertions);
+            });
+        });
+
+        assertions.assertAll();
+        upgradeAssertions.assertAll();
+    }
+
+    private static Description typeRelDesc(String rel, AbstractType<?> left, 
AbstractType<?> right)
+    {
+        return typeRelDesc(rel, left, right, null);
+    }
+
+    private static Description typeRelDesc(String rel, AbstractType<?> left, 
AbstractType<?> right, String extraInfo)
+    {
+        return new Description()
+        {
+            @Override
+            public String value()
+            {
+                if (extraInfo != null)
+                {
+                    return TYPE_PREFIX_PATTERN.matcher(String.format("%s %s 
%s, %s", left, rel, right, extraInfo)).replaceAll("");
+                }
+                else if (!left.equals(right))
+                {
+                    String extraInfo = Streams.zip(left.subTypes().stream(), 
right.subTypes().stream(), (l, r) -> {
+                        if (l.equals(r))
+                            return "";
+
+                        StringBuilder out = new StringBuilder();
+                        if (l.isCompatibleWith(r))
+                            out.append(" cmp");
+                        if (l.isValueCompatibleWith(r))
+                            out.append(" val");
+                        if (l.isSerializationCompatibleWith(r))
+                            out.append(" ser");
+                        if (out.length() > 0)
+                            return String.format("%s is%s compatible with %s", 
l, out, r);
+                        else
+                            return String.format("%s is not compatible with 
%s", l, r);
+                    }).collect(Collectors.joining("; ", "{", "}"));
+                    return TYPE_PREFIX_PATTERN.matcher(String.format("%s %s 
%s, %s", left, rel, right, extraInfo)).replaceAll("");
+                }
+                else
+                {
+                    return TYPE_PREFIX_PATTERN.matcher(String.format("%s %s 
%s", left, rel, right)).replaceAll("");
+                }
+            }
+        };
+    }
+
+    private static Description isCompatibleWithDesc(AbstractType<?> left, 
AbstractType<?> right)
+    {
+        return typeRelDesc("isCompatibleWith", left, right);
+    }
+
+    private static Description isValueCompatibleWithDesc(AbstractType<?> left, 
AbstractType<?> right)
+    {
+        return typeRelDesc("isValueCompatibleWith", left, right);
+    }
+
+    private static Description 
isSerializationCompatibleWithDesc(AbstractType<?> left, AbstractType<?> right)
+    {
+        return typeRelDesc("isSerializationCompatibleWith", left, right);
+    }
+
+    /**
+     * The instances of this class provides types compatibility checks valid 
for a certain version of Cassandra.
+     * This way we can verify whether the current implementation satisfy 
assumed compatibility rules, as well as
+     * upgrade compatibility (that is, whether the new implementation ensures 
the compatibility rules from the previous
+     * verion of Cassandra are still satisfied).
+     */
+    public abstract static class TypesCompatibility
+    {
+        public final Set<Class<? extends AbstractType>> 
multiCellSupportingTypes = new HashSet<>();
+        public final Set<Class<? extends AbstractType>> 
multiCellSupportingTypesForReading = new HashSet<>();
+
+
+        public final Multimap<AbstractType<?>, AbstractType<?>> 
valueCompatibleWith = HashMultimap.create();
+        public final Multimap<AbstractType<?>, AbstractType<?>> 
serializationCompatibleWith = HashMultimap.create();
+        public final Multimap<AbstractType<?>, AbstractType<?>> compatibleWith 
= HashMultimap.create();
+
+        public final TypesCompatibility prev;
+
+        protected static final Set<AbstractType<?>> PRIMITIVE_TYPES = 
PRIMITIVE_TYPE_DATA_GENS.keySet();
+
+        public TypesCompatibility(TypesCompatibility prev)
+        {
+            this.prev = prev;
+        }
+
+        static <T> Multimap<T, T> buildTransitiveClosure(Multimap<T, T> 
relation, Multimap<T, T> transitiveClosure, Collection<T> domain)
+        {
+            transitiveClosure.clear();
+            Deque<T> path = new LinkedList<>();
+            domain.forEach(lt -> {
+                transitiveClosure.put(lt, lt);
+                path.addLast(lt);
+                relation.get(lt).forEach(rt -> 
addToTransitiveClosure(relation, transitiveClosure, path, rt));
+                path.removeLast();
+                assert path.isEmpty();
+            });
+            return transitiveClosure;
+        }
+
+        private static <T> void addToTransitiveClosure(Multimap<T, T> 
relation, Multimap<T, T> transitiveClosure, Deque<T> path, T element)
+        {
+            assert !path.isEmpty();
+
+            if (path.contains(element))
+                return; // cycle
+
+            path.forEach(lt -> transitiveClosure.put(lt, element));
+            path.addLast(element);
+            relation.get(element).forEach(t -> 
addToTransitiveClosure(relation, transitiveClosure, path, t));
+            path.removeLast();
+        }
+
+        static Stream<Pair<AbstractType<?>, AbstractType<?>>> 
inverse(Multimap<AbstractType<?>, AbstractType<?>> relation, 
Collection<AbstractType<?>> domain)
+        {
+            return domain.stream()
+                         .flatMap(l -> domain.stream()
+                                             .filter(r -> 
!relation.containsEntry(l, r))
+                                             .map(r -> Pair.create(l, r)));
+        }
+
+        static Stream<Pair<AbstractType<?>, AbstractType<?>>> 
inverse(Multimap<AbstractType<?>, AbstractType<?>> relation)
+        {
+            return inverse(relation, PRIMITIVE_TYPES);
+        }
+
+        <L extends AbstractType, R extends AbstractType> void 
checkMultiCellTypeCompatibility(Collection<L> leftVariants, Collection<R> 
rightVariants, BiPredicate<L, R> checkPredicate, BiPredicate<L, R> 
expectPredicate, BiFunction<L, R, Description> descProvider, SoftAssertions 
assertions)
+        {
+            for (L left : leftVariants)
+                if (left.isMultiCell() && 
multiCellSupportingTypes.contains(left.getClass()) || !left.isMultiCell())
+                    for (R right : rightVariants)
+                        if (right.isMultiCell() && 
multiCellSupportingTypes.contains(right.getClass()) || !right.isMultiCell())
+                        {
+                            assertions.assertThat(checkPredicate.test(left, 
right))
+                                      .as(descProvider.apply(left, right))
+                                      .isEqualTo(expectPredicate.test(left, 
right));
+
+                            verifyTypesCompatibility(left, right, 
getTypeSupport(right, integers().between(2, 2), 
SourceDSL.arbitrary().constant(AbstractTypeGenerators.ValueDomain.NORMAL)).valueGen);

Review Comment:
   ```suggestion
                               verifyTypesCompatibility(left, right, 
getTypeSupport(right, ignore -> 2, null).valueGen);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]


Reply via email to