lukasz-antoniak commented on code in PR #4431:
URL: https://github.com/apache/cassandra/pull/4431#discussion_r2613732273
##########
test/unit/org/apache/cassandra/db/marshal/AbstractTypeTest.java:
##########
@@ -960,17 +960,31 @@ public void testAssumedCompatibility()
@Test
public void testBackwardCompatibility()
{
+ // Fix for CASSANDRA-20979 altered behaviour of SimpleDateType and
TimeType by correctly
+ // returning the fix-length size in valueLengthIfFixed(). This change
impacted logic of
+ // isSerializationCompatibleWith(), which as a result complains about
backward compatibility of:
+ // 1) [SimpleDateType isSerializationCompatibleWith Int32Type, {}]
expected:<[fals]e> but was:<[tru]e>
+ // 2) [TimeType isSerializationCompatibleWith LongType, {}]
expected:<[fals]e> but was:<[tru]e>
+ // 3) [BytesType isSerializationCompatibleWith SimpleDateType, {}]
expected:<[tru]e> but was:<[fals]e>
+ // 4) [BytesType isSerializationCompatibleWith TimeType, {}]
expected:<[tru]e> but was:<[fals]e>
+ ImmutableMap<AbstractType, Set<AbstractType>> exclusions =
ImmutableMap.<AbstractType, Set<AbstractType>>builder()
+
.put(SimpleDateType.instance, ImmutableSet.of(Int32Type.instance))
+
.put(TimeType.instance, ImmutableSet.of(LongType.instance))
+
.put(BytesType.instance, ImmutableSet.of(SimpleDateType.instance,
TimeType.instance))
+
.build();
+
Review Comment:
To serialize the vector, we use `VectorCodec` which relies on
`TypeCodec#serializedSize` to decide is subtype is fixed or variable length:
```
public static <E> VectorCodec<E> of(VectorType type, TypeCodec<E>
subtypeCodec)
{
return subtypeCodec.isSerializedSizeFixed()
? new FixedLength<>(type, subtypeCodec)
: new VariableLength<>(type, subtypeCodec);
}
```
When constructing `VectorType`, we actually relay on
`AbstractType#valueLengthIfFixed`:
```
private VectorType(AbstractType<T> elementType, int dimension)
{
super(ComparisonType.CUSTOM);
// ...
this.serializer = elementType.isValueLengthFixed() ?
new FixedLengthSerializer() :
new VariableLengthSerializer();
}
```
Looking at failed stack trace:
```
org.apache.cassandra.serializers.MarshalException: Expected 8 byte long for
time (0)
at
org.apache.cassandra.serializers.TimeSerializer.validate(TimeSerializer.java:74)
at
org.apache.cassandra.db.marshal.VectorType$VariableLengthSerializer.unpack(VectorType.java:613)
at
org.apache.cassandra.db.marshal.VectorType.unpack(VectorType.java:142)
at
org.apache.cassandra.db.marshal.MultiElementType.unpack(MultiElementType.java:81)
at
org.apache.cassandra.cql3.terms.MultiElements$Value.fromSerialized(MultiElements.java:69)
at org.apache.cassandra.cql3.terms.Marker.bind(Marker.java:85)
at
org.apache.cassandra.cql3.terms.Term$NonTerminal.bindAndGet(Term.java:304)
at
org.apache.cassandra.cql3.terms.Constants$Setter.execute(Constants.java:482)
at
org.apache.cassandra.cql3.statements.UpdateStatement.addUpdateForKey(UpdateStatement.java:126)
at
org.apache.cassandra.io.sstable.CQLSSTableWriter.rawAddRow(CQLSSTableWriter.java:314)
at
org.apache.cassandra.io.sstable.CQLSSTableWriter.addRow(CQLSSTableWriter.java:215)
at
org.apache.cassandra.io.sstable.CQLSSTableWriter.addRow(CQLSSTableWriter.java:185)
at
org.apache.cassandra.io.sstable.CQLSSTableWriterTest.testWritingVectorData(CQLSSTableWriterTest.java:1678)
```
The `unpack` operation is performed on `MultiElementType`, which in the end
uses `VectorType`. If `VectorCodec` thinks that type is of fixed-length,
whereas `VectorType` assumes it is variable-length, the `unpack` operation will
fail.
Proposed changes do not modify serialization or deserialization of types,
but just synchronize behaviour of `TypeCodec#serializedSize` and
`AbstractType#valueLengthIfFixed`.
--
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]