mike-tr-adamson commented on code in PR #2989:
URL: https://github.com/apache/cassandra/pull/2989#discussion_r1430451000


##########
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:
   It's really because we don't support proper indexing on frozen indexing, so 
we don't want the underlying collection as a multi-component `ByteComparable` 
but as a single-component `ByteComparable`. I'll try and come up with some 
wording here that is clearer. We could even mention the ticket for full 
indexing on frozen collections in the comment.



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