maedhroz commented on code in PR #2989:
URL: https://github.com/apache/cassandra/pull/2989#discussion_r1430328927


##########
src/java/org/apache/cassandra/index/sai/utils/IndexTermType.java:
##########
@@ -525,11 +525,15 @@ public ByteSource asComparableBytes(ByteBuffer value, 
ByteComparable.Version ver
     {
         if (isInetAddress() || isBigInteger() || isBigDecimal())
             return ByteSource.optionalFixedLength(ByteBufferAccessor.instance, 
value);
+        else if (isLong())
             // The LongType.asComparableBytes uses variableLengthInteger which 
doesn't play well with
             // the balanced tree because it is expecting fixed length data. So 
for SAI we use a optionalSignedFixedLengthNumber
             // to keep all comparable values the same length
-        else if (isLong())
             return 
ByteSource.optionalSignedFixedLengthNumber(ByteBufferAccessor.instance, value);
+        else if (isFrozen())
+            // We need to override the default frozen implementation here 
because it will defer to the underlying
+            // type's implementation which will be incorrect, for us, for the 
case of multi-cell types.
+            return ByteSource.of(value, version);

Review Comment:
   Trying to clarify here...
   
   If the underlying implementation is incorrect for multi-cell types, why is 
it the frozen case that we override?



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