You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ad...@apache.org on 2020/10/19 14:31:52 UTC

[cassandra] branch cassandra-3.11 updated: Fix ColumnFilter to avoid querying cells of unselected complex columns

This is an automated email from the ASF dual-hosted git repository.

adelapena pushed a commit to branch cassandra-3.11
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/cassandra-3.11 by this push:
     new c3636bf  Fix ColumnFilter to avoid querying cells of unselected complex columns
c3636bf is described below

commit c3636bf5996fe1dc4541d89d979f6ca18a54fd83
Author: Andrés de la Peña <a....@gmail.com>
AuthorDate: Fri Sep 25 16:30:28 2020 +0100

    Fix ColumnFilter to avoid querying cells of unselected complex columns
    
    patch by Andrés de la Peña; reviewed by Caleb Rackliffe and Benjamin Lerer for CASSANDRA-15977
---
 CHANGES.txt                                        |   1 +
 .../apache/cassandra/db/filter/ColumnFilter.java   |   7 +-
 .../cassandra/db/filter/ColumnFilterTest.java      | 115 ++++++++++++++++++++-
 3 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 98c23c5..df8e6ba 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.11.9
+ * Fix ColumnFilter to avoid querying cells of unselected complex columns (CASSANDRA-15977)
  * Fix memory leak in CompressedChunkReader (CASSANDRA-15880)
  * Don't attempt value skipping with mixed version cluster (CASSANDRA-15833)
  * Avoid failing compactions with very large partitions (CASSANDRA-15164)
diff --git a/src/java/org/apache/cassandra/db/filter/ColumnFilter.java b/src/java/org/apache/cassandra/db/filter/ColumnFilter.java
index 3c79539..e18717c 100644
--- a/src/java/org/apache/cassandra/db/filter/ColumnFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/ColumnFilter.java
@@ -180,7 +180,12 @@ public class ColumnFilter
     public boolean fetchedCellIsQueried(ColumnDefinition column, CellPath path)
     {
         assert path != null;
-        if (!isFetchAll || subSelections == null)
+
+        // first verify that the column to which the cell belongs is queried
+        if (!fetchedColumnIsQueried(column))
+            return false;
+
+        if (subSelections == null)
             return true;
 
         SortedSet<ColumnSubselection> s = subSelections.get(column.name);
diff --git a/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java b/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java
index 41fd0a7..5bbd2f5 100644
--- a/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java
+++ b/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java
@@ -23,7 +23,10 @@ import org.junit.Test;
 import junit.framework.Assert;
 import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ColumnDefinition;
+import org.apache.cassandra.db.PartitionColumns;
 import org.apache.cassandra.db.marshal.Int32Type;
+import org.apache.cassandra.db.marshal.SetType;
+import org.apache.cassandra.db.rows.CellPath;
 import org.apache.cassandra.dht.Murmur3Partitioner;
 import org.apache.cassandra.io.util.DataInputBuffer;
 import org.apache.cassandra.io.util.DataInputPlus;
@@ -31,9 +34,11 @@ import org.apache.cassandra.io.util.DataOutputBuffer;
 import org.apache.cassandra.net.MessagingService;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
+import static org.junit.Assert.assertEquals;
+
 public class ColumnFilterTest
 {
-    final static ColumnFilter.Serializer serializer = new ColumnFilter.Serializer();
+    private static final ColumnFilter.Serializer serializer = new ColumnFilter.Serializer();
 
     @Test
     public void columnFilterSerialisationRoundTrip() throws Exception
@@ -56,7 +61,7 @@ public class ColumnFilterTest
         testRoundTrip(ColumnFilter.selection(metadata.partitionColumns().without(v1)), metadata, MessagingService.VERSION_3014);
     }
 
-    static void testRoundTrip(ColumnFilter columnFilter, CFMetaData metadata, int version) throws Exception
+    private static void testRoundTrip(ColumnFilter columnFilter, CFMetaData metadata, int version) throws Exception
     {
         DataOutputBuffer output = new DataOutputBuffer();
         serializer.serialize(columnFilter, output, version);
@@ -64,4 +69,110 @@ public class ColumnFilterTest
         DataInputPlus input = new DataInputBuffer(output.buffer(), false);
         Assert.assertEquals(serializer.deserialize(input, version, metadata), columnFilter);
     }
+
+    /**
+     * Tests whether a filter fetches and/or queries columns and cells.
+     */
+    @Test
+    public void testFetchedQueried()
+    {
+        CFMetaData metadata = CFMetaData.Builder.create("ks", "table")
+                                              .withPartitioner(Murmur3Partitioner.instance)
+                                              .addPartitionKey("k", Int32Type.instance)
+                                              .addRegularColumn("simple", Int32Type.instance)
+                                              .addRegularColumn("complex", SetType.getInstance(Int32Type.instance, true))
+                                              .build();
+
+        ColumnDefinition simple = metadata.getColumnDefinition(ByteBufferUtil.bytes("simple"));
+        ColumnDefinition complex = metadata.getColumnDefinition(ByteBufferUtil.bytes("complex"));
+        CellPath path1 = CellPath.create(ByteBufferUtil.bytes(1));
+        CellPath path2 = CellPath.create(ByteBufferUtil.bytes(2));
+        ColumnFilter filter;
+
+        // select only the simple column, without table metadata
+        filter = ColumnFilter.selection(PartitionColumns.builder().add(simple).build());
+        assertFetchedQueried(true, true, filter, simple);
+        assertFetchedQueried(false, false, filter, complex);
+        assertFetchedQueried(false, false, filter, complex, path1);
+        assertFetchedQueried(false, false, filter, complex, path2);
+
+        // select only the complex column, without table metadata
+        filter = ColumnFilter.selection(PartitionColumns.builder().add(complex).build());
+        assertFetchedQueried(false, false, filter, simple);
+        assertFetchedQueried(true, true, filter, complex);
+        assertFetchedQueried(true, true, filter, complex, path1);
+        assertFetchedQueried(true, true, filter, complex, path2);
+
+        // select both the simple and complex columns, without table metadata
+        filter = ColumnFilter.selection(PartitionColumns.builder().add(simple).add(complex).build());
+        assertFetchedQueried(true, true, filter, simple);
+        assertFetchedQueried(true, true, filter, complex);
+        assertFetchedQueried(true, true, filter, complex, path1);
+        assertFetchedQueried(true, true, filter, complex, path2);
+
+        // select only the simple column, with table metadata
+        filter = ColumnFilter.selection(metadata, PartitionColumns.builder().add(simple).build());
+        assertFetchedQueried(true, true, filter, simple);
+        assertFetchedQueried(true, false, filter, complex);
+        assertFetchedQueried(true, false, filter, complex, path1);
+        assertFetchedQueried(true, false, filter, complex, path2);
+
+        // select only the complex column, with table metadata
+        filter = ColumnFilter.selection(metadata, PartitionColumns.builder().add(complex).build());
+        assertFetchedQueried(true, false, filter, simple);
+        assertFetchedQueried(true, true, filter, complex);
+        assertFetchedQueried(true, true, filter, complex, path1);
+        assertFetchedQueried(true, true, filter, complex, path2);
+
+        // select both the simple and complex columns, with table metadata
+        filter = ColumnFilter.selection(metadata, PartitionColumns.builder().add(simple).add(complex).build());
+        assertFetchedQueried(true, true, filter, simple);
+        assertFetchedQueried(true, true, filter, complex);
+        assertFetchedQueried(true, true, filter, complex, path1);
+        assertFetchedQueried(true, true, filter, complex, path2);
+
+        // select only the simple column, with selection builder
+        filter = ColumnFilter.selectionBuilder().add(simple).build();
+        assertFetchedQueried(true, true, filter, simple);
+        assertFetchedQueried(false, false, filter, complex);
+        assertFetchedQueried(false, false, filter, complex, path1);
+        assertFetchedQueried(false, false, filter, complex, path2);
+
+        // select only a cell of the complex column, with selection builder
+        filter = ColumnFilter.selectionBuilder().select(complex, path1).build();
+        assertFetchedQueried(false, false, filter, simple);
+        assertFetchedQueried(true, true, filter, complex);
+        assertFetchedQueried(true, true, filter, complex, path1);
+        assertFetchedQueried(true, false, filter, complex, path2);
+
+        // select both the simple column and a cell of the complex column, with selection builder
+        filter = ColumnFilter.selectionBuilder().add(simple).select(complex, path1).build();
+        assertFetchedQueried(true, true, filter, simple);
+        assertFetchedQueried(true, true, filter, complex);
+        assertFetchedQueried(true, true, filter, complex, path1);
+        assertFetchedQueried(true, false, filter, complex, path2);
+    }
+
+    private static void assertFetchedQueried(boolean expectedFetched,
+                                             boolean expectedQueried,
+                                             ColumnFilter filter,
+                                             ColumnDefinition column)
+    {
+        assert !expectedQueried || expectedFetched;
+        boolean actualFetched = filter.fetches(column);
+        assertEquals(expectedFetched, actualFetched);
+        assertEquals(expectedQueried, actualFetched && filter.fetchedColumnIsQueried(column));
+    }
+
+    private static void assertFetchedQueried(boolean expectedFetched,
+                                             boolean expectedQueried,
+                                             ColumnFilter filter,
+                                             ColumnDefinition column,
+                                             CellPath path)
+    {
+        assert !expectedQueried || expectedFetched;
+        boolean actualFetched = filter.fetches(column);
+        assertEquals(expectedFetched, actualFetched);
+        assertEquals(expectedQueried, actualFetched && filter.fetchedCellIsQueried(column, path));
+    }
 }
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org