From a118a704cbe31bc72699595f112dda7870c85287 Mon Sep 17 00:00:00 2001 From: Caleb Rackliffe Date: Thu, 30 Jan 2025 17:28:37 -0600 Subject: [PATCH] Avoid under-skipping during intersections when an iterator has mixed STATIC and WIDE keys patch by Caleb Rackliffe; reviewed by David Capwell for CASSANDRA-20258 --- CHANGES.txt | 1 + .../index/sai/StorageAttachedIndexGroup.java | 6 ++- .../KeyRangeIntersectionIterator.java | 30 ++++++++++--- .../cql/CompositePartitionKeyIndexTest.java | 42 +++++++++++++++++++ 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a28736122f..4553d03c0a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0.3 + * Avoid under-skipping during intersections when an iterator has mixed STATIC and WIDE keys (CASSANDRA-20258) * 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) diff --git a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java index 30d23f5de6..bddeacecc7 100644 --- a/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java +++ b/src/java/org/apache/cassandra/index/sai/StorageAttachedIndexGroup.java @@ -176,8 +176,10 @@ public class StorageAttachedIndexGroup implements Index.Group, INotificationCons @Override public void insertRow(Row row) { - for (Index.Indexer indexer : indexers) - indexer.insertRow(row); + // SAI does not index deletions, as these are resolved during post-filtering. + if (row.deletion().isLive()) + for (Index.Indexer indexer : indexers) + indexer.insertRow(row); } @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 e820b2fc4b..6237aa0131 100644 --- a/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java +++ b/src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java @@ -85,12 +85,7 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator 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 - // go too far, as WIDE keys are stored after STATIC in the posting list. - PrimaryKey nextKey = range.peek().kind() == Kind.STATIC - ? skipAndPeek(range, highestKey.toStatic()) - : skipAndPeek(range, highestKey); + PrimaryKey nextKey = skipToHighestKey(range); // 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 @@ -129,6 +124,29 @@ public class KeyRangeIntersectionIterator extends KeyRangeIterator return endOfData(); } + private PrimaryKey skipToHighestKey(KeyRangeIterator range) + { + if (range.peek().kind() == highestKey.kind()) + return skipAndPeek(range, highestKey); + + if (range.peek().kind() == Kind.STATIC) + { + // 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 + // go too far, as WIDE keys are stored after STATIC in the posting list. + PrimaryKey nextKey = skipAndPeek(range, highestKey.toStatic()); + + if (nextKey != null && nextKey.compareTo(highestKey, true) < 0 && nextKey.kind() == Kind.WIDE) + // This iterator may have mixed STATIC and non-STATIC postings. Advance again if we've + // landed on a WIDE key that sorts lower in the same partition. + nextKey = skipAndPeek(range, highestKey); + + return nextKey; + } + + return skipAndPeek(range, highestKey); + } + /** * Advances the iterator of one range to the next item, which becomes the highest seen so far. * Iterators pointing to STATIC keys are advanced only if no non-STATIC keys have been advanced. diff --git a/test/unit/org/apache/cassandra/index/sai/cql/CompositePartitionKeyIndexTest.java b/test/unit/org/apache/cassandra/index/sai/cql/CompositePartitionKeyIndexTest.java index f384ca74e4..5d3c9e11d7 100644 --- a/test/unit/org/apache/cassandra/index/sai/cql/CompositePartitionKeyIndexTest.java +++ b/test/unit/org/apache/cassandra/index/sai/cql/CompositePartitionKeyIndexTest.java @@ -22,12 +22,54 @@ import java.math.BigInteger; import org.junit.Test; import org.apache.cassandra.cql3.restrictions.StatementRestrictions; +import org.apache.cassandra.db.marshal.FloatType; import org.apache.cassandra.db.marshal.SimpleDateType; import org.apache.cassandra.db.marshal.TimeType; +import org.apache.cassandra.db.marshal.UUIDType; import org.apache.cassandra.index.sai.SAITester; public class CompositePartitionKeyIndexTest extends SAITester { + @Test + public void testIntersectionOnMixedPostingsOnDelete() throws Throwable + { + createTable("CREATE TABLE %s (pk0 boolean, pk1 uuid, ck0 date, ck1 smallint, s0 timeuuid static, v0 bigint, v1 float, PRIMARY KEY ((pk0, pk1), ck0, ck1)) WITH CLUSTERING ORDER BY (ck0 DESC, ck1 ASC)"); + + createIndex("CREATE INDEX tbl_pk0 ON %s(pk0) USING 'sai'"); + createIndex("CREATE INDEX tbl_ck0 ON %s(ck0) USING 'sai'"); + + execute("INSERT INTO %s (pk0, pk1, ck0, ck1, s0) VALUES (true, 00000000-0000-4700-8d00-000000000000, '-3038243-10-30', -12906, 00000000-0000-1900-aa00-000000000000)"); + execute("INSERT INTO %s (pk0, pk1, ck0, ck1, v0, v1) VALUES (false, 00000000-0000-4f00-a200-000000000000, '-1225324-10-07', -3223, -7318794006633168842, 8.0350916E-32 + 6.127658E28)"); + execute("DELETE FROM %s WHERE pk0 = false AND pk1 = 00000000-0000-4f00-a200-000000000000 AND ck0 = '-1111567-10-09' AND ck1 = 25967"); + execute("DELETE s0 FROM %s WHERE pk0 = false AND pk1 = 00000000-0000-4500-9200-000000000000"); + + beforeAndAfterFlush(() -> + assertRows(execute("SELECT * FROM %s WHERE pk0 = false AND ck0 = '-1225324-10-07'"), + row(false, UUIDType.instance.fromString("00000000-0000-4f00-a200-000000000000"), + SimpleDateType.instance.fromString("-1225324-10-07"), (short) -3223, null, + -7318794006633168842L, FloatType.instance.fromString("6.127658E28")))); + } + + @Test + public void testIntersectionOnMixedPostingsOnUpdate() throws Throwable + { + createTable("CREATE TABLE %s (pk0 boolean, pk1 uuid, ck0 date, ck1 smallint, s0 timeuuid static, v0 bigint, v1 float, PRIMARY KEY ((pk0, pk1), ck0, ck1)) WITH CLUSTERING ORDER BY (ck0 DESC, ck1 ASC)"); + + createIndex("CREATE INDEX tbl_pk0 ON %s(pk0) USING 'sai'"); + createIndex("CREATE INDEX tbl_ck0 ON %s(ck0) USING 'sai'"); + + execute("INSERT INTO %s (pk0, pk1, ck0, ck1, s0) VALUES (true, 00000000-0000-4700-8d00-000000000000, '-3038243-10-30', -12906, 00000000-0000-1900-aa00-000000000000)"); + execute("INSERT INTO %s (pk0, pk1, ck0, ck1, v0, v1) VALUES (false, 00000000-0000-4f00-a200-000000000000, '-1225324-10-07', -3223, -7318794006633168842, 8.0350916E-32 + 6.127658E28)"); + execute("UPDATE %s SET v1 = 2.1 WHERE pk0 = false AND pk1 = 00000000-0000-4f00-a200-000000000000 AND ck0 = '-1111567-10-09' AND ck1 = 25967"); + execute("UPDATE %s SET s0 = 00000000-0000-1900-aa00-000000000000 WHERE pk0 = false AND pk1 = 00000000-0000-4500-9200-000000000000"); + + beforeAndAfterFlush(() -> + assertRows(execute("SELECT * FROM %s WHERE pk0 = false AND ck0 = '-1225324-10-07'"), + row(false, UUIDType.instance.fromString("00000000-0000-4f00-a200-000000000000"), + SimpleDateType.instance.fromString("-1225324-10-07"), (short) -3223, null, + -7318794006633168842L, FloatType.instance.fromString("6.127658E28")))); + } + @Test public void testIntersectionWithStaticOverlap() throws Throwable {