mirrors/scylladb
1
0
Fork 0
mirror of https://github.com/scylladb/scylladb.git synced 2026-06-01 04:26:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

merge: cql3/restrictions: exclude NULLs from comparison in filtering

Browse Source 
Merge pull request https://github.com/scylladb/scylla/pull/6834 by
Juliusz Stasiewicz:

NULLs used to give false positives in GT, LT, GEQ and LEQ ops performed upon
ALLOW FILTERING. That was a consequence of not distinguishing NULL from an
empty buffer.

This patch excludes NULLs on high level, preventing them from entering LHS
of any comparison, i.e. it assumes that any binary operation should return
false whenever the LHS operand is NULL (note: at the moment filters with
RHS NULL, such as ...WHERE x=NULL ALLOW FILTERING, return empty sets anyway).

Fixes #6295

* '6295-do-not-compare-nulls-v2' of github.com:jul-stas/scylla:
  filtering_test: check that NULLs do not compare to normal values
  cql3/restrictions: exclude NULLs from comparison in filtering
This commit is contained in:
Nadav Har'El
2020-07-15 18:32:14 +03:00
parent 9b5f28a2e3 c25398e8cf
commit 09a71ccd84
2 changed files with 43 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cql3/restrictions/statement_restrictions.cc
View File

@@ -807,7 +807,7 @@ bool limits(const binary_operator& opr, const column_value_eval_bag& bag) {
} else if (columns.size() == 1) {
auto lhs = get_value(columns[0], bag);
if (!lhs) {
lhs = bytes(); // Compatible with old code, which feeds null to type comparators.
return false; // NULL never compares to anything (#6295)
}
const auto tup = get_tuple(*opr.rhs, bag.options);
auto rhs = (tup && tup->size() == 1) ? tup->get_elements()[0] // Assume an external query WHERE (ck1)>(123).

44
test/boost/restrictions_test.cc
View File

@@ -232,7 +232,7 @@ SEASTAR_THREAD_TEST_CASE(regular_col_slice) {
require_rows(e, "select * from t where q<11 and q>11 allow filtering", {});
require_rows(e, "select q from t where q<=12 and r>=21 allow filtering", {{I(11), I(21)}, {I(12), I(22)}});
cquery_nofail(e, "insert into t(p) values (4)");
require_rows(e, "select q from t where q<12 allow filtering", {{std::nullopt}, {I(10)}, {I(11)}});
require_rows(e, "select q from t where q<12 allow filtering", {{I(10)}, {I(11)}});
require_rows(e, "select q from t where q>10 allow filtering", {{I(11)}, {I(12)}, {I(13)}});
require_rows(e, "select q from t where q<12 and q>10 allow filtering", {{I(11)}});
}).get();
@@ -260,7 +260,47 @@ SEASTAR_THREAD_TEST_CASE(regular_col_neq) {
}
#endif // 0
SEASTAR_THREAD_TEST_CASE(null) {
namespace {
/// The `op` argument can be one of: "<", "<=", ">", ">=", "=". Remove float args to use "!=".
future<> test_lhs_null_with_operator(sstring op) {
return do_with_cql_env_thread([op = std::move(op)] (cql_test_env& e) {
cquery_nofail(e, "CREATE TABLE cf (pk int primary key, flt float,"
" dbl double, dec decimal, b boolean, ti tinyint, si smallint,"
" i int, bi bigint, vi varint, ip inet, u uuid, tu timeuuid,"
" t time, d date, a ascii, txt text);");
cquery_nofail(e, "INSERT INTO cf (pk) VALUES (0);");
// Now we have all types of NULLs
require_rows(e, format("SELECT * FROM cf WHERE flt {} 99.9 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE dbl {} 99.9 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE dec {} 99.9 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE b {} true ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE ti {} 99 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE si {} 999 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE i {} 999 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE bi {} 999 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE vi {} 999 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE ip {} '255.255.255.255' ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE u {} ffffffff-e89b-12d3-a456-426655440000 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE tu {} ffffffff-e89b-12d3-a456-426655440000 ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE t {} '23:59:59' ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE d {} '2137-01-01' ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE a {} 'Z' ALLOW FILTERING;", op), {});
require_rows(e, format("SELECT * FROM cf WHERE txt {} 'Ż' ALLOW FILTERING;", op), {});
});
}
} // anon. namespace
// This test checks that NULLs on LHS of comparison operator never evaluate to `true` (#6295).
// The expected behavior aligns with C*, but the test also ensures that NULLs are not converted
// to empty buffers somewhere along the way (since empty buffers would lexicographically
// precede anything).
SEASTAR_THREAD_TEST_CASE(null_lhs) {
test_lhs_null_with_operator("<").get();
test_lhs_null_with_operator(">").get();
}
SEASTAR_THREAD_TEST_CASE(null_rhs) {
do_with_cql_env_thread([](cql_test_env& e) {
cquery_nofail(e, "create table t (pk1 int, pk2 int, ck1 float, ck2 float, r text, primary key((pk1,pk2),ck1,ck2))");
const auto q = [&] (const char* stmt) { return e.execute_cql(stmt).get(); };