diff --git a/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java b/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java index c019ba200eb..3e61e6dc32b 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java +++ b/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java @@ -27,6 +27,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.PriorityQueue; import java.util.Queue; import java.util.Set; @@ -365,6 +366,17 @@ public UnfilteredRowIterator computeNext() return key; } + private boolean isEqualToLastKey(PrimaryKey key) + { + // We don't want key.equals(lastKey) because some PrimaryKey implementations consider more than just + // partition key and clustering for equality. This can break lastKey skipping, which is necessary for + // correctness when PrimaryKey doesn't have a clustering (as otherwise, the same partition may get + // filtered and considered as a result multiple times). + return lastKey != null && + Objects.equals(lastKey.partitionKey(), key.partitionKey()) && + Objects.equals(lastKey.clustering(), key.clustering()); + } + private void fillNextSelectedKeysInPartition(DecoratedKey partitionKey, List nextPrimaryKeys) { while (operation.hasNext() @@ -376,7 +388,7 @@ private void fillNextSelectedKeysInPartition(DecoratedKey partitionKey, List nextSelectedKeysInRange() if (firstKey == null) return Collections.emptyList(); } - while (!controller.selects(firstKey) || firstKey.equals(lastKey)); + while (!controller.selects(firstKey) || isEqualToLastKey(firstKey)); lastKey = firstKey; threadLocalNextKeys.add(firstKey); diff --git a/test/unit/org/apache/cassandra/index/sai/cql/LargePartitionsTest.java b/test/unit/org/apache/cassandra/index/sai/cql/LargePartitionsTest.java index bb68459b1e6..7e7e3a6a407 100644 --- a/test/unit/org/apache/cassandra/index/sai/cql/LargePartitionsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/cql/LargePartitionsTest.java @@ -50,7 +50,10 @@ public void testLargePartition() throws Throwable } } - beforeAndAfterFlush( () -> { + // Compaction produces sstables differently than flush, so it is necessary to test sstables written by + // compaction as well. (For example, AA indexes are written as if they were row aware in compaction, but not + // when flushed from the memtable, and that produced the bug fixed in this commit.) + runThenFlushThenCompact( () -> { // test filtering with single partition queries int numExpectedRows = LARGE_PARTITION_SIZE / 2;