From 69edeaa46b78bb168f7e9d0b1c991c07b90f41ca Mon Sep 17 00:00:00 2001 From: Alex Petrov Date: Thu, 14 Apr 2016 12:26:52 +0200 Subject: [PATCH 1/2] Allow only DISTINCT queries with partition keys restrictions patch by Alex Petrov; reviewed by Benjamin Lerer for CASSANDRA-11339 --- CHANGES.txt | 1 + .../restrictions/StatementRestrictions.java | 9 ++++ .../cql3/statements/SelectStatement.java | 3 ++ .../validation/operations/SelectTest.java | 45 +++++++++++++++++++ 4 files changed, 58 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 54013a3dda..c72b6cbd7a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.2.6 + * Allow only DISTINCT queries with partition keys restrictions (CASSANDRA-11339) * CqlConfigHelper no longer requires both a keystore and truststore to work (CASSANDRA-11532) * Make deprecated repair methods backward-compatible with previous notification service (CASSANDRA-11430) * IncomingStreamingConnection version check message wrong (CASSANDRA-11462) diff --git a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java index e0cf7432f8..3934f33597 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java @@ -278,6 +278,15 @@ public final class StatementRestrictions return !partitionKeyRestrictions.isEmpty(); } + /** + * Checks if the restrictions contain any non-primary key restrictions + * @return true if the restrictions contain any non-primary key restrictions, false otherwise. + */ + public boolean hasNonPrimaryKeyRestrictions() + { + return !nonPrimaryKeyRestrictions.isEmpty(); + } + /** * Returns the partition key components that are not restricted. * @return the partition key components that are not restricted. diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 291e3e459c..7bba330225 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -885,6 +885,9 @@ public class SelectStatement implements CQLStatement StatementRestrictions restrictions) throws InvalidRequestException { + checkFalse(restrictions.hasClusteringColumnsRestriction() || restrictions.hasNonPrimaryKeyRestrictions(), + "SELECT DISTINCT with WHERE clause only supports restriction by partition key."); + Collection requestedColumns = selection.getColumns(); for (ColumnDefinition def : requestedColumns) checkFalse(!def.isPartitionKey() && !def.isStatic(), diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java index d8cd3c3309..d444fdebd2 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java @@ -1253,6 +1253,51 @@ public class SelectTest extends CQLTester Assert.assertEquals(9, rows.length); } + @Test + public void testSelectDistinctWithWhereClause() throws Throwable { + createTable("CREATE TABLE %s (k int, a int, b int, PRIMARY KEY (k, a))"); + createIndex("CREATE INDEX ON %s (b)"); + + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (k, a, b) VALUES (?, ?, ?)", i, i, i); + execute("INSERT INTO %s (k, a, b) VALUES (?, ?, ?)", i, i * 10, i * 10); + } + + String distinctQueryErrorMsg = "SELECT DISTINCT with WHERE clause only supports restriction by partition key."; + assertInvalidMessage(distinctQueryErrorMsg, + "SELECT DISTINCT k FROM %s WHERE a >= 80 ALLOW FILTERING"); + + assertInvalidMessage(distinctQueryErrorMsg, + "SELECT DISTINCT k FROM %s WHERE k IN (1, 2, 3) AND a = 10"); + + assertInvalidMessage(distinctQueryErrorMsg, + "SELECT DISTINCT k FROM %s WHERE b = 5"); + + assertRows(execute("SELECT DISTINCT k FROM %s WHERE k = 1"), + row(1)); + assertRows(execute("SELECT DISTINCT k FROM %s WHERE k IN (5, 6, 7)"), + row(5), + row(6), + row(7)); + + // With static columns + createTable("CREATE TABLE %s (k int, a int, s int static, b int, PRIMARY KEY (k, a))"); + createIndex("CREATE INDEX ON %s (b)"); + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (k, a, b, s) VALUES (?, ?, ?, ?)", i, i, i, i); + execute("INSERT INTO %s (k, a, b, s) VALUES (?, ?, ?, ?)", i, i * 10, i * 10, i * 10); + } + + assertRows(execute("SELECT DISTINCT s FROM %s WHERE k = 5"), + row(50)); + assertRows(execute("SELECT DISTINCT s FROM %s WHERE k IN (5, 6, 7)"), + row(50), + row(60), + row(70)); + } + /** * Migrated from cql_tests.py:TestCQL.bug_6327_test() */ From 6ad874509d6c7edd53bb3a4b897477d6a2753c19 Mon Sep 17 00:00:00 2001 From: Alex Petrov Date: Thu, 14 Apr 2016 12:35:07 +0200 Subject: [PATCH 2/2] Allow only DISTINCT queries with partition keys or static columns restrictions patch by Alex Petrov; reviewed by Benjamin Lerer for CASSANDRA-11339 --- CHANGES.txt | 1 + .../restrictions/StatementRestrictions.java | 9 +++ .../cql3/statements/SelectStatement.java | 4 ++ .../validation/operations/SelectTest.java | 72 +++++++++++++++++++ 4 files changed, 86 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index ed4c412763..3b4d4732bc 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.6 + * Allow only DISTINCT queries with partition keys or static columns restrictions (CASSANDRA-11339) * LogAwareFileLister should only use OLD sstable files in current folder to determine disk consistency (CASSANDRA-11470) * Notify indexers of expired rows during compaction (CASSANDRA-11329) * Properly respond with ProtocolError when a v1/v2 native protocol diff --git a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java index 797b8e424b..763a7bee42 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java @@ -395,6 +395,15 @@ public final class StatementRestrictions return !partitionKeyRestrictions.isEmpty(); } + /** + * Checks if the restrictions contain any non-primary key restrictions + * @return true if the restrictions contain any non-primary key restrictions, false otherwise. + */ + public boolean hasNonPrimaryKeyRestrictions() + { + return !nonPrimaryKeyRestrictions.isEmpty(); + } + /** * Returns the partition key components that are not restricted. * @return the partition key components that are not restricted. diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 51d675b79f..b4215ac0b1 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -896,6 +896,10 @@ public class SelectStatement implements CQLStatement StatementRestrictions restrictions) throws InvalidRequestException { + checkFalse(restrictions.hasClusteringColumnsRestriction() || + (restrictions.hasNonPrimaryKeyRestrictions() && !restrictions.nonPKRestrictedColumns(true).stream().allMatch(ColumnDefinition::isStatic)), + "SELECT DISTINCT with WHERE clause only supports restriction by partition key and/or static columns."); + Collection requestedColumns = selection.getColumns(); for (ColumnDefinition def : requestedColumns) checkFalse(!def.isPartitionKey() && !def.isStatic(), diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java index a7eeeb8034..5c19e1beb7 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java @@ -1253,6 +1253,78 @@ public class SelectTest extends CQLTester Assert.assertEquals(9, rows.length); } + @Test + public void testSelectDistinctWithWhereClause() throws Throwable { + createTable("CREATE TABLE %s (k int, a int, b int, PRIMARY KEY (k, a))"); + createIndex("CREATE INDEX ON %s (b)"); + + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (k, a, b) VALUES (?, ?, ?)", i, i, i); + execute("INSERT INTO %s (k, a, b) VALUES (?, ?, ?)", i, i * 10, i * 10); + } + + String distinctQueryErrorMsg = "SELECT DISTINCT with WHERE clause only supports restriction by partition key and/or static columns."; + assertInvalidMessage(distinctQueryErrorMsg, + "SELECT DISTINCT k FROM %s WHERE a >= 80 ALLOW FILTERING"); + + assertInvalidMessage(distinctQueryErrorMsg, + "SELECT DISTINCT k FROM %s WHERE k IN (1, 2, 3) AND a = 10"); + + assertInvalidMessage(distinctQueryErrorMsg, + "SELECT DISTINCT k FROM %s WHERE b = 5"); + + assertRows(execute("SELECT DISTINCT k FROM %s WHERE k = 1"), + row(1)); + assertRows(execute("SELECT DISTINCT k FROM %s WHERE k IN (5, 6, 7)"), + row(5), + row(6), + row(7)); + + // With static columns + createTable("CREATE TABLE %s (k int, a int, s int static, b int, PRIMARY KEY (k, a))"); + createIndex("CREATE INDEX ON %s (b)"); + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (k, a, b, s) VALUES (?, ?, ?, ?)", i, i, i, i); + execute("INSERT INTO %s (k, a, b, s) VALUES (?, ?, ?, ?)", i, i * 10, i * 10, i * 10); + } + + assertRows(execute("SELECT DISTINCT s FROM %s WHERE k = 5"), + row(50)); + assertRows(execute("SELECT DISTINCT s FROM %s WHERE k IN (5, 6, 7)"), + row(50), + row(60), + row(70)); + } + + @Test + public void testSelectDistinctWithWhereClauseOnStaticColumn() throws Throwable + { + createTable("CREATE TABLE %s (k int, a int, s int static, s1 int static, b int, PRIMARY KEY (k, a))"); + + for (int i = 0; i < 10; i++) + { + execute("INSERT INTO %s (k, a, b, s, s1) VALUES (?, ?, ?, ?, ?)", i, i, i, i, i); + execute("INSERT INTO %s (k, a, b, s, s1) VALUES (?, ?, ?, ?, ?)", i, i * 10, i * 10, i * 10, i * 10); + } + + execute("INSERT INTO %s (k, a, b, s, s1) VALUES (?, ?, ?, ?, ?)", 2, 10, 10, 10, 10); + + assertRows(execute("SELECT DISTINCT k, s, s1 FROM %s WHERE s = 90 AND s1 = 90 ALLOW FILTERING"), + row(9, 90, 90)); + + assertRows(execute("SELECT DISTINCT k, s, s1 FROM %s WHERE s = 90 AND s1 = 90 ALLOW FILTERING"), + row(9, 90, 90)); + + assertRows(execute("SELECT DISTINCT k, s, s1 FROM %s WHERE s = 10 AND s1 = 10 ALLOW FILTERING"), + row(1, 10, 10), + row(2, 10, 10)); + + assertRows(execute("SELECT DISTINCT k, s, s1 FROM %s WHERE k = 1 AND s = 10 AND s1 = 10 ALLOW FILTERING"), + row(1, 10, 10)); + } + /** * Migrated from cql_tests.py:TestCQL.bug_6327_test() */