diff --git a/src/java/org/apache/cassandra/index/sai/disk/PrimaryKeyMapIterator.java b/src/java/org/apache/cassandra/index/sai/disk/PrimaryKeyMapIterator.java index 44b5258d90f4..1f10741c0ba6 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/PrimaryKeyMapIterator.java +++ b/src/java/org/apache/cassandra/index/sai/disk/PrimaryKeyMapIterator.java @@ -111,7 +111,7 @@ protected PrimaryKey computeNext() while (currentRowId >= 0 && currentRowId < keys.count()) { PrimaryKey key = keys.primaryKeyFromRowId(currentRowId++, getMinimum(), getMaximum()); - if (filter == KeyFilter.KEYS_WITH_CLUSTERING && key.hasEmptyClustering()) + if (filter == KeyFilter.KEYS_WITH_CLUSTERING && !key.hasClustering()) continue; return key; } diff --git a/src/java/org/apache/cassandra/index/sai/disk/v2/RowAwarePrimaryKeyFactory.java b/src/java/org/apache/cassandra/index/sai/disk/v2/RowAwarePrimaryKeyFactory.java index ca92dc17326c..c474c1a75a0a 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/v2/RowAwarePrimaryKeyFactory.java +++ b/src/java/org/apache/cassandra/index/sai/disk/v2/RowAwarePrimaryKeyFactory.java @@ -41,13 +41,13 @@ public class RowAwarePrimaryKeyFactory implements PrimaryKey.Factory { private final ClusteringComparator clusteringComparator; - private final boolean hasEmptyClustering; + private final boolean hasClustering; public RowAwarePrimaryKeyFactory(ClusteringComparator clusteringComparator) { this.clusteringComparator = clusteringComparator; - this.hasEmptyClustering = clusteringComparator.size() == 0; + this.hasClustering = clusteringComparator.size() > 0; } @Override @@ -183,7 +183,7 @@ public int compareTo(PrimaryKey o) // clusterings then we can return the result of this without // needing to compare the clusterings cmp = partitionKey().compareTo(o.partitionKey()); - if (cmp != 0 || hasEmptyClustering() || o.hasEmptyClustering()) + if (cmp != 0 || !hasClustering() || !o.hasClustering()) return cmp; return clusteringComparator.compare(clustering(), o.clustering()); } @@ -191,9 +191,10 @@ public int compareTo(PrimaryKey o) @Override public int hashCode() { - if (hasEmptyClustering) + if (hasClustering) + return Objects.hash(token, clustering()); + else return Objects.hash(token); - return Objects.hash(token, clustering()); } @Override diff --git a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java index 822fc2d9e838..e753490b21a3 100644 --- a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java +++ b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java @@ -102,7 +102,7 @@ protected PrimaryKey computeNext() // More specific keys should win over full partitions, // because they match a single row instead of the whole partition. // However, because this key matches with the earlier keys, we can continue the inner loop. - if (!nextKey.hasEmptyClustering()) + if (nextKey.hasClustering()) { highestKey = nextKey; indexOfHighestKey = index; @@ -113,13 +113,13 @@ protected PrimaryKey computeNext() // Now we need to advance the iterators to avoid returning the same key again. // This is tricky because of empty clustering keys that match the whole partition. - // We must not advance ranges at keys with empty clustering because they + // We must not advance ranges at keys with no clustering because they // may still match the next keys returned by other iterators in the next cycles. - // However, if all ranges are at the same partition with empty clustering (highestKey.hasEmptyClustering()), + // However, if all ranges are at the same partition with no clustering (!highestKey.hasClustering()), // we must advance all of them, because we return the key for the whole partition and that partition is done. for (var range : ranges) { - if (highestKey.hasEmptyClustering() || !range.peek().hasEmptyClustering()) + if (!highestKey.hasClustering() || range.peek().hasClustering()) range.next(); } diff --git a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeUnionIterator.java b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeUnionIterator.java index 2b0199dae143..57e28e1d5511 100644 --- a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeUnionIterator.java +++ b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeUnionIterator.java @@ -84,7 +84,7 @@ public PrimaryKey computeNext() // If we chose one of the specific keys with non-empty clustering (e.g. pick the first one we see), // we may miss rows matched by the non-row-aware index, as well as the rows matched by // the other row-aware indexes. - if (range.peek().hasEmptyClustering() && !candidate.peek().hasEmptyClustering()) + if (!range.peek().hasClustering() && candidate.peek().hasClustering()) candidate = range; else range.next(); // truly equal by partition and clustering, so we can just get rid of one @@ -105,7 +105,7 @@ else if (cmp > 0) // advance all other ranges to the end of this partition to avoid duplicates. // We delay that to the next call to computeNext() though, because if we have a wide partition, it's better // to first let the caller consume all the rows from this partition - maybe they won't call again. - if (result.hasEmptyClustering()) + if (!result.hasClustering()) partitionToSkip = result.partitionKey(); return result; diff --git a/src/java/org/apache/cassandra/index/sai/memory/MemtableKeyRangeIterator.java b/src/java/org/apache/cassandra/index/sai/memory/MemtableKeyRangeIterator.java index bc8fab194c8a..986198d1f834 100644 --- a/src/java/org/apache/cassandra/index/sai/memory/MemtableKeyRangeIterator.java +++ b/src/java/org/apache/cassandra/index/sai/memory/MemtableKeyRangeIterator.java @@ -107,7 +107,7 @@ protected void performSkipTo(PrimaryKey nextKey) if (partitionIterator.hasNext()) { this.rowIterator = partitionIterator.next(); - if (!nextKey.hasEmptyClustering() && rowIterator.partitionKey().equals(nextKey.partitionKey())) + if (nextKey.hasClustering() && rowIterator.partitionKey().equals(nextKey.partitionKey())) { Slice slice = Slice.make(nextKey.clustering(), Clustering.EMPTY); Slices slices = Slices.with(memtable.metadata().comparator, slice); diff --git a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java index 97b2122b72ba..bfc8405ac1b7 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java +++ b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java @@ -799,7 +799,7 @@ public Orderer getOrderer() */ public boolean selects(PrimaryKey key) { - return key.hasEmptyClustering() || + return !key.hasClustering() || command.clusteringIndexFilter(key.partitionKey()).selects(key.clustering()); } @@ -816,7 +816,7 @@ private ClusteringIndexFilter makeFilter(PrimaryKey key) { ClusteringIndexFilter clusteringIndexFilter = command.clusteringIndexFilter(key.partitionKey()); - if (!indexFeatureSet.isRowAware() || key.hasEmptyClustering()) + if (!indexFeatureSet.isRowAware() || !key.hasClustering()) return clusteringIndexFilter; else return new ClusteringIndexNamesFilter(FBUtilities.singleton(key.clustering(), cfs.metadata().comparator), @@ -828,12 +828,12 @@ private ClusteringIndexFilter makeFilter(List keys) PrimaryKey firstKey = keys.get(0); assert !indexFeatureSet.isRowAware() || - cfs.metadata().comparator.size() == 0 && firstKey.hasEmptyClustering() || - cfs.metadata().comparator.size() > 0 && (!firstKey.hasEmptyClustering() || cfs.metadata().hasStaticColumns()): + cfs.metadata().comparator.size() == 0 && !firstKey.hasClustering() || + cfs.metadata().comparator.size() > 0 && (firstKey.hasClustering() || cfs.metadata().hasStaticColumns()) : "PrimaryKey " + firstKey + " clustering does not match table. There should be a clustering of size " + cfs.metadata().comparator.size(); ClusteringIndexFilter clusteringIndexFilter = command.clusteringIndexFilter(firstKey.partitionKey()); - if (cfs.metadata().comparator.size() == 0 || firstKey.hasEmptyClustering()) + if (cfs.metadata().comparator.size() == 0 || !firstKey.hasClustering()) { return clusteringIndexFilter; } 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 64eb418dcf3d..722c5e0f1b86 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java +++ b/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java @@ -354,7 +354,7 @@ private boolean isEqualToLastKey(PrimaryKey key) // filtered and considered as a result multiple times). return lastKey != null && Objects.equals(lastKey.partitionKey(), key.partitionKey()) && - (lastKey.hasEmptyClustering() || key.hasEmptyClustering() || Objects.equals(lastKey.clustering(), key.clustering())); + (!lastKey.hasClustering() || !key.hasClustering() || Objects.equals(lastKey.clustering(), key.clustering())); } private void fillNextSelectedKeysInPartition(DecoratedKey partitionKey, List nextPrimaryKeys) diff --git a/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java b/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java index fb654ee955cc..584e63f84ca9 100644 --- a/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java +++ b/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java @@ -150,15 +150,15 @@ default boolean isTokenOnly() Clustering clustering(); /** - * Return whether the primary key has an empty clustering or not. - * By default the clustering is empty if the internal clustering - * is null or is empty. + * Return whether the primary key has a clustering or not. + * By default the clustering exists if the internal clustering + * is not null and is not empty. * - * @return {@code true} if the clustering is empty, otherwise {@code false} + * @return {@code true} if the clustering exists, otherwise {@code false} */ - default boolean hasEmptyClustering() + default boolean hasClustering() { - return clustering() == null || clustering().isEmpty(); + return clustering() != null && !clustering().isEmpty(); } /** diff --git a/test/unit/org/apache/cassandra/index/sai/cql/VectorCompactionTest.java b/test/unit/org/apache/cassandra/index/sai/cql/VectorCompactionTest.java index e33d1514c910..33533d25e766 100644 --- a/test/unit/org/apache/cassandra/index/sai/cql/VectorCompactionTest.java +++ b/test/unit/org/apache/cassandra/index/sai/cql/VectorCompactionTest.java @@ -372,7 +372,7 @@ private void validatePostingsStructureAndOrdinalToVectorMapping(String indexName for (long i = segment.metadata.minSSTableRowId; i <= segment.metadata.maxSSTableRowId; i++) { var primaryKey = pkm.primaryKeyFromRowId(i); - assertTrue("The subsequent logic assumes that we have no clustering columns", primaryKey.hasEmptyClustering()); + assertTrue("The subsequent logic assumes that we have no clustering columns", !primaryKey.hasClustering()); try (var sstableIter = segment.sstableContext .sstable() .iterator(primaryKey.partitionKey(), Slices.ALL, columnFilter, false, SSTableReadsListener.NOOP_LISTENER)) 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 8812b48d4877..047fcc933f7d 100644 --- a/test/unit/org/apache/cassandra/index/sai/iterators/AbstractKeyRangeIteratorTest.java +++ b/test/unit/org/apache/cassandra/index/sai/iterators/AbstractKeyRangeIteratorTest.java @@ -273,10 +273,10 @@ static void assertIncreasing(Collection keys) Clustering lastClustering = Clustering.EMPTY; for (PrimaryKey key : keys) { - if (key.hasEmptyClustering() && key.partitionKey().equals(lastPartitionKey)) + if (!key.hasClustering() && key.partitionKey().equals(lastPartitionKey)) throw new AssertionError("A primary key with empty clustering follows a key in the same partition:\n" + key + "\nafter:\n" + lastPrimaryKey); - if (!key.hasEmptyClustering() && lastClustering.isEmpty() && key.partitionKey().equals(lastPartitionKey)) + if (key.hasClustering() && lastClustering.isEmpty() && key.partitionKey().equals(lastPartitionKey)) throw new AssertionError("A primary key with non-empty clustering follows a key with empty clustering in the same partition:\n" + key + "\nafter:\n" + lastPrimaryKey); if (Objects.equals(key, lastPrimaryKey)) @@ -304,7 +304,7 @@ public PrimaryKeySet(Collection keys) { for (PrimaryKey pk : keys) { - if (pk.hasEmptyClustering()) + if (!pk.hasClustering()) partitions.add(pk.partitionKey()); else rows.add(Pair.create(pk.partitionKey(), pk.clustering()));