Repository: cassandra Updated Branches: refs/heads/trunk c23560347 -> a0a30e03a
Fix validation with multiple CONTAINS clauses patch by blerer; reviewed by slebresne for CASSANDRA-8131 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/18cd3e32 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/18cd3e32 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/18cd3e32 Branch: refs/heads/trunk Commit: 18cd3e3205ac3301ebce0b047aa444e50388083b Parents: ec866fa Author: Benjamin Lerer <[email protected]> Authored: Tue Oct 21 16:39:35 2014 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Tue Oct 21 16:40:38 2014 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../cql3/statements/SelectStatement.java | 39 +++++++++++++++++++- .../statements/SingleColumnRestriction.java | 10 +++++ .../cassandra/cql3/ContainsRelationTest.java | 22 +++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/18cd3e32/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 815bce1..09ab91b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.1.1 + * Fix validation with multiple CONTAINS clause (CASSANDRA-8131) * Fix validation of collections in TriggerExecutor (CASSANDRA-8146) * Fix IllegalArgumentException when a list of IN values containing tuples is passed as a single arg to a prepared statement with the v1 or v2 http://git-wip-us.apache.org/repos/asf/cassandra/blob/18cd3e32/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index a8c9d44..233f3db 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -31,6 +31,7 @@ import org.github.jamm.MemoryMeter; import org.apache.cassandra.auth.Permission; import org.apache.cassandra.cql3.*; +import org.apache.cassandra.cql3.statements.SingleColumnRestriction.Contains; import org.apache.cassandra.db.composites.*; import org.apache.cassandra.transport.messages.ResultMessage; import org.apache.cassandra.config.CFMetaData; @@ -1349,6 +1350,27 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache throw new InvalidRequestException(String.format("SELECT DISTINCT queries must request all the partition key columns (missing %s)", def.name)); } + /** + * Checks if the specified column is restricted by multiple contains or contains key. + * + * @param columnDef the definition of the column to check + * @return <code>true</code> the specified column is restricted by multiple contains or contains key, + * <code>false</code> otherwise + */ + private boolean isRestrictedByMultipleContains(ColumnDefinition columnDef) + { + if (!columnDef.type.isCollection()) + return false; + + Restriction restriction = metadataRestrictions.get(columnDef.name); + + if (!(restriction instanceof Contains)) + return false; + + Contains contains = (Contains) restriction; + return (contains.numberOfValues() + contains.numberOfKeys()) > 1; + } + public static class RawStatement extends CFStatement { private final Parameters parameters; @@ -2011,7 +2033,7 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache // We will potentially filter data if either: // - Have more than one IndexExpression // - Have no index expression and the column filter is not the identity - if (stmt.restrictedColumns.size() > 1 || (stmt.restrictedColumns.isEmpty() && !stmt.columnFilterIsIdentity())) + if (needFiltering(stmt)) throw new InvalidRequestException("Cannot execute this query as it might involve data filtering and " + "thus may have unpredictable performance. If you want to execute " + "this query despite the performance unpredictability, use ALLOW FILTERING"); @@ -2036,6 +2058,21 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache } } + /** + * Checks if the specified statement will need to filter the data. + * + * @param stmt the statement to test. + * @return <code>true</code> if the specified statement will need to filter the data, <code>false</code> + * otherwise. + */ + private static boolean needFiltering(SelectStatement stmt) + { + return stmt.restrictedColumns.size() > 1 + || (stmt.restrictedColumns.isEmpty() && !stmt.columnFilterIsIdentity()) + || (!stmt.restrictedColumns.isEmpty() + && stmt.isRestrictedByMultipleContains(Iterables.getOnlyElement(stmt.restrictedColumns))); + } + private int indexOf(ColumnDefinition def, Selection selection) { return indexOf(def, selection.getColumns().iterator()); http://git-wip-us.apache.org/repos/asf/cassandra/blob/18cd3e32/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java b/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java index bc77357..1ee0ebe 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java +++ b/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java @@ -334,6 +334,16 @@ public abstract class SingleColumnRestriction implements Restriction return keys != null; } + public int numberOfValues() + { + return values == null ? 0 : values.size(); + } + + public int numberOfKeys() + { + return keys == null ? 0 : keys.size(); + } + public void add(Term t, boolean isKey) { if (isKey) http://git-wip-us.apache.org/repos/asf/cassandra/blob/18cd3e32/test/unit/org/apache/cassandra/cql3/ContainsRelationTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/ContainsRelationTest.java b/test/unit/org/apache/cassandra/cql3/ContainsRelationTest.java index 605f3ed..f854ec6 100644 --- a/test/unit/org/apache/cassandra/cql3/ContainsRelationTest.java +++ b/test/unit/org/apache/cassandra/cql3/ContainsRelationTest.java @@ -42,6 +42,9 @@ public class ContainsRelationTest extends CQLTester assertRows(execute("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS ?", "test", 5, "lmn"), row("test", 5, set("lmn")) ); + + assertInvalid("SELECT * FROM %s WHERE account = ? AND categories CONTAINS ? AND categories CONTAINS ?", "xyz", "lmn", "notPresent"); + assertEmpty(execute("SELECT * FROM %s WHERE account = ? AND categories CONTAINS ? AND categories CONTAINS ? ALLOW FILTERING", "xyz", "lmn", "notPresent")); } @Test @@ -65,6 +68,11 @@ public class ContainsRelationTest extends CQLTester assertRows(execute("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS ?;", "test", 5, "lmn"), row("test", 5, list("lmn")) ); + + assertInvalid("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS ? AND categories CONTAINS ?", + "test", 5, "lmn", "notPresent"); + assertEmpty(execute("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS ? AND categories CONTAINS ? ALLOW FILTERING", + "test", 5, "lmn", "notPresent")); } @Test @@ -87,6 +95,14 @@ public class ContainsRelationTest extends CQLTester assertRows(execute("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS KEY ?", "test", 5, "lmn"), row("test", 5, map("lmn", "foo")) ); + + assertInvalid("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS KEY ? AND categories CONTAINS KEY ?", + "test", 5, "lmn", "notPresent"); + assertEmpty(execute("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS KEY ? AND categories CONTAINS KEY ? ALLOW FILTERING", + "test", 5, "lmn", "notPresent")); + + assertInvalid("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS KEY ? AND categories CONTAINS ?", + "test", 5, "lmn", "foo"); } @Test @@ -110,6 +126,12 @@ public class ContainsRelationTest extends CQLTester assertRows(execute("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS ?", "test", 5, "foo"), row("test", 5, map("lmn", "foo")) ); + + assertInvalid("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS ? AND categories CONTAINS ?" + , "test", 5, "foo", "notPresent"); + + assertEmpty(execute("SELECT * FROM %s WHERE account = ? AND id = ? AND categories CONTAINS ? AND categories CONTAINS ? ALLOW FILTERING" + , "test", 5, "foo", "notPresent")); } // See CASSANDRA-7525
