Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -183,17 +183,18 @@ 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());
}

@Override
public int hashCode()
{
if (hasEmptyClustering)
if (hasClustering)
return Objects.hash(token, clustering());
else
return Objects.hash(token);
return Objects.hash(token, clustering());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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),
Expand All @@ -828,12 +828,12 @@ private ClusteringIndexFilter makeFilter(List<PrimaryKey> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<PrimaryKey> nextPrimaryKeys)
Expand Down
12 changes: 6 additions & 6 deletions src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,10 @@ static void assertIncreasing(Collection<PrimaryKey> 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))
Expand Down Expand Up @@ -304,7 +304,7 @@ public PrimaryKeySet(Collection<PrimaryKey> keys)
{
for (PrimaryKey pk : keys)
{
if (pk.hasEmptyClustering())
if (!pk.hasClustering())
partitions.add(pk.partitionKey());
else
rows.add(Pair.create(pk.partitionKey(), pk.clustering()));
Expand Down
Loading