cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ble...@apache.org
Subject [2/2] cassandra git commit: Allow filtering on clustering columns for queries without secondary indexes
Date Fri, 15 Apr 2016 20:27:46 GMT
Allow filtering on clustering columns for queries without secondary indexes

patch by Alex Petrov; reviewed by Benjamin Lerer for CASSANDRA-11310


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

Branch: refs/heads/trunk
Commit: a600920cb5ee2866b09ee6c1ebae9518096e5bc4
Parents: 831bebd
Author: Alex Petrov <oleksandr.petrov@gmail.com>
Authored: Fri Apr 15 22:26:02 2016 +0200
Committer: Benjamin Lerer <b.lerer@gmail.com>
Committed: Fri Apr 15 22:26:02 2016 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |    1 +
 .../ClusteringColumnRestrictions.java           |   80 +-
 .../restrictions/StatementRestrictions.java     |   58 +-
 .../entities/FrozenCollectionsTest.java         |    9 +-
 .../cql3/validation/operations/SelectTest.java  | 1067 ++++++++++++------
 5 files changed, 800 insertions(+), 415 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index acbbfa5..7bb97e5 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.6
+ * Allow filtering on clustering columns for queries without secondary indexes (CASSANDRA-11310)
  * Refactor Restriction hierarchy (CASSANDRA-11354)
  * Eliminate allocations in R/W path (CASSANDRA-11421)
  * Update Netty to 4.0.36 (CASSANDRA-11567)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java
b/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java
index 47cf76b..ab16ebc 100644
--- a/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java
+++ b/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java
@@ -42,16 +42,28 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper
      */
     protected final ClusteringComparator comparator;
 
+    /**
+     * <code>true</code> if filtering is allowed for this restriction, <code>false</code>
otherwise
+     */
+    private final boolean allowFiltering;
+
     public ClusteringColumnRestrictions(CFMetaData cfm)
     {
-        super(new RestrictionSet());
-        this.comparator = cfm.comparator;
+        this(cfm, false);
     }
 
-    private ClusteringColumnRestrictions(ClusteringComparator comparator, RestrictionSet
restrictionSet)
+    public ClusteringColumnRestrictions(CFMetaData cfm, boolean allowFiltering)
+    {
+        this(cfm.comparator, new RestrictionSet(), allowFiltering);
+    }
+
+    private ClusteringColumnRestrictions(ClusteringComparator comparator,
+                                         RestrictionSet restrictionSet,
+                                         boolean allowFiltering)
     {
         super(restrictionSet);
         this.comparator = comparator;
+        this.allowFiltering = allowFiltering;
     }
 
     public ClusteringColumnRestrictions mergeWith(Restriction restriction) throws InvalidRequestException
@@ -59,7 +71,7 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper
         SingleRestriction newRestriction = (SingleRestriction) restriction;
         RestrictionSet newRestrictionSet = restrictions.addRestriction(newRestriction);
 
-        if (!isEmpty())
+        if (!isEmpty() && !allowFiltering)
         {
             SingleRestriction lastRestriction = restrictions.lastRestriction();
             ColumnDefinition lastRestrictionStart = lastRestriction.getFirstColumn();
@@ -76,7 +88,7 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper
                                      newRestrictionStart.name);
         }
 
-        return new ClusteringColumnRestrictions(this.comparator, newRestrictionSet);
+        return new ClusteringColumnRestrictions(this.comparator, newRestrictionSet, allowFiltering);
     }
 
     private boolean hasMultiColumnSlice()
@@ -105,11 +117,10 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper
     {
         MultiCBuilder builder = MultiCBuilder.create(comparator, hasIN() || hasMultiColumnSlice());
         int keyPosition = 0;
+
         for (SingleRestriction r : restrictions)
         {
-            ColumnDefinition def = r.getFirstColumn();
-
-            if (keyPosition != def.position() || r.isContains() || r.isLIKE())
+            if (handleInFilter(r, keyPosition))
                 break;
 
             if (r.isSlice())
@@ -155,6 +166,32 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper
         return restrictions.stream().anyMatch(SingleRestriction::isSlice);
     }
 
+    /**
+     * Checks if underlying restrictions would require filtering
+     *
+     * @return <code>true</code> if any underlying restrictions require filtering,
<code>false</code>
+     * otherwise
+     */
+    public final boolean needFiltering()
+    {
+        int position = 0;
+        SingleRestriction slice = null;
+        for (SingleRestriction 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 hasContains();
+    }
+
     @Override
     public void addRowFilterTo(RowFilter filter,
                                SecondaryIndexManager indexManager,
@@ -162,18 +199,31 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper
     {
         int position = 0;
 
+        SingleRestriction slice = null;
         for (SingleRestriction restriction : restrictions)
         {
-            ColumnDefinition columnDef = restriction.getFirstColumn();
-
             // We ignore all the clustering columns that can be handled by slices.
-            if (!(restriction.isContains() || restriction.isLIKE()) && position ==
columnDef.position())
+            if (handleInFilter(restriction, position) || restriction.hasSupportingIndex(indexManager))
             {
-                position = restriction.getLastColumn().position() + 1;
-                if (!restriction.hasSupportingIndex(indexManager))
-                    continue;
+                restriction.addRowFilterTo(filter, indexManager, options);
+                continue;
             }
-            restriction.addRowFilterTo(filter, indexManager, options);
+
+            if (slice != null && !slice.getFirstColumn().equals(restriction.getFirstColumn()))
+            {
+                restriction.addRowFilterTo(filter, indexManager, options);
+                continue;
+            }
+
+            if (slice == null && restriction.isSlice())
+                slice = restriction;
+            else
+                position = restriction.getLastColumn().position() + 1;
         }
     }
+
+    private boolean handleInFilter(SingleRestriction restriction, int index) {
+        return restriction.isContains() || restriction.isLIKE() || index != restriction.getFirstColumn().position();
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/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 032a622..b146cfd 100644
--- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java
+++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java
@@ -85,7 +85,7 @@ public final class StatementRestrictions
     /**
      * The restrictions used to build the row filter
      */
-    private final IndexRestrictions indexRestrictions = new IndexRestrictions();
+    private final IndexRestrictions filterRestrictions = new IndexRestrictions();
 
     /**
      * <code>true</code> if the secondary index need to be queried, <code>false</code>
otherwise
@@ -106,15 +106,15 @@ public final class StatementRestrictions
      */
     public static StatementRestrictions empty(StatementType type, CFMetaData cfm)
     {
-        return new StatementRestrictions(type, cfm);
+        return new StatementRestrictions(type, cfm, false);
     }
 
-    private StatementRestrictions(StatementType type, CFMetaData cfm)
+    private StatementRestrictions(StatementType type, CFMetaData cfm, boolean allowFiltering)
     {
         this.type = type;
         this.cfm = cfm;
         this.partitionKeyRestrictions = new PartitionKeySingleRestrictionSet(cfm.getKeyValidatorAsClusteringComparator());
-        this.clusteringColumnsRestrictions = new ClusteringColumnRestrictions(cfm);
+        this.clusteringColumnsRestrictions = new ClusteringColumnRestrictions(cfm, allowFiltering);
         this.nonPrimaryKeyRestrictions = new RestrictionSet();
         this.notNullColumns = new HashSet<>();
     }
@@ -125,10 +125,10 @@ public final class StatementRestrictions
                                  VariableSpecifications boundNames,
                                  boolean selectsOnlyStaticColumns,
                                  boolean selectsComplexColumn,
-                                 boolean useFiltering,
-                                 boolean forView) throws InvalidRequestException
+                                 boolean allowFiltering,
+                                 boolean forView)
     {
-        this(type, cfm);
+        this(type, cfm, allowFiltering);
 
 
         ColumnFamilyStore cfs;
@@ -185,7 +185,7 @@ public final class StatementRestrictions
                 processCustomIndexExpressions(whereClause.expressions, boundNames, secondaryIndexManager);
 
             hasQueriableClusteringColumnIndex = clusteringColumnsRestrictions.hasSupportingIndex(secondaryIndexManager);
-            hasQueriableIndex = !indexRestrictions.getCustomIndexExpressions().isEmpty()
+            hasQueriableIndex = !filterRestrictions.getCustomIndexExpressions().isEmpty()
                     || hasQueriableClusteringColumnIndex
                     || partitionKeyRestrictions.hasSupportingIndex(secondaryIndexManager)
                     || nonPrimaryKeyRestrictions.hasSupportingIndex(secondaryIndexManager);
@@ -197,7 +197,7 @@ public final class StatementRestrictions
         // Some but not all of the partition key columns have been specified;
         // hence we need turn these restrictions into a row filter.
         if (usesSecondaryIndexing)
-            indexRestrictions.add(partitionKeyRestrictions);
+            filterRestrictions.add(partitionKeyRestrictions);
 
         if (selectsOnlyStaticColumns && hasClusteringColumnsRestriction())
         {
@@ -218,16 +218,18 @@ public final class StatementRestrictions
                 throw invalidRequest("Cannot restrict clustering columns when selecting only
static columns");
         }
 
-        processClusteringColumnsRestrictions(hasQueriableIndex, selectsOnlyStaticColumns,
selectsComplexColumn, forView);
+        processClusteringColumnsRestrictions(hasQueriableIndex,
+                                             selectsOnlyStaticColumns,
+                                             selectsComplexColumn,
+                                             forView,
+                                             allowFiltering);
 
         // Covers indexes on the first clustering column (among others).
         if (isKeyRange && hasQueriableClusteringColumnIndex)
             usesSecondaryIndexing = true;
 
-        usesSecondaryIndexing = usesSecondaryIndexing || clusteringColumnsRestrictions.hasContains();
-
-        if (usesSecondaryIndexing)
-            indexRestrictions.add(clusteringColumnsRestrictions);
+        if (usesSecondaryIndexing || clusteringColumnsRestrictions.needFiltering())
+            filterRestrictions.add(clusteringColumnsRestrictions);
 
         // Even if usesSecondaryIndexing is false at this point, we'll still have to use
one if
         // there is restrictions not covered by the PK.
@@ -243,10 +245,10 @@ public final class StatementRestrictions
             }
             if (hasQueriableIndex)
                 usesSecondaryIndexing = true;
-            else if (!useFiltering)
+            else if (!allowFiltering)
                 throw invalidRequest(StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE);
 
-            indexRestrictions.add(nonPrimaryKeyRestrictions);
+            filterRestrictions.add(nonPrimaryKeyRestrictions);
         }
 
         if (usesSecondaryIndexing)
@@ -274,7 +276,7 @@ public final class StatementRestrictions
     // may be used by QueryHandler implementations
     public IndexRestrictions getIndexRestrictions()
     {
-        return indexRestrictions;
+        return filterRestrictions;
     }
 
     /**
@@ -285,7 +287,7 @@ public final class StatementRestrictions
     public Set<ColumnDefinition> nonPKRestrictedColumns(boolean includeNotNullRestrictions)
     {
         Set<ColumnDefinition> columns = new HashSet<>();
-        for (Restrictions r : indexRestrictions.getRestrictions())
+        for (Restrictions r : filterRestrictions.getRestrictions())
         {
             for (ColumnDefinition def : r.getColumnDefs())
                 if (!def.isPrimaryKeyColumn())
@@ -451,7 +453,8 @@ public final class StatementRestrictions
     private void processClusteringColumnsRestrictions(boolean hasQueriableIndex,
                                                       boolean selectsOnlyStaticColumns,
                                                       boolean selectsComplexColumn,
-                                                      boolean forView) throws InvalidRequestException
+                                                      boolean forView,
+                                                      boolean allowFiltering)
     {
         checkFalse(!type.allowClusteringColumnSlices() && clusteringColumnsRestrictions.hasSlice(),
                    "Slice restrictions are not supported on the clustering columns in %s
statements", type);
@@ -467,9 +470,10 @@ public final class StatementRestrictions
         {
             checkFalse(clusteringColumnsRestrictions.hasIN() && selectsComplexColumn,
                        "Cannot restrict clustering columns by IN relations when a collection
is selected by the query");
-            checkFalse(clusteringColumnsRestrictions.hasContains() && !hasQueriableIndex,
+            checkFalse(clusteringColumnsRestrictions.hasContains() && !hasQueriableIndex
&& !allowFiltering,
                        "Cannot restrict clustering columns by a CONTAINS relation without
a secondary index");
 
+
             if (hasClusteringColumnsRestriction())
             {
                 List<ColumnDefinition> clusteringColumns = cfm.clusteringColumns();
@@ -480,7 +484,7 @@ public final class StatementRestrictions
                     ColumnDefinition clusteringColumn = clusteringColumns.get(i);
                     ColumnDefinition restrictedColumn = restrictedColumns.get(i);
 
-                    if (!clusteringColumn.equals(restrictedColumn))
+                    if (!clusteringColumn.equals(restrictedColumn) && !allowFiltering)
                     {
                         checkTrue(hasQueriableIndex || forView,
                                   "PRIMARY KEY column \"%s\" cannot be restricted as preceding
column \"%s\" is not restricted",
@@ -552,19 +556,19 @@ public final class StatementRestrictions
 
         expression.prepareValue(cfm, expressionType, boundNames);
 
-        indexRestrictions.add(expression);
+        filterRestrictions.add(expression);
     }
 
     public RowFilter getRowFilter(SecondaryIndexManager indexManager, QueryOptions options)
     {
-        if (indexRestrictions.isEmpty())
+        if (filterRestrictions.isEmpty())
             return RowFilter.NONE;
 
         RowFilter filter = RowFilter.create();
-        for (Restrictions restrictions : indexRestrictions.getRestrictions())
+        for (Restrictions restrictions : filterRestrictions.getRestrictions())
             restrictions.addRowFilterTo(filter, indexManager, options);
 
-        for (CustomIndexExpression expression : indexRestrictions.getCustomIndexExpressions())
+        for (CustomIndexExpression expression : filterRestrictions.getCustomIndexExpressions())
             expression.addToRowFilter(filter, cfm, options);
 
         return filter;
@@ -743,8 +747,8 @@ public final class StatementRestrictions
      */
     public boolean needFiltering()
     {
-        int numberOfRestrictions = indexRestrictions.getCustomIndexExpressions().size();
-        for (Restrictions restrictions : indexRestrictions.getRestrictions())
+        int numberOfRestrictions = filterRestrictions.getCustomIndexExpressions().size();
+        for (Restrictions restrictions : filterRestrictions.getRestrictions())
             numberOfRestrictions += restrictions.size();
 
         return numberOfRestrictions > 1

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java
b/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java
index 9df8ea0..4aa178a 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java
@@ -633,14 +633,15 @@ public class FrozenCollectionsTest extends CQLTester
         assertInvalidMessage("Cannot restrict clustering columns by a CONTAINS relation without
a secondary index",
                              "SELECT * FROM %s WHERE b CONTAINS ?", 1);
 
-        assertInvalidMessage("Cannot restrict clustering columns by a CONTAINS relation without
a secondary index",
-                             "SELECT * FROM %s WHERE b CONTAINS ? ALLOW FILTERING", 1);
+        assertRows(execute("SELECT * FROM %s WHERE b CONTAINS ? ALLOW FILTERING", 1),
+                   row(0, list(1, 2, 3), set(1, 2, 3), map(1, "a")),
+                   row(1, list(1, 2, 3), set(4, 5, 6), map(2, "b")));
 
         assertInvalidMessage(StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE,
                              "SELECT * FROM %s WHERE d CONTAINS KEY ?", 1);
 
-        assertInvalidMessage("Cannot restrict clustering columns by a CONTAINS relation without
a secondary index",
-                             "SELECT * FROM %s WHERE b CONTAINS ? AND d CONTAINS KEY ? ALLOW
FILTERING", 1, 1);
+        assertRows(execute("SELECT * FROM %s WHERE b CONTAINS ? AND d CONTAINS KEY ? ALLOW
FILTERING", 1, 1),
+                   row(0, list(1, 2, 3), set(1, 2, 3), map(1, "a")));
 
         // index lookup on b
         assertRows(execute("SELECT * FROM %s WHERE b=?", list(1, 2, 3)),


Mime
View raw message