diff --git a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIterator.java b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIterator.java index 800ca7bccdb4..3a2bc21e2bc5 100644 --- a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIterator.java +++ b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIterator.java @@ -38,6 +38,7 @@ */ public class KeyRangeConcatIterator extends KeyRangeIterator { + public static final String MUST_BE_SORTED_ERROR = "RangeIterator must be sorted, previous max: %s, next min: %s"; private final Iterator ranges; private KeyRangeIterator currentRange; private final List toRelease; diff --git a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIterator.java b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIterator.java index 51b83875c669..a98a05c9eeaa 100644 --- a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIterator.java +++ b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIterator.java @@ -217,7 +217,7 @@ public void update(KeyRangeIterator range) } else if (tokenCount > 0 && max.compareTo(range.getMinimum()) > 0) { - throw new IllegalArgumentException("KeyRangeIterator must be sorted, previous max: " + max + ", next min: " + range.getMinimum()); + throw new IllegalArgumentException(String.format(KeyRangeConcatIterator.MUST_BE_SORTED_ERROR, max, range.getMinimum())); } max = range.getMaximum(); diff --git a/test/unit/org/apache/cassandra/index/sai/iterators/AbstractKeyRangeIteratorTest.java b/test/unit/org/apache/cassandra/index/sai/iterators/AbstractKeyRangeIteratorTest.java index c2ef5a2225e4..8812b48d4877 100644 --- a/test/unit/org/apache/cassandra/index/sai/iterators/AbstractKeyRangeIteratorTest.java +++ b/test/unit/org/apache/cassandra/index/sai/iterators/AbstractKeyRangeIteratorTest.java @@ -48,8 +48,8 @@ public class AbstractKeyRangeIteratorTest extends SaiRandomizedTest { - private static final PrimaryKey.Factory TEST_PRIMARY_KEY_FACTORY = SAIUtil.currentVersion().onDiskFormat() - .newPrimaryKeyFactory(new ClusteringComparator(LongType.instance)); + static final PrimaryKey.Factory TEST_PRIMARY_KEY_FACTORY = SAIUtil.currentVersion().onDiskFormat() + .newPrimaryKeyFactory(new ClusteringComparator(LongType.instance)); protected long[] arr(long... longArray) { diff --git a/test/unit/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIteratorTest.java b/test/unit/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIteratorTest.java index 033fa02d718a..11ec94e11daa 100644 --- a/test/unit/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIteratorTest.java +++ b/test/unit/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIteratorTest.java @@ -23,54 +23,35 @@ import org.junit.Test; +import org.apache.cassandra.dht.Murmur3Partitioner; import org.apache.cassandra.utils.Pair; import static org.apache.cassandra.index.sai.iterators.LongIterator.convert; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class KeyRangeConcatIteratorTest extends AbstractKeyRangeIteratorTest { @Test public void testValidation() { - try - { - buildConcat(build(1L, 4L), build(2L, 3L)); - fail("Flows for a merging concatenation must not contain one another."); - } - catch (IllegalArgumentException ignored) - { - } + // Iterators being merged via concatanation must not include each other + assertThatThrownBy(() -> buildConcat(build(1L, 4L), build(2L, 3L))).isInstanceOf(IllegalArgumentException.class) + .hasMessage(createErrorMessage(4, 2)); - try - { - buildConcat(build(1L, 4L), build(2L, 5L)); - fail("Minimum for flow must not be included in exclusive range of previous flow."); - } - catch (IllegalArgumentException ignored) - { - } + // Iterators being merged via concatanation must not overlap + assertThatThrownBy(() -> buildConcat(build(1L, 4L), build(2L, 5L))).isInstanceOf(IllegalArgumentException.class) + .hasMessage(createErrorMessage(4, 2)); // allow min boundary included KeyRangeIterator concat = buildConcat(build(1L, 4L), build(4L, 5L)); assertEquals(convert(1L, 4L, 4L, 5L), convert(concat)); - try - { - buildConcat(build(1L, 4L), build(0L, 3L)); - fail("Maximum for flow must not be included in exclusive range of previous flow."); - } - catch (IllegalArgumentException ignored) - { - } + assertThatThrownBy(() -> buildConcat(build(1L, 4L), build(0L, 3L))).isInstanceOf(IllegalArgumentException.class) + .hasMessage(createErrorMessage(4, 0)); - try - { - buildConcat(build(2L, 4L), build(0L, 1L)); - fail("Flows for merging concatenation must be sorted."); - } - catch (IllegalArgumentException ignored) - { - } + // Iterators being merged via concatanation must be sorted + assertThatThrownBy(() -> buildConcat(build(2L, 4L), build(0L, 1L))).isInstanceOf(IllegalArgumentException.class) + .hasMessage(createErrorMessage(4, 0)); // with empty flow concat = buildConcat(build(), build(0L, 1L)); @@ -439,4 +420,11 @@ private KeyRangeIterator.Builder getConcatBuilder() { return KeyRangeConcatIterator.builder(); } + + private String createErrorMessage(int max, int min) + { + return String.format(KeyRangeConcatIterator.MUST_BE_SORTED_ERROR, + TEST_PRIMARY_KEY_FACTORY.createTokenOnly(new Murmur3Partitioner.LongToken(max)), + TEST_PRIMARY_KEY_FACTORY.createTokenOnly(new Murmur3Partitioner.LongToken(min))); + } }