cql3:statement_restrictions.cc add more conditions to prevent "allow filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes #12474

Closes scylladb/scylladb#15811
This commit is contained in:
Kurashkin Nikita
2023-10-23 16:23:05 +03:00
committed by Nadav Har'El
parent 9c0f05efa1
commit c071cd92b5
6 changed files with 32 additions and 34 deletions

View File

@@ -399,7 +399,7 @@ statement_restrictions::statement_restrictions(data_dictionary::database db,
}
// At this point, the select statement if fully constructed, but we still have a few things to validate
process_partition_key_restrictions(for_view, allow_filtering);
process_partition_key_restrictions(for_view, allow_filtering, type);
// Some but not all of the partition key columns have been specified;
// hence we need turn these restrictions into index expressions.
@@ -444,7 +444,7 @@ statement_restrictions::statement_restrictions(data_dictionary::database db,
if (!expr::is_empty_restriction(_nonprimary_key_restrictions)) {
if (_has_queriable_regular_index && _partition_range_is_simple) {
_uses_secondary_indexing = true;
} else if (!allow_filtering) {
} else if (!allow_filtering && !type.is_delete() && !type.is_update()) {
throw exceptions::invalid_request_exception("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");
@@ -790,7 +790,7 @@ void statement_restrictions::add_single_column_nonprimary_key_restriction(const
_nonprimary_key_restrictions = expr::make_conjunction(_nonprimary_key_restrictions, restr);
}
void statement_restrictions::process_partition_key_restrictions(bool for_view, bool allow_filtering) {
void statement_restrictions::process_partition_key_restrictions(bool for_view, bool allow_filtering, statements::statement_type type) {
// If there is a queryable index, no special condition are required on the other restrictions.
// But we still need to know 2 things:
// - If we don't have a queryable index, is the query ok
@@ -805,7 +805,7 @@ void statement_restrictions::process_partition_key_restrictions(bool for_view, b
}
if (pk_restrictions_need_filtering()) {
if (!allow_filtering && !for_view && !_has_queriable_pk_index) {
if (!allow_filtering && !for_view && !_has_queriable_pk_index && !type.is_delete() && !type.is_update()) {
throw exceptions::invalid_request_exception("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");

View File

@@ -261,7 +261,7 @@ private:
void add_multi_column_clustering_key_restriction(const expr::binary_operator& restr);
void add_single_column_nonprimary_key_restriction(const expr::binary_operator& restr);
void process_partition_key_restrictions(bool for_view, bool allow_filtering);
void process_partition_key_restrictions(bool for_view, bool allow_filtering, statements::statement_type type);
/**
* Processes the clustering column restrictions.

View File

@@ -920,7 +920,7 @@ def testCreateIndexOnCompactTableWithoutClusteringColumns(cql, test_keyspace):
# DeleteTest
@pytest.mark.xfail(reason="issue #12474")
@pytest.mark.parametrize("forceFlush", [False, True])
def testDeleteWithNoClusteringColumns(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionkey int PRIMARY KEY, value int) WITH COMPACT STORAGE") as table:
@@ -972,7 +972,7 @@ def testDeleteWithNoClusteringColumns(cql, test_keyspace, forceFlush):
# Non primary key in the where clause
# Reproduces #12474:
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
assertInvalidMessage(cql, table, "where clause",
"DELETE FROM %s WHERE partitionKey = ? AND value = ?", 0, 1)

View File

@@ -299,7 +299,7 @@ def testDeletedRowsDoNotResurface(cql, test_keyspace):
assertRows(execute(cql, table, "SELECT * FROM %s WHERE a = ?", 1),
row(1, 3, "3"))
@pytest.mark.xfail(reason="#12474")
@pytest.mark.parametrize("forceFlush", [False, True])
def testDeleteWithNoClusteringColumns(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionKey int PRIMARY KEY, value int)") as table:
@@ -352,11 +352,11 @@ def testDeleteWithNoClusteringColumns(cql, test_keyspace, forceFlush):
# Non primary key in the where clause
# Reproduces #12474:
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
assertInvalidMessage(cql, table, "where clause",
"DELETE FROM %s WHERE partitionKey = ? AND value = ?", 0, 1)
@pytest.mark.xfail(reason="#12474")
@pytest.mark.parametrize("forceFlush", [False, True])
def testDeleteWithOneClusteringColumns(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionKey int, clustering int, value int, PRIMARY KEY (partitionKey, clustering))") as table:
@@ -432,10 +432,10 @@ def testDeleteWithOneClusteringColumns(cql, test_keyspace, forceFlush):
# Non primary key in the where clause
# Reproduces #12474:
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
assertInvalidMessage(cql, table, "where clause",
"DELETE FROM %s WHERE partitionKey = ? AND clustering = ? AND value = ?", 0, 1, 3)
@pytest.mark.xfail(reason="#4244, #12474")
@pytest.mark.xfail(reason="#4244")
@pytest.mark.parametrize("forceFlush", [False, True])
def testDeleteWithTwoClusteringColumns(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionKey int, clustering_1 int, clustering_2 int, value int, PRIMARY KEY (partitionKey, clustering_1, clustering_2))") as table:
@@ -532,7 +532,7 @@ def testDeleteWithTwoClusteringColumns(cql, test_keyspace, forceFlush):
# Non primary key in the where clause
# Reproduces #12474:
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
assertInvalidMessage(cql, table, "where clause",
"DELETE FROM %s WHERE partitionKey = ? AND clustering_1 = ? AND clustering_2 = ? AND value = ?", 0, 1, 1, 3)
def testDeleteWithNonoverlappingRange(cql, test_keyspace):
@@ -905,7 +905,7 @@ def testDeleteWithAStaticColumn(cql, test_keyspace, forceFlush):
"DELETE staticValue FROM %s WHERE partitionKey = ? AND (clustering_1, clustering_2) >= (?, ?)",
0, 0, 1)
@pytest.mark.xfail(reason="#12474")
@pytest.mark.parametrize("forceFlush", [False, True])
def testDeleteWithSecondaryIndices(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionKey int, clustering_1 int, value int, values set<int>, primary key (partitionKey, clustering_1))") as table:
@@ -923,22 +923,23 @@ def testDeleteWithSecondaryIndices(cql, test_keyspace, forceFlush):
flush(cql, table)
# Reproduces #12474:
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
assertInvalidMessage(cql, table, "where clause",
"DELETE FROM %s WHERE partitionKey = ? AND clustering_1 = ? AND value = ?", 3, 3, 3)
assertInvalidMessage(cql, table, "Cannot use DELETE with CONTAINS",
# Different error message returned, changed to assertInvalid instead of assertInvalidMessage
assertInvalid(cql, table,
"DELETE FROM %s WHERE partitionKey = ? AND clustering_1 = ? AND values CONTAINS ?", 3, 3, 3)
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
assertInvalidMessage(cql, table, "where clause",
"DELETE FROM %s WHERE partitionKey = ? AND value = ?", 3, 3)
assertInvalidMessage(cql, table, "Cannot use DELETE with CONTAINS",
# Different error message returned, changed to assertInvalid instead of assertInvalidMessage
assertInvalid(cql, table,
"DELETE FROM %s WHERE partitionKey = ? AND values CONTAINS ?", 3, 3)
assertInvalidMessage(cql, table, "partitionkey",
# Different error message returned, changed to assertInvalid instead of assertInvalidMessage
assertInvalid(cql, table,
"DELETE FROM %s WHERE clustering_1 = ?", 3)
# Reproduces #12474:
assertInvalidMessage(cql, table, "partitionkey",
# Different error message returned, changed to assertInvalid instead of assertInvalidMessage
assertInvalid(cql, table,
"DELETE FROM %s WHERE value = ?", 3)
# Different error messages in Scylla and Cassandra
assertInvalid(cql, table,
"DELETE FROM %s WHERE values CONTAINS ?", 3)

View File

@@ -31,7 +31,6 @@ def testTypeCasts(cql, test_keyspace):
assertInvalid(cql, table, "UPDATE %s SET d = (int)3 WHERE k = ?", 0)
assertInvalid(cql, table, "UPDATE %s SET i = (double)3 WHERE k = ?", 0)
@pytest.mark.xfail(reason="#12474")
@pytest.mark.parametrize("forceFlush", [False, True])
def testUpdate(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionKey int, clustering_1 int, value int, PRIMARY KEY (partitionKey, clustering_1))") as table:
@@ -145,8 +144,8 @@ def testUpdate(cql, test_keyspace, forceFlush):
assertInvalid(cql, table,
"UPDATE %s SET value = ? WHERE partitionKey CONTAINS ? AND clustering_1 = ?", 7, 0, 1)
# Reproduces #12474
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
# Reproduces #12474:
assertInvalidMessage(cql, table, "where clause",
"UPDATE %s SET value = ? WHERE partitionKey = ? AND clustering_1 = ? AND value = ?", 7, 0, 1, 3)
# Scylla and Cassandra print different error messages in this case:
@@ -210,7 +209,6 @@ def testUpdateWithSecondaryIndices(cql, test_keyspace, forceFlush):
assertInvalid(cql, table,
"UPDATE %s SET value= ? WHERE values CONTAINS ?", 6, 3)
@pytest.mark.xfail(reason="#12474")
@pytest.mark.parametrize("forceFlush", [False, True])
def testUpdateWithTwoClusteringColumns(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionKey int, clustering_1 int, clustering_2 int, value int, PRIMARY KEY (partitionKey, clustering_1, clustering_2))") as table:
@@ -332,9 +330,8 @@ def testUpdateWithTwoClusteringColumns(cql, test_keyspace, forceFlush):
assertInvalid(cql, table,
"UPDATE %s SET value = ? WHERE partitionKey CONTAINS ? AND clustering_1 = ? AND clustering_2 = ?", 7, 0, 1, 1)
# Reproduces #12474
assertInvalidMessage(cql, table, "Non PRIMARY KEY columns found in where clause: value",
# Reproduces #12474:
assertInvalidMessage(cql, table, "where clause",
"UPDATE %s SET value = ? WHERE partitionKey = ? AND clustering_1 = ? AND clustering_2 = ? AND value = ?", 7, 0, 1, 1, 3)
assertInvalid(cql, table,
@@ -343,7 +340,6 @@ def testUpdateWithTwoClusteringColumns(cql, test_keyspace, forceFlush):
assertInvalid(cql, table,
"UPDATE %s SET value = ? WHERE partitionKey = ? AND (clustering_1, clustering_2) > (?, ?)", 7, 0, 1, 1)
@pytest.mark.xfail(reason="#12474")
@pytest.mark.parametrize("forceFlush", [False, True])
def testUpdateWithMultiplePartitionKeyComponents(cql, test_keyspace, forceFlush):
with create_table(cql, test_keyspace, "(partitionKey_1 int, partitionKey_2 int, clustering_1 int, clustering_2 int, value int, PRIMARY KEY ((partitionKey_1, partitionKey_2), clustering_1, clustering_2))") as table:
@@ -382,8 +378,9 @@ def testUpdateWithMultiplePartitionKeyComponents(cql, test_keyspace, forceFlush)
row(1, 1, 0, 1, 10))
# missing primary key element
# Reproduces #12474
assertInvalidMessage(cql, table, "Some partition key parts are missing: partitionkey_2",
# Reproduces #12474:
# Different error message returned, changed to assertInvalid instead of assertInvalidMessage
assertInvalid(cql, table,
"UPDATE %s SET value = ? WHERE partitionKey_1 = ? AND clustering_1 = ? AND clustering_2 = ?", 7, 1, 1)
@pytest.mark.parametrize("forceFlush", [False, True])

View File

@@ -54,7 +54,7 @@ OK
+-----+-----+
> -- update if exists with a scan: proper error message
> update lwt set b = 2 where b = 1 if exists;
Error from server: code=2200 [Invalid query] message="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"
Error from server: code=2200 [Invalid query] message="Invalid where clause contains non PRIMARY KEY columns: b"
> select * from lwt allow filtering;
+-----+-----+
| a | b |