From 4bcae8e57e847200c9f202b325fff97f62eaa7d0 Mon Sep 17 00:00:00 2001 From: Caleb Rackliffe Date: Thu, 17 Apr 2025 14:35:47 -0500 Subject: [PATCH] Ensure RowFilter#isMutableIntersection() properly evaluates numeric ranges on a single column patch by Caleb Rackliffe; reviewed by Ariel Weisberg for CASSANDRA-20566 --- CHANGES.txt | 1 + .../apache/cassandra/db/filter/RowFilter.java | 13 +++++- .../cassandra/db/filter/RowFilterTest.java | 41 ++++++++++++++++--- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 545ba01dab..5ec727e79f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0.5 + * Ensure RowFilter#isMutableIntersection() properly evaluates numeric ranges on a single column (CASSANDRA-20566) * Switch memtable-related off-heap objects to Native Endian and Memory to Little Endian (CASSANDRA-20190) * SAI marks an index as non-empty when a partial partition/row modifications is flushed due to repair (CASSANDRA-20567) * SAI fails queries when multiple columns exist and a non-indexed column is a composite with a map (CASSANDRA-19891) diff --git a/src/java/org/apache/cassandra/db/filter/RowFilter.java b/src/java/org/apache/cassandra/db/filter/RowFilter.java index f1b095920f..2cb0af969d 100644 --- a/src/java/org/apache/cassandra/db/filter/RowFilter.java +++ b/src/java/org/apache/cassandra/db/filter/RowFilter.java @@ -21,13 +21,16 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; import com.google.common.base.Objects; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -173,15 +176,21 @@ public class RowFilter implements Iterable */ public boolean isMutableIntersection() { - int count = 0; + Set columns = null; for (Expression e : expressions) { if (e.column.isStatic() && expressions.size() > 1) return true; if (!e.column.isPrimaryKeyColumn()) - if (++count > 1) + { + if (columns == null) + columns = new HashSet<>(expressions.size()); + + columns.add(e.column); + if (columns.size() > 1) return true; + } } return false; } diff --git a/test/unit/org/apache/cassandra/db/filter/RowFilterTest.java b/test/unit/org/apache/cassandra/db/filter/RowFilterTest.java index 8952262e2b..d2def5ba97 100644 --- a/test/unit/org/apache/cassandra/db/filter/RowFilterTest.java +++ b/test/unit/org/apache/cassandra/db/filter/RowFilterTest.java @@ -20,9 +20,10 @@ package org.apache.cassandra.db.filter; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; -import org.junit.Assert; +import com.google.common.collect.ImmutableList; import org.junit.Test; import org.apache.cassandra.cql3.ColumnIdentifier; @@ -33,6 +34,7 @@ import org.apache.cassandra.db.DeletionTime; import org.apache.cassandra.db.LivenessInfo; import org.apache.cassandra.db.RegularAndStaticColumns; import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.db.partitions.SingletonUnfilteredPartitionIterator; import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; import org.apache.cassandra.db.rows.BTreeRow; @@ -45,8 +47,12 @@ import org.apache.cassandra.db.rows.Unfiltered; import org.apache.cassandra.db.rows.UnfilteredRowIterator; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.btree.BTree; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + public class RowFilterTest { @@ -88,8 +94,8 @@ public class RowFilterTest closed.set(true); } }), 1); - Assert.assertFalse(iter.hasNext()); - Assert.assertTrue(closed.get()); + assertFalse(iter.hasNext()); + assertTrue(closed.get()); filter = RowFilter.none().withNewExpressions(new ArrayList<>()); filter.add(r, Operator.NEQ, one); @@ -122,9 +128,34 @@ public class RowFilterTest closed.set(true); } }), 1); - Assert.assertFalse(iter.hasNext()); - Assert.assertTrue(closed.get()); + assertFalse(iter.hasNext()); + assertTrue(closed.get()); } + @Test + public void testMutableIntersections() + { + TableMetadata metadata = TableMetadata.builder("testks", "testcf") + .addPartitionKeyColumn("pk", Int32Type.instance) + .addRegularColumn("r", Int32Type.instance) + .addRegularColumn("t", UTF8Type.instance) + .build(); + RowFilter filter = RowFilter.none().withNewExpressions(new ArrayList<>()); + assertFalse(filter.isMutableIntersection()); + + ColumnMetadata r = metadata.getColumn(new ColumnIdentifier("r", true)); + RowFilter.Expression gt = new RowFilter.SimpleExpression(r, Operator.GT, ByteBufferUtil.EMPTY_BYTE_BUFFER); + filter = filter.withNewExpressions(Collections.singletonList(gt)); + assertFalse(filter.isMutableIntersection()); + + RowFilter.Expression lt = new RowFilter.SimpleExpression(r, Operator.LT, ByteBufferUtil.EMPTY_BYTE_BUFFER); + filter = filter.withNewExpressions(ImmutableList.of(gt, lt)); + assertFalse(filter.isMutableIntersection()); + + ColumnMetadata t = metadata.getColumn(new ColumnIdentifier("t", true)); + RowFilter.Expression eq = new RowFilter.SimpleExpression(t, Operator.EQ, ByteBufferUtil.EMPTY_BYTE_BUFFER); + filter = filter.withNewExpressions(ImmutableList.of(gt, lt, eq)); + assertTrue(filter.isMutableIntersection()); + } }