maedhroz commented on code in PR #2540:
URL: https://github.com/apache/cassandra/pull/2540#discussion_r1283994231
##########
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:
Okay, so I think I see what happened! The sort takes the token into account,
and that reorders the byte-comparables. So you could do this, and it would work
just fine:
```
@Test
public void singlePartitionTest()
{
PrimaryKey.Factory factory = new
PrimaryKey.Factory(simplePartition.comparator);
int rows = nextInt(10, 100);
PrimaryKey[] keys = new PrimaryKey[rows];
for (int index = 0; index < rows; index++)
keys[index] = factory.create(makeKey(simplePartition, index),
Clustering.EMPTY);
byteComparisonTests(keys);
Arrays.sort(keys);
compareToAndEqualsTests(factory, keys);
}
```
...and then...
```
private void byteComparisonTests(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++)
{
assertByteComparison(key, keys[comparisonIndex], -1);
}
}
}
```
I suppose your removal says that you felt this wasn't really that useful,
given we're not in the business of testing byte-comparable types themselves,
and our usage of this is covered in `SortedTermsTest`?
--
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]
