mirror of
https://github.com/scylladb/scylladb.git
synced 2026-05-12 19:02:12 +00:00
cql3: Reject updates with NULL key values
We were silently ignoring INSERTs with NULL values for primary-key
columns, which Cassandra rejects. Fix it by rejecting any
modification_statement that would operate on empty partition or
clustering range.
This is the most direct fix, because range and slice are calculated in
one place for all modification statements. It covers not only NULL
cases, but also impossible restrictions like c>0 AND c<0.
Unfortunately, Cassandra doesn't treat all modification statements
consistently, so this fix cannot fully match its behavior. We err on
the side of tolerance, accepting some DELETE statements that Cassandra
rejects. We add a TODO for rejecting such DELETEs later.
Fixes #7852.
Tests: unit (dev), cql-pytest against Cassandra 4.0
Signed-off-by: Dejan Mircevski <dejan@scylladb.com>
Closes #9286
(cherry picked from commit 1fdaeca7d0)
This commit is contained in:
committed by
Avi Kivity
parent
e1c7a906f0
commit
146f7b5421
@@ -139,7 +139,13 @@ modification_statement::get_mutations(service::storage_proxy& proxy, const query
|
||||
auto cl = options.get_consistency();
|
||||
auto json_cache = maybe_prepare_json_cache(options);
|
||||
auto keys = build_partition_keys(options, json_cache);
|
||||
if (keys.empty()) {
|
||||
throw exceptions::invalid_request_exception("Invalid partition key in a modification statement");
|
||||
}
|
||||
auto ranges = create_clustering_ranges(options, json_cache);
|
||||
if (ranges.empty() && !type.is_delete() /* DELETE is allowed on empty ranges (TODO: but not on WHERE ck=NULL).*/) {
|
||||
throw exceptions::invalid_request_exception("Invalid clustering key in a modification statement");
|
||||
}
|
||||
auto f = make_ready_future<update_parameters::prefetch_data>(s);
|
||||
|
||||
if (is_counter()) {
|
||||
|
||||
@@ -86,15 +86,12 @@ def do_test_partition_key_usage(cql, test_keyspace, typ, v1, v2, v3, v4):
|
||||
[v2, 0],
|
||||
[v4, 0])
|
||||
|
||||
@pytest.mark.xfail(reason="fails just because of null key, issue #7852")
|
||||
def test_partition_key_usage_set(cql, test_keyspace):
|
||||
do_test_partition_key_usage(cql, test_keyspace, "set<int>", set(), {1, 2, 3}, {4, 5, 6}, {7, 8, 9})
|
||||
|
||||
@pytest.mark.xfail(reason="fails just because of null key, issue #7852")
|
||||
def test_partition_key_usage_list(cql, test_keyspace):
|
||||
do_test_partition_key_usage(cql, test_keyspace, "list<int>", [], [1, 2, 3], [4, 5, 6], [7, 8, 9])
|
||||
|
||||
@pytest.mark.xfail(reason="fails just because of null key, issue #7852")
|
||||
def test_partition_key_usage_map(cql, test_keyspace):
|
||||
do_test_partition_key_usage(cql, test_keyspace, "map<int, int>", {}, {1: 10, 2: 20, 3: 30}, {4: 40, 5: 50, 6: 60}, {7: 70, 8: 80, 9: 90})
|
||||
|
||||
|
||||
@@ -48,20 +48,19 @@ def test_insert_missing_key(cql, table1):
|
||||
|
||||
# A null key, like a missing one, is also not allowed.
|
||||
# This reproduces issue #7852.
|
||||
@pytest.mark.xfail(reason="issue #7852")
|
||||
def test_insert_null_key(cql, table1):
|
||||
s = random_string()
|
||||
with pytest.raises(InvalidRequest, match='null value'):
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|clustering)')):
|
||||
cql.execute(f"INSERT INTO {table1} (p,c) VALUES ('{s}', null)")
|
||||
with pytest.raises(InvalidRequest, match='null value'):
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|partition)')):
|
||||
cql.execute(f"INSERT INTO {table1} (p,c) VALUES (null, '{s}')")
|
||||
# Try the same thing with prepared statement, where a "None" stands for
|
||||
# a null. Note that this is completely different from UNSET_VALUE - only
|
||||
# with the latter should the insertion be ignored.
|
||||
stmt = cql.prepare(f"INSERT INTO {table1} (p,c) VALUES (?, ?)")
|
||||
with pytest.raises(InvalidRequest, match='null value'):
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|clustering)')):
|
||||
cql.execute(stmt, [s, None])
|
||||
with pytest.raises(InvalidRequest, match='null value'):
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|partition)')):
|
||||
cql.execute(stmt, [None, s])
|
||||
|
||||
def test_primary_key_in_null(cql, table1):
|
||||
@@ -80,3 +79,18 @@ def test_regular_column_in_null(scylla_only, cql, table1):
|
||||
cql.execute(f"INSERT INTO {table1} (p,c) VALUES ('p', 'c')")
|
||||
with pytest.raises(InvalidRequest, match='null value'):
|
||||
cql.execute(cql.prepare(f"SELECT v FROM {table1} WHERE v IN ? ALLOW FILTERING"), [None])
|
||||
|
||||
# Though nonsensical, this operation is allowed by Cassandra. Ensure we allow it, too.
|
||||
def test_delete_impossible_clustering_range(cql, table1):
|
||||
cql.execute(f"DELETE FROM {table1} WHERE p='p' and c<'a' and c>'a'")
|
||||
|
||||
@pytest.mark.xfail(reason="Unimplemented for clustering key")
|
||||
def test_delete_null_key(cql, table1):
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|partition)')):
|
||||
cql.execute(f"DELETE FROM {table1} WHERE p=null")
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|partition)')):
|
||||
cql.execute(cql.prepare(f"DELETE FROM {table1} WHERE p=?"), [None])
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|clustering)')):
|
||||
cql.execute(f"DELETE FROM {table1} WHERE p='p' AND c=null")
|
||||
with pytest.raises(InvalidRequest, match=re.compile('Invalid (null|clustering)')):
|
||||
cql.execute(cql.prepare(f"DELETE FROM {table1} WHERE p='p' AND c=?"), [None])
|
||||
|
||||
Reference in New Issue
Block a user