natas_hw
/
cassandra
mirror of https://github.com/apache/cassandra
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Correct the default behavior of compareTo() when comparing WIDE and STATIC PrimaryKeys 

patch by Caleb Rackliffe; reviewed by David Capwell for CASSANDRA-20238
This commit is contained in:
Caleb Rackliffe 2025-01-23 11:52:38 -06:00
parent b0226c8ea1
commit 225eb2feeb
13 changed files with 101 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGES.txt
View File

@ -1,4 +1,5 @@
5.0.3
* Correct the default behavior of compareTo() when comparing WIDE and STATIC PrimaryKeys (CASSANDRA-20238)
* Make sure we can set parameters when configuring CassandraCIDRAuthorizer (CASSANDRA-20220)
* Add selected SAI index state and query performance metrics to nodetool tablestats (CASSANDRA-20026)
* Remove v30 and v3X from 5.x in-JVM upgrade tests (CASSANDRA-20103)

4
src/java/org/apache/cassandra/index/sai/disk/v1/postings/PostingListRangeIterator.java
View File

@ -89,7 +89,7 @@ public class PostingListRangeIterator extends KeyRangeIterator
@Override
protected void performSkipTo(PrimaryKey nextKey)
{
if (skipToKey != null && skipToKey.compareTo(nextKey) > 0)
if (skipToKey != null && skipToKey.compareTo(nextKey, false) > 0)
return;
skipToKey = nextKey;
@ -137,7 +137,7 @@ public class PostingListRangeIterator extends KeyRangeIterator
private boolean exhausted()
{
return needsSkipping && skipToKey.compareTo(getMaximum()) > 0;
return needsSkipping && skipToKey.compareTo(getMaximum(), false) > 0;
}
/**

6
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeConcatIterator.java
View File

@ -59,10 +59,10 @@ public class KeyRangeConcatIterator extends KeyRangeIterator
{
KeyRangeIterator currentIterator = ranges.get(current);
if (currentIterator.hasNext() && currentIterator.peek().compareTo(nextKey) >= 0)
if (currentIterator.hasNext() && currentIterator.peek().compareTo(nextKey, false) >= 0)
break;
if (currentIterator.getMaximum().compareTo(nextKey) >= 0)
if (currentIterator.getMaximum().compareTo(nextKey, false) >= 0)
{
currentIterator.skipTo(nextKey);
break;
@ -178,7 +178,7 @@ public class KeyRangeConcatIterator extends KeyRangeIterator
{
min = range.getMinimum();
}
else if (count > 0 && max.compareTo(range.getMinimum()) > 0)
else if (count > 0 && max.compareTo(range.getMinimum(), false) > 0)
{
throw new IllegalArgumentException(String.format(MUST_BE_SORTED_ERROR, max, range.getMinimum()));
}

13
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java
View File

@ -74,7 +74,7 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator
// compare to other keys only by partition.) This loop continues until all iterators point to the same key,
// or if we run out of keys on any of them, or if we exceed the maximum key.
// There is no point in iterating after maximum, because no keys will match beyond that point.
while (highestKey != null && highestKey.compareTo(getMaximum()) <= 0)
while (highestKey != null && highestKey.compareTo(getMaximum(), false) <= 0)
{
// Try to advance all iterators to the highest key seen so far.
// Once this inner loop finishes normally, all iterators are guaranteed to be at the same value.
@ -83,7 +83,7 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator
if (!range.hasNext())
return endOfData();
if (range.peek().compareTo(highestKey) < 0)
if (range.peek().compareTo(highestKey, false) < 0)
{
// If we advance a STATIC key, then we must advance it to the same partition as the highestKey.
// Advancing a STATIC key to a WIDE key directly (without throwing away the clustering) would
@ -95,7 +95,7 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator
// We use strict comparison here, since it orders WIDE primary keys after STATIC primary keys
// in the same partition. When WIDE keys are present, we want to return them rather than STATIC
// keys to avoid retrieving and post-filtering entire partitions.
if (nextKey == null || nextKey.compareToStrict(highestKey) > 0)
if (nextKey == null || nextKey.compareTo(highestKey, true) > 0)
{
// We jumped over the highest key seen so far, so make it the new highest key.
highestKey = nextKey;
@ -105,7 +105,7 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator
// Therefore, restart the inner loop in order to advance the lagging iterators.
continue outer;
}
assert nextKey.compareTo(highestKey) == 0 :
assert nextKey.compareTo(highestKey, false) == 0 :
String.format("Skipped to a key smaller than the target! " +
"iterator: %s, target key: %s, returned key: %s", range, highestKey, nextKey);
}
@ -161,7 +161,7 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator
{
if (!range.hasNext())
return null;
if (range.peek().compareToStrict(max) > 0)
if (range.peek().compareTo(max, true) > 0)
max = range.peek();
}
return max;
@ -394,6 +394,7 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator
*/
private static boolean isDisjointInternal(PrimaryKey min, PrimaryKey max, KeyRangeIterator b)
{
return min == null || max == null || b.getMaxKeys() == 0 || min.compareTo(b.getMaximum()) > 0 || (b.hasNext() && b.peek().compareTo(max) > 0);
return min == null || max == null || b.getMaxKeys() == 0
|| min.compareTo(b.getMaximum(), false) > 0 || (b.hasNext() && b.peek().compareTo(max, false) > 0);
}
}

15
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIterator.java
View File

@ -108,10 +108,10 @@ public abstract class KeyRangeIterator extends AbstractGuavaIterator<PrimaryKey>
if (state == State.DONE)
return;
if (state == State.READY && next.compareTo(nextKey) >= 0)
if (state == State.READY && next.compareTo(nextKey, false) >= 0)
return;
if (max.compareTo(nextKey) < 0)
if (max.compareTo(nextKey, false) < 0)
{
endOfData();
return;
@ -229,7 +229,7 @@ public abstract class KeyRangeIterator extends AbstractGuavaIterator<PrimaryKey>
if (a == null) return b;
if (b == null) return a;
return a.compareToStrict(b) > 0 ? b : a;
return a.compareTo(b) > 0 ? b : a;
}
protected static PrimaryKey nullSafeMax(PrimaryKey a, PrimaryKey b)
@ -237,6 +237,13 @@ public abstract class KeyRangeIterator extends AbstractGuavaIterator<PrimaryKey>
if (a == null) return b;
if (b == null) return a;
return a.compareToStrict(b) > 0 ? a : b;
// The STATIC key sorts before WIDE keys in its partition, but to avoid missing rows while
// intersecting, the STATIC key must override any WIDE key.
if (a.kind() == PrimaryKey.Kind.STATIC && b.kind() == PrimaryKey.Kind.WIDE)
return a.compareTo(b, false) >= 0 ? a : b;
else if (b.kind() == PrimaryKey.Kind.STATIC && a.kind() == PrimaryKey.Kind.WIDE)
return b.compareTo(a, false) >= 0 ? b : a;
return a.compareTo(b) > 0 ? a : b;
}
}

2
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeListIterator.java
View File

@ -50,7 +50,7 @@ public class KeyRangeListIterator extends KeyRangeIterator
{
while (keyQueue.hasNext())
{
if (keyQueue.peek().compareTo(nextKey) >= 0)
if (keyQueue.peek().compareTo(nextKey, false) >= 0)
break;
keyQueue.next();
}

4
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeUnionIterator.java
View File

@ -61,7 +61,7 @@ public class KeyRangeUnionIterator extends KeyRangeIterator
{
PrimaryKey peeked = range.peek();
int cmp = candidateKey.compareTo(peeked);
int cmp = candidateKey.compareTo(peeked, false);
if (cmp == 0)
{
@ -91,7 +91,7 @@ public class KeyRangeUnionIterator extends KeyRangeIterator
// Consume the remaining values equal to the candidate key:
candidate.next();
}
while (candidate.hasNext() && candidate.peek().compareTo(candidateKey) == 0);
while (candidate.hasNext() && candidate.peek().compareTo(candidateKey, false) == 0);
}
return candidateKey;

4
src/java/org/apache/cassandra/index/sai/memory/InMemoryKeyRangeIterator.java
View File

@ -70,7 +70,7 @@ public class InMemoryKeyRangeIterator extends KeyRangeIterator
if (uniqueKeys)
return key;
if (lastKey == null || lastKey.compareTo(key) != 0)
if (lastKey == null || lastKey.compareTo(key, false) != 0)
{
next = key;
lastKey = key;
@ -87,7 +87,7 @@ public class InMemoryKeyRangeIterator extends KeyRangeIterator
while (!keys.isEmpty())
{
PrimaryKey key = keys.peek();
if (key.compareTo(nextKey) >= 0)
if (key.compareTo(nextKey, false) >= 0)
break;
// consume smaller key

2
src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java
View File

@ -316,7 +316,7 @@ public class StorageAttachedIndexSearcher implements Index.Searcher
*/
private boolean isWithinUpperBound(PrimaryKey key)
{
return lastPrimaryKey.token().isMinimum() || lastPrimaryKey.compareTo(key) >= 0;
return lastPrimaryKey.token().isMinimum() || lastPrimaryKey.compareTo(key, false) >= 0;
}
/**

44
src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java
View File

@ -269,10 +269,9 @@ public interface PrimaryKey extends Comparable<PrimaryKey>, ByteComparable
// Otherwise, if the other key is token only we can only compare tokens
// This is used by the ResultRetriever to skip to the current key range start position
// during result retrieval.
if ((cmp != 0) || o.kind() == Kind.TOKEN)
if (cmp != 0 || o.kind() == Kind.TOKEN)
return cmp;
// Finally compare the partition keys
return partitionKey().compareTo(o.partitionKey());
}
@ -320,24 +319,25 @@ public interface PrimaryKey extends Comparable<PrimaryKey>, ByteComparable
}
@Override
public int compareTo(PrimaryKey o)
public int compareTo(PrimaryKey o, boolean strict)
{
int cmp = super.compareTo(o);
if (cmp != 0 || o.kind() == Kind.TOKEN || o.kind() == Kind.SKINNY)
return cmp;
// At this point the other key is in the same partition as this static key so is equal to it. This
// has to be the case because otherwise, intersections between static column indexes and ordinary
// indexes will fail.
// If we're comparing strictly, order this STATIC key before a WIDE key, as this corresponds to the
// order of the corresponding row IDs in an on-disk postings list. If we're not being strict, treat
// the keys as being equal, given they are in the same partition.
if (strict && o.kind() == Kind.WIDE)
return -1;
return 0;
}
@Override
public int compareToStrict(PrimaryKey o)
public int compareTo(PrimaryKey o)
{
int cmp = compareTo(o);
// Always order this STATIC key before a WIDE key in the same partition, as this corresponds to the
// order of the corresponding row IDs in an on-disk postings list.
return o.kind() == Kind.WIDE && cmp == 0 ? -1 : cmp;
return compareTo(o, true);
}
@Override
@ -395,25 +395,25 @@ public interface PrimaryKey extends Comparable<PrimaryKey>, ByteComparable
}
@Override
public int compareTo(PrimaryKey o)
public int compareTo(PrimaryKey o, boolean strict)
{
int cmp = super.compareTo(o);
if (cmp != 0 || o.kind() == Kind.TOKEN || o.kind() == Kind.SKINNY)
return cmp;
// At this point this key is in the same partition as the other key so if the other key is a static
// key then it must be equal to it. See comment in the compareTo for static keys above.
if (o.kind() == Kind.STATIC)
return 0;
// If we're comparing strictly, order this WIDE key after the STATIC key, as this corresponds to the
// order of the corresponding row IDs in an on-disk postings list. If we're not being strict, treat
// the keys as being equal, given they are in the same partition.
return strict ? 1 : 0;
return clusteringComparator.compare(clustering(), o.clustering());
}
@Override
public int compareToStrict(PrimaryKey o)
public int compareTo(PrimaryKey o)
{
int cmp = compareTo(o);
// Always order this WIDE key before a STATIC key in the same partition, as this corresponds to the
// order of the corresponding row IDs in an on-disk postings list.
return o.kind() == Kind.STATIC && cmp == 0 ? 1 : cmp;
return compareTo(o, true);
}
@Override
@ -490,8 +490,8 @@ public interface PrimaryKey extends Comparable<PrimaryKey>, ByteComparable
throw new UnsupportedOperationException("Only STATIC and WIDE keys can be converted to STATIC");
}
default int compareToStrict(PrimaryKey o)
default int compareTo(PrimaryKey key, boolean strict)
{
return compareTo(o);
return compareTo(key);
}
}

46
test/unit/org/apache/cassandra/index/sai/cql/CompositePartitionKeyIndexTest.java
View File

@ -17,13 +17,59 @@
*/
package org.apache.cassandra.index.sai.cql;
import java.math.BigInteger;
import org.junit.Test;
import org.apache.cassandra.cql3.restrictions.StatementRestrictions;
import org.apache.cassandra.db.marshal.SimpleDateType;
import org.apache.cassandra.db.marshal.TimeType;
import org.apache.cassandra.index.sai.SAITester;
public class CompositePartitionKeyIndexTest extends SAITester
{
@Test
public void testIntersectionWithStaticOverlap() throws Throwable
{
createTable("CREATE TABLE %s (pk0 int, pk1 int, ck0 int, s1 int static, v0 int, PRIMARY KEY((pk0, pk1), ck0))");
createIndex("CREATE INDEX ON %s(pk0) USING 'sai'");
execute("UPDATE %s USING TIMESTAMP 1 SET s1 = 0, v0 = 0 WHERE pk0 = 0 AND pk1 = 1 AND ck0 = 0");
execute("DELETE FROM %s USING TIMESTAMP 2 WHERE pk0 = 0 AND pk1 = 1");
// If the STATIC and WIDE PrimaryKey objects in this partition are not compared strictly, the new WIDE key
// will be interpreted as a duplicate and not added to the Memtable-adjacent index. Then, on flush, the row
// corresponding to that WIDE key will be missing from the index.
execute("UPDATE %s USING TIMESTAMP 3 SET v0 = 1 WHERE pk0 = 0 AND pk1 = 1 AND ck0 = 1");
beforeAndAfterFlush(() -> assertRows(execute("SELECT * FROM %s WHERE v0 = 1 AND pk0 = 0 ALLOW FILTERING"), row(0, 1, 1, null, 1)));
}
@Test
public void testIntersectionWithStaticUpdate() throws Throwable
{
createTable("CREATE TABLE %s (pk0 time, pk1 varint, ck0 date, s0 boolean static, s1 text static, v0 boolean, PRIMARY KEY ((pk0, pk1), ck0))");
createIndex("CREATE INDEX tbl_pk0 ON %s(pk0) USING 'sai'");
createIndex("CREATE INDEX tbl_s0 ON %s(s0) USING 'sai'");
// pk0: 23:15:13.897962392 -> (static clustering, -1296648-01-08)
// s0: false -> (static clustering, -1296648-01-08)
execute("INSERT INTO %s (pk0, pk1, ck0, s0, s1, v0) VALUES ('23:15:13.897962392', -2272, '-1296648-01-08', false, 'ᕊଖꥬ㨢걲映㚃', false)");
// pk0: 23:15:13.897962392 -> (static clustering (existing), -1296648-01-08, -1306427-11-21)
// s0: true -> (static clustering, -1306427-11-21)
execute("UPDATE %s SET s0=true, s1='뾕⌒籖' + '鋿紞', v0=true WHERE pk0 = '23:15:13.897962392' AND pk1 = -2272 AND ck0 = '-1306427-11-21'");
// Since the value of "true" is never mapped to the clustering -1296648-01-08, the intersection must begin
// at the STATIC key. Otherwise, we will miss the WIDE key for clustering -1296648-01-08.
beforeAndAfterFlush(() ->
assertRows(execute("SELECT * FROM %s WHERE s0 = true AND pk0 = '23:15:13.897962392'"),
row(TimeType.instance.fromString("23:15:13.897962392"), new BigInteger("-2272"),
SimpleDateType.instance.fromString("-1306427-11-21"), true, "뾕⌒籖鋿紞", true),
row(TimeType.instance.fromString("23:15:13.897962392"), new BigInteger("-2272"),
SimpleDateType.instance.fromString("-1296648-01-08"), true, "뾕⌒籖鋿紞", false)));
}
@Test
public void testCompositePartitionIndex() throws Throwable
{

4
test/unit/org/apache/cassandra/index/sai/iterators/LongIterator.java
View File

@ -63,8 +63,8 @@ public class LongIterator extends KeyRangeIterator
{
for ( ; currentIdx < keys.size(); currentIdx++)
{
PrimaryKey token = keys.get(currentIdx);
if (token.compareTo(nextKey) >= 0)
PrimaryKey key = keys.get(currentIdx);
if (key.compareTo(nextKey, false) >= 0)
break;
}
}

7
test/unit/org/apache/cassandra/index/sai/utils/PrimaryKeyTest.java
View File

@ -372,14 +372,9 @@ public class PrimaryKeyTest extends AbstractPrimaryKeyTester
assertCompareToAndEquals(key, key, 0);
assertCompareToAndEquals(tokenOnlyKey, tokenOnlyKey, 0);
// StaticPrimaryKey is a special case. All other keys in the partition are equal to it
boolean staticComparison = key.kind() == PrimaryKey.Kind.STATIC;
boolean inPartition = staticComparison;
for (int comparisonIndex = index + 1; comparisonIndex < keys.length; comparisonIndex++)
{
if (staticComparison && keys[comparisonIndex].kind() == PrimaryKey.Kind.STATIC)
inPartition = false;
assertCompareToAndEquals(key, keys[comparisonIndex], inPartition ? 0 : -1);
assertCompareToAndEquals(key, keys[comparisonIndex], -1);
assertCompareToAndEquals(tokenOnlyKey, keys[comparisonIndex], tokenOnlyKey.token().equals(keys[comparisonIndex].token()) ? 0 : -1);
}
}