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


##########
test/unit/org/apache/cassandra/index/sai/utils/PrimaryKeyTest.java:
##########
@@ -360,22 +348,4 @@ private void compareToAndEqualsTests(PrimaryKey.Factory 
factory, PrimaryKey... k
             }
         }
     }
-
-    private void byteComparisonTests(PrimaryKey.Factory factory, PrimaryKey... 
keys)
-    {
-        for (int index = 0; index < keys.length - 1; index++)
-        {
-            PrimaryKey key = keys[index];
-            PrimaryKey tokenOnlyKey = factory.createTokenOnly(key.token());
-            assertByteComparison(tokenOnlyKey, key, -1);
-            assertByteComparison(key, key, 0);
-            assertByteComparison(tokenOnlyKey, tokenOnlyKey, 0);
-
-            for (int comparisonIndex = index + 1; comparisonIndex < 
keys.length; comparisonIndex++)
-            {
-                assertByteComparison(key, keys[comparisonIndex], -1);
-                assertByteComparison(tokenOnlyKey, keys[comparisonIndex], -1);
-            }
-        }
-    }

Review Comment:
   So I'm trying to understand this in the context of the changes in 
`PrimaryKey`, and it might just be late, but I'm a bit confused. Instead of 
deleting this, I tried something else...
   
   ```
   private void byteComparisonTests(PrimaryKey.Factory factory, PrimaryKey... 
keys)
   {
       for (int index = 0; index < keys.length - 1; index++)
       {
           PrimaryKey key = keys[index];
           assertByteComparison(key, key, 0);
   
           for (int comparisonIndex = index + 1; comparisonIndex < keys.length; 
comparisonIndex++)
           {
               assertTrue(compareTerms(readBytes(key), 
readBytes(keys[comparisonIndex])) < 0);
           }
       }
   }
   
   private int compareTerms(BytesRef left, BytesRef right)
   {
       return FastByteOperations.compareUnsigned(left.bytes, left.offset, 
left.offset + left.length,
                                                 right.bytes, right.offset, 
right.offset + right.length);
   }
   
   private BytesRef readBytes(ByteComparable source)
   {
       BytesRefBuilder builder = new BytesRefBuilder();
   
       ByteSource byteSource = 
source.asComparableBytes(ByteComparable.Version.OSS50);
       int val;
       while ((val = byteSource.next()) != ByteSource.END_OF_STREAM)
           builder.append((byte) val);
       return builder.get();
   }
   ```
   This seems to be more or less what `SortedTermsReader` does, and we've got a 
sequence of strictly increasing integers for our simple primary keys, but it 
fails. Is there no way to compare them in unsigned byte space?



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