cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From s...@apache.org
Subject [3/6] cassandra git commit: Consider secondary indexes when preparing clustering column restrictions
Date Fri, 29 Apr 2016 11:01:27 GMT
Consider secondary indexes when preparing clustering column restrictions

Patch by Sam Tunnicliffe; reviewed by Benjamin Lerer for CASSANDRA-11510


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/dff7b447
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/dff7b447
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/dff7b447

Branch: refs/heads/trunk
Commit: dff7b447a74d639888eca76c940c057d3fa647e7
Parents: 38e973d
Author: Sam Tunnicliffe <sam@beobal.com>
Authored: Wed Apr 20 12:01:53 2016 +0100
Committer: Sam Tunnicliffe <sam@beobal.com>
Committed: Fri Apr 29 11:46:16 2016 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../restrictions/PrimaryKeyRestrictionSet.java  | 71 +++++++++++++++++++-
 .../restrictions/StatementRestrictions.java     | 27 +++++---
 .../SelectSingleColumnRelationTest.java         |  6 +-
 4 files changed, 90 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/dff7b447/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index d5cccfa..fb06cd6 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.2.7
+ * Restore ability to filter on clustering columns when using a 2i (CASSANDRA-11510)
  * JSON datetime formatting needs timezone (CASSANDRA-11137)
  * Fix is_dense recalculation for Thrift-updated tables (CASSANDRA-11502)
  * Remove unnescessary file existence check during anticompaction (CASSANDRA-11660)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/dff7b447/src/java/org/apache/cassandra/cql3/restrictions/PrimaryKeyRestrictionSet.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/restrictions/PrimaryKeyRestrictionSet.java
b/src/java/org/apache/cassandra/cql3/restrictions/PrimaryKeyRestrictionSet.java
index 7ce228c..0c10f13 100644
--- a/src/java/org/apache/cassandra/cql3/restrictions/PrimaryKeyRestrictionSet.java
+++ b/src/java/org/apache/cassandra/cql3/restrictions/PrimaryKeyRestrictionSet.java
@@ -22,16 +22,18 @@ import java.util.*;
 
 import com.google.common.collect.Lists;
 
-import org.apache.cassandra.db.composites.Composite;
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ColumnDefinition;
 import org.apache.cassandra.cql3.QueryOptions;
 import org.apache.cassandra.cql3.functions.Function;
 import org.apache.cassandra.cql3.statements.Bound;
 import org.apache.cassandra.db.IndexExpression;
+import org.apache.cassandra.db.Keyspace;
 import org.apache.cassandra.db.composites.*;
 import org.apache.cassandra.db.composites.Composite.EOC;
 import org.apache.cassandra.db.index.SecondaryIndexManager;
 import org.apache.cassandra.exceptions.InvalidRequestException;
+
 import static org.apache.cassandra.cql3.statements.RequestValidations.checkFalse;
 import static org.apache.cassandra.cql3.statements.RequestValidations.invalidRequest;
 
@@ -65,20 +67,48 @@ final class PrimaryKeyRestrictionSet extends AbstractPrimaryKeyRestrictions
      */
     private boolean contains;
 
+
+    /**
+     * If restrictions apply to clustering columns, we need to check whether they can be
satisfied by an index lookup
+     * as this affects which other restrictions can legally be specified (if an index is
present, we are more lenient
+     * about what additional filtering can be performed on the results of a lookup - see
CASSANDRA-11510).
+     *
+     * We don't hold a reference to the SecondaryIndexManager itself as this is not strictly
a singleton (although
+     * we often treat is as one), the field would also require annotation with @Unmetered
to avoid blowing up the
+     * object size (used when calculating the size of prepared statements for caching). Instead,
we refer to the
+     * CFMetaData and retrieve the index manager when necessary.
+     *
+     * There are a couple of scenarios where the CFM can be null (and we make sure and test
for null when we use it):
+     *  * where an empty set of restrictions are created for use in processing query results
- see
+     *    SelectStatement.forSelection
+     *  * where the restrictions apply to partition keys and not clustering columns e.g.
+     *    StatementRestrictions.partitionKeyRestrictions
+     *  * in unit tests (in particular PrimaryKeyRestrictionSetTest which is primarily concerned
with the correct
+     *    generation of bounds when secondary indexes are not used).
+     */
+    private final CFMetaData cfm;
+
     public PrimaryKeyRestrictionSet(CType ctype)
     {
+        this(ctype, null);
+    }
+
+    public PrimaryKeyRestrictionSet(CType ctype, CFMetaData cfm)
+    {
         super(ctype);
+        this.cfm = cfm;
         this.restrictions = new RestrictionSet();
         this.eq = true;
     }
 
     private PrimaryKeyRestrictionSet(PrimaryKeyRestrictionSet primaryKeyRestrictions,
-                                               Restriction restriction) throws InvalidRequestException
+                                     Restriction restriction) throws InvalidRequestException
     {
         super(primaryKeyRestrictions.ctype);
         this.restrictions = primaryKeyRestrictions.restrictions.addRestriction(restriction);
+        this.cfm = primaryKeyRestrictions.cfm;
 
-        if (!primaryKeyRestrictions.isEmpty())
+        if (!primaryKeyRestrictions.isEmpty() && !hasSupportingIndex(restriction))
         {
             ColumnDefinition lastRestrictionStart = primaryKeyRestrictions.restrictions.lastRestriction().getFirstColumn();
             ColumnDefinition newRestrictionStart = restriction.getFirstColumn();
@@ -104,6 +134,12 @@ final class PrimaryKeyRestrictionSet extends AbstractPrimaryKeyRestrictions
             this.eq = true;
     }
 
+    private boolean hasSupportingIndex(Restriction restriction)
+    {
+        return cfm != null
+               && restriction.hasSupportingIndex(Keyspace.open(cfm.ksName).getColumnFamilyStore(cfm.cfId).indexManager);
+    }
+
     @Override
     public boolean isSlice()
     {
@@ -392,4 +428,33 @@ final class PrimaryKeyRestrictionSet extends AbstractPrimaryKeyRestrictions
     {
         return restrictions.lastColumn();
     }
+
+    public final boolean needsFiltering()
+    {
+        // Backported from ClusteringColumnRestrictions from CASSANDRA-11310 for 3.6
+        // As that suggests, this should only be called on clustering column
+        // and not partition key restrictions.
+        int position = 0;
+        Restriction slice = null;
+        for (Restriction restriction : restrictions)
+        {
+            if (handleInFilter(restriction, position))
+                return true;
+
+            if (slice != null && !slice.getFirstColumn().equals(restriction.getFirstColumn()))
+                return true;
+
+            if (slice == null && restriction.isSlice())
+                slice = restriction;
+            else
+                position = restriction.getLastColumn().position() + 1;
+        }
+
+        return false;
+    }
+
+    private boolean handleInFilter(Restriction restriction, int index)
+    {
+        return restriction.isContains() || index != restriction.getFirstColumn().position();
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/dff7b447/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java
index 3934f33..5b7c58d 100644
--- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java
+++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java
@@ -25,6 +25,7 @@ import com.google.common.collect.Iterables;
 
 import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ColumnDefinition;
+import org.apache.cassandra.config.Schema;
 import org.apache.cassandra.cql3.*;
 import org.apache.cassandra.cql3.functions.Function;
 import org.apache.cassandra.cql3.statements.Bound;
@@ -115,7 +116,7 @@ public final class StatementRestrictions
     {
         this.cfm = cfm;
         this.partitionKeyRestrictions = new PrimaryKeyRestrictionSet(cfm.getKeyValidatorAsCType());
-        this.clusteringColumnsRestrictions = new PrimaryKeyRestrictionSet(cfm.comparator);
+        this.clusteringColumnsRestrictions = new PrimaryKeyRestrictionSet(cfm.comparator,
cfm);
         this.nonPrimaryKeyRestrictions = new RestrictionSet();
 
         /*
@@ -128,9 +129,7 @@ public final class StatementRestrictions
         for (Relation relation : whereClause)
             addRestriction(relation.toRestriction(cfm, boundNames));
 
-        ColumnFamilyStore cfs = Keyspace.open(cfm.ksName).getColumnFamilyStore(cfm.cfName);
-        SecondaryIndexManager secondaryIndexManager = cfs.indexManager;
-
+        SecondaryIndexManager secondaryIndexManager = Keyspace.open(cfm.ksName).getColumnFamilyStore(cfm.cfName).indexManager;
         boolean hasQueriableClusteringColumnIndex = clusteringColumnsRestrictions.hasSupportingIndex(secondaryIndexManager);
         boolean hasQueriableIndex = hasQueriableClusteringColumnIndex
                 || partitionKeyRestrictions.hasSupportingIndex(secondaryIndexManager)
@@ -313,8 +312,14 @@ public final class StatementRestrictions
         checkFalse(clusteringColumnsRestrictions.isContains() && !hasQueriableIndex,
                    "Cannot restrict clustering columns by a CONTAINS relation without a secondary
index");
 
-        if (hasClusteringColumnsRestriction())
+        if (hasClusteringColumnsRestriction() && clusteringRestrictionsNeedFiltering())
         {
+            if (hasQueriableIndex)
+            {
+                usesSecondaryIndexing = true;
+                return;
+            }
+
             List<ColumnDefinition> clusteringColumns = cfm.clusteringColumns();
             List<ColumnDefinition> restrictedColumns = new LinkedList<>(clusteringColumnsRestrictions.getColumnDefs());
 
@@ -325,19 +330,19 @@ public final class StatementRestrictions
 
                 if (!clusteringColumn.equals(restrictedColumn))
                 {
-                    checkTrue(hasQueriableIndex,
+                    throw invalidRequest(
                               "PRIMARY KEY column \"%s\" cannot be restricted as preceding
column \"%s\" is not restricted",
                               restrictedColumn.name,
                               clusteringColumn.name);
-
-                    usesSecondaryIndexing = true; // handle gaps and non-keyrange cases.
-                    break;
                 }
             }
         }
+    }
 
-        if (clusteringColumnsRestrictions.isContains())
-            usesSecondaryIndexing = true;
+    public final boolean clusteringRestrictionsNeedFiltering()
+    {
+        assert clusteringColumnsRestrictions instanceof PrimaryKeyRestrictionSet;
+        return ((PrimaryKeyRestrictionSet) clusteringColumnsRestrictions).needsFiltering();
     }
 
     public List<IndexExpression> getIndexExpressions(SecondaryIndexManager indexManager,

http://git-wip-us.apache.org/repos/asf/cassandra/blob/dff7b447/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java
b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java
index 08bf0db..f8e5a28 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java
@@ -347,6 +347,10 @@ public class SelectSingleColumnRelationTest extends CQLTester
 
         assertInvalidMessage("IN restrictions are not supported on indexed columns",
                              "SELECT v1 FROM %s WHERE id2 = 0 and time IN (1, 2) ALLOW FILTERING");
+
+        assertRows(execute("SELECT v1 FROM %s WHERE author > 'ted' AND time = 1 ALLOW
FILTERING"), row("E"));
+        assertRows(execute("SELECT v1 FROM %s WHERE author > 'amy' AND author < 'zoe'
AND time = 0 ALLOW FILTERING"),
+                           row("A"), row("D"));
     }
 
     @Test
@@ -605,4 +609,4 @@ public class SelectSingleColumnRelationTest extends CQLTester
         assertInvalidMessage("Aliases aren't allowed in the where clause ('d CONTAINS KEY
0')", "SELECT c AS d FROM %s WHERE d CONTAINS KEY 0");
         assertInvalidMessage("Undefined name d in selection clause", "SELECT d FROM %s WHERE
a = 0");
     }
-}
\ No newline at end of file
+}


Mime
View raw message