mike-tr-adamson commented on code in PR #2540:
URL: https://github.com/apache/cassandra/pull/2540#discussion_r1288504966
##########
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:
I tried various options here but, as you say, we aren't really in the
business of testing byte comparables so it makes no sense to try and do it
here. By removing the token from the byte comparable, the primary keys no
longer maintain any order.
I have considered not using byte comparable at all for the primary keys and
this may be option for the future. We could just serialize the keys and use
byte buffers in place of the comparables. This would allow us to remove some of
the array copying that we currently do and remove our dependency of the
`BytesRef` in the sorted terms.
--
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]
