Skip to content

Commit

Permalink
cql3: Drop unneeded filtering for continuous CK
Browse files Browse the repository at this point in the history 
Don't require filtering when a continuous slice of the clustering key
is requested, even if partition is unrestricted.  The read command we
generate will fetch just the selected data; filtering is unnecessary.

Some tests needed to update the expected results now that we're not
fetching the extra data needed for filtering.  (Because tests don't do
the final trim to match selectors and assert instead on all the data
read.)

Signed-off-by: Dejan Mircevski <dejan@scylladb.com>
  • Loading branch information
Dejan Mircevski committed Oct 19, 2020
1 parent 86bbf17 commit 6773563
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 56 deletions.
1 change: 0 additions & 1 deletion cql3/restrictions/statement_restrictions.cc
View file
Expand Up @@ -504,7 +504,6 @@ bool statement_restrictions::need_filtering() const {
number_of_filtering_restrictions += _clustering_columns_restrictions->size() - _clustering_columns_restrictions->prefix_size();
}
return number_of_restricted_columns_for_indexing > 1
|| (number_of_restricted_columns_for_indexing == 0 && _partition_key_restrictions->empty() && !_clustering_columns_restrictions->empty())
|| (number_of_restricted_columns_for_indexing != 0 && _nonprimary_key_restrictions->has_multiple_contains())
|| (number_of_restricted_columns_for_indexing != 0 && !_uses_secondary_indexing)
|| (_uses_secondary_indexing && number_of_filtering_restrictions > 1);
Expand Down
2 changes: 1 addition & 1 deletion test/boost/cql_query_group_test.cc
View file
Expand Up @@ -226,7 +226,7 @@ SEASTAR_TEST_CASE(test_group_by_text_key) {
require_rows(e, "select avg(v) from t2 group by p", {{I(25), T(" ")}});
require_rows(e, "select avg(v) from t2 group by p, c1", {{I(15), T(" "), T("")}, {I(35), T(" "), T("a")}});
require_rows(e, "select sum(v) from t2 where c1='' group by p, c2 allow filtering",
{{I(10), T(""), T(" "), T("")}, {I(20), T(""), T(" "), T("b")}});
{{I(10), T(" "), T("")}, {I(20), T(" "), T("b")}});
return make_ready_future<>();
});
}
Expand Down
8 changes: 4 additions & 4 deletions test/boost/cql_query_test.cc
View file
Expand Up @@ -2397,8 +2397,8 @@ SEASTAR_TEST_CASE(test_in_restriction) {
{
auto msg = e.execute_cql("select r1 from tir2 where (c1,r1) in ((0, 1),(1,2),(0,1),(1,2),(3,3)) ALLOW FILTERING;").get0();
assert_that(msg).is_rows().with_rows({
{int32_type->decompose(1), int32_type->decompose(0)},
{int32_type->decompose(2), int32_type->decompose(1)},
{int32_type->decompose(1)},
{int32_type->decompose(2)},
});
}
{
Expand All @@ -2422,8 +2422,8 @@ SEASTAR_TEST_CASE(test_in_restriction) {
raw_values.emplace_back(cql3::raw_value::make_value(in_values_list));
auto msg = e.execute_prepared(prepared_id,raw_values).get0();
assert_that(msg).is_rows().with_rows({
{int32_type->decompose(1), int32_type->decompose(0)},
{int32_type->decompose(2), int32_type->decompose(1)},
{int32_type->decompose(1)},
{int32_type->decompose(2)},
});
}
});
Expand Down
15 changes: 1 addition & 14 deletions test/boost/filtering_test.cc
View file
Expand Up @@ -65,16 +65,6 @@ SEASTAR_TEST_CASE(test_allow_filtering_check) {
e.execute_cql(q + " ALLOW FILTERING").get();
}

queries = {
"SELECT * FROM t WHERE c = 2",
"SELECT * FROM t WHERE c <= 4"
};

for (const sstring& q : queries) {
BOOST_CHECK_THROW(e.execute_cql(q).get(), exceptions::invalid_request_exception);
e.execute_cql(q + " ALLOW FILTERING").get();
}

e.execute_cql("CREATE TABLE t2 (p int PRIMARY KEY, a int, b int);").get();
e.require_table_exists("ks", "t2").get();
e.execute_cql("CREATE INDEX ON t2(a)").get();
Expand Down Expand Up @@ -344,10 +334,7 @@ SEASTAR_TEST_CASE(test_allow_filtering_clustering_column) {
int32_type->decompose(1)
}});

BOOST_CHECK_THROW(e.execute_cql("SELECT * FROM t WHERE c = 2").get(), exceptions::invalid_request_exception);
BOOST_CHECK_THROW(e.execute_cql("SELECT * FROM t WHERE c > 2 AND c <= 4").get(), exceptions::invalid_request_exception);

msg = e.execute_cql("SELECT * FROM t WHERE c = 2 ALLOW FILTERING").get0();
msg = e.execute_cql("SELECT * FROM t WHERE c = 2").get0();
assert_that(msg).is_rows().with_rows({
{
int32_type->decompose(1),
Expand Down
70 changes: 35 additions & 35 deletions test/boost/restrictions_test.cc
View file
Expand Up @@ -183,11 +183,11 @@ SEASTAR_THREAD_TEST_CASE(tuple_of_list) {
cquery_nofail(e, "insert into t (p, l1, l2) values (1, [11,12], [101,102])");
cquery_nofail(e, "insert into t (p, l1, l2) values (2, [21,22], [201,202])");
require_rows(e, "select * from t where (l1,l2)<([],[]) allow filtering", {});
require_rows(e, "select l1 from t where (l1,l2)<([20],[200]) allow filtering", {{LI({11, 12}), LI({101, 102})}});
require_rows(e, "select l1 from t where (l1,l2)<([20],[200]) allow filtering", {{LI({11, 12})}});
require_rows(e, "select l1 from t where (l1,l2)>=([11,12],[101,102]) allow filtering",
{{LI({11, 12}), LI({101, 102})}, {LI({21, 22}), LI({201, 202})}});
{{LI({11, 12})}, {LI({21, 22})}});
require_rows(e, "select l1 from t where (l1,l2)<([11,12],[101,103]) allow filtering",
{{LI({11, 12}), LI({101, 102})}});
{{LI({11, 12})}});
}).get();
}

Expand Down Expand Up @@ -345,17 +345,17 @@ SEASTAR_THREAD_TEST_CASE(multi_col_eq) {
require_rows(e, "select c2 from t where p=1 and (c1,c2)=('one',11)", {{F(11)}});
require_rows(e, "select c1 from t where p=1 and (c1)=('one')", {{T("one")}});
require_rows(e, "select c2 from t where p=2 and (c1,c2)=('one',11)", {});
require_rows(e, "select p from t where (c1,c2)=('two',12) allow filtering", {{I(2), T("two"), F(12)}});
require_rows(e, "select p from t where (c1,c2)=('two',12) allow filtering", {{I(2)}});
require_rows(e, "select c2 from t where (c1,c2)=('one',12) allow filtering", {});
require_rows(e, "select c2 from t where (c1,c2)=('two',11) allow filtering", {});
require_rows(e, "select c1 from t where (c1)=('one') allow filtering", {{T("one")}});
require_rows(e, "select c1 from t where (c1)=('x') allow filtering", {});
auto stmt = e.prepare("select p from t where (c1,c2)=:t allow filtering").get0();
require_rows(e, stmt, {{"t"}}, {make_tuple({utf8_type, float_type}, {sstring("two"), 12.f})},
{{I(2), T("two"), F(12)}});
{{I(2)}});
require_rows(e, stmt, {{"t"}}, {make_tuple({utf8_type, float_type}, {sstring("x"), 12.f})}, {});
stmt = e.prepare("select p from t where (c1,c2)=('two',?) allow filtering").get0();
require_rows(e, stmt, {}, {F(12)}, {{I(2), T("two"), F(12)}});
require_rows(e, stmt, {}, {F(12)}, {{I(2)}});
require_rows(e, stmt, {}, {F(99)}, {});
stmt = e.prepare("select c1 from t where (c1)=? allow filtering").get0();
require_rows(e, stmt, {}, {make_tuple({utf8_type}, {sstring("one")})}, {{T("one")}});
Expand All @@ -374,21 +374,21 @@ SEASTAR_THREAD_TEST_CASE(multi_col_slice) {
cquery_nofail(e, "insert into t (p, c1, c2) values (1, 'a', 11);");
cquery_nofail(e, "insert into t (p, c1, c2) values (2, 'b', 2);");
cquery_nofail(e, "insert into t (p, c1, c2) values (3, 'c', 13);");
require_rows(e, "select c2 from t where (c1,c2)>('a',20) allow filtering", {{F(2), T("b")}, {F(13), T("c")}});
require_rows(e, "select c2 from t where (c1,c2)>('a',20) allow filtering", {{F(2)}, {F(13)}});
require_rows(e, "select p from t where (c1,c2)>=('a',20) and (c1,c2)<('b',3) allow filtering",
{{I(2), T("b"), F(2)}});
{{I(2)}});
require_rows(e, "select * from t where (c1,c2)<('a',11) allow filtering", {});
require_rows(e, "select c1 from t where (c1,c2)<('a',12) allow filtering", {{T("a"), F(11)}});
require_rows(e, "select c1 from t where (c1,c2)<('a',12) allow filtering", {{T("a")}});
require_rows(e, "select c1 from t where (c1)>=('c') allow filtering", {{T("c")}});
require_rows(e, "select c1 from t where (c1,c2)<=('c',13) allow filtering",
{{T("a"), F(11)}, {T("b"), F(2)}, {T("c"), F(13)}});
{{T("a")}, {T("b")}, {T("c")}});
require_rows(e, "select c1 from t where (c1,c2)>=('b',2) and (c1,c2)<=('b',2) allow filtering",
{{T("b"), F(2)}});
{{T("b")}});
auto stmt = e.prepare("select c1 from t where (c1,c2)<? allow filtering").get0();
require_rows(e, stmt, {}, {make_tuple({utf8_type, float_type}, {sstring("a"), 12.f})}, {{T("a"), F(11)}});
require_rows(e, stmt, {}, {make_tuple({utf8_type, float_type}, {sstring("a"), 12.f})}, {{T("a")}});
require_rows(e, stmt, {}, {make_tuple({utf8_type, float_type}, {sstring("a"), 11.f})}, {});
stmt = e.prepare("select c1 from t where (c1,c2)<('a',:c2) allow filtering").get0();
require_rows(e, stmt, {{"c2"}}, {F(12)}, {{T("a"), F(11)}});
require_rows(e, stmt, {{"c2"}}, {F(12)}, {{T("a")}});
require_rows(e, stmt, {{"c2"}}, {F(11)}, {});
stmt = e.prepare("select c1 from t where (c1)>=? allow filtering").get0();
require_rows(e, stmt, {}, {make_tuple({utf8_type}, {sstring("c")})}, {{T("c")}});
Expand All @@ -407,9 +407,9 @@ SEASTAR_THREAD_TEST_CASE(multi_col_slice_reversed) {
cquery_nofail(e, "insert into t(p,c1,c2) values (1,12,22)");
cquery_nofail(e, "insert into t(p,c1,c2) values (1,12,23)");
require_rows(e, "select c1 from t where (c1,c2)>(10,99) allow filtering",
{{I(11), F(21)}, {I(12), F(22)}, {I(12), F(23)}});
require_rows(e, "select c1 from t where (c1,c2)<(12,0) allow filtering", {{I(11), F(21)}});
require_rows(e, "select c1 from t where (c1,c2)>(12,22) allow filtering", {{I(12), F(23)}});
{{I(11)}, {I(12)}, {I(12)}});
require_rows(e, "select c1 from t where (c1,c2)<(12,0) allow filtering", {{I(11)}});
require_rows(e, "select c1 from t where (c1,c2)>(12,22) allow filtering", {{I(12)}});
require_rows(e, "select c1 from t where (c1)>(12) allow filtering", {});
require_rows(e, "select c1 from t where (c1)<=(12) allow filtering", {{I(11)}, {I(12)}, {I(12)}});
}).get();
Expand Down Expand Up @@ -743,25 +743,25 @@ SEASTAR_THREAD_TEST_CASE(multi_col_in) {
cquery_nofail(e, "create table t (pk int, ck1 int, ck2 float, r text, primary key (pk, ck1, ck2))");
require_rows(e, "select ck1 from t where (ck1,ck2) in ((11,21),(12,22)) allow filtering", {});
cquery_nofail(e, "insert into t(pk,ck1,ck2) values (1,11,21)");
require_rows(e, "select ck1 from t where (ck1,ck2) in ((11,21),(12,22)) allow filtering", {{I(11), F(21)}});
require_rows(e, "select ck1 from t where (ck1,ck2) in ((11,21)) allow filtering", {{I(11), F(21)}});
require_rows(e, "select ck1 from t where (ck1,ck2) in ((11,21),(12,22)) allow filtering", {{I(11)}});
require_rows(e, "select ck1 from t where (ck1,ck2) in ((11,21)) allow filtering", {{I(11)}});
cquery_nofail(e, "insert into t(pk,ck1,ck2) values (2,12,22)");
require_rows(e, "select ck1 from t where (ck1,ck2) in ((11,21),(12,22)) allow filtering",
{{I(11), F(21)}, {I(12), F(22)}});
{{I(11)}, {I(12)}});
cquery_nofail(e, "insert into t(pk,ck1,ck2) values (3,13,23)");
require_rows(e, "select ck1 from t where (ck1,ck2) in ((11,21),(12,22)) allow filtering",
{{I(11), F(21)}, {I(12), F(22)}});
require_rows(e, "select ck1 from t where (ck1,ck2) in ((13,23)) allow filtering", {{I(13), F(23)}});
{{I(11)}, {I(12)}});
require_rows(e, "select ck1 from t where (ck1,ck2) in ((13,23)) allow filtering", {{I(13)}});
cquery_nofail(e, "insert into t(pk,ck1,ck2,r) values (4,13,23,'a')");
require_rows(e, "select pk from t where (ck1,ck2) in ((13,23)) allow filtering",
{{I(3), I(13), F(23)}, {I(4), I(13), F(23)}});
{{I(3)}, {I(4)}});
require_rows(e, "select pk from t where (ck1) in ((13),(33),(44)) allow filtering",
{{I(3), I(13)}, {I(4), I(13)}});
{{I(3)}, {I(4)}});
// TODO: uncomment when #6200 is fixed.
// require_rows(e, "select pk from t where (ck1,ck2) in ((13,23)) and r='a' allow filtering",
// {{I(4), I(13), F(23), T("a")}});
cquery_nofail(e, "delete from t where pk=4");
require_rows(e, "select pk from t where (ck1,ck2) in ((13,23)) allow filtering", {{I(3), I(13), F(23)}});
require_rows(e, "select pk from t where (ck1,ck2) in ((13,23)) allow filtering", {{I(3)}});
auto stmt = e.prepare("select ck1 from t where (ck1,ck2) in ? allow filtering").get0();
auto bound_tuples = [] (std::vector<std::tuple<int32_t, float>> tuples) {
const auto tuple_type = tuple_type_impl::get_instance({int32_type, float_type});
Expand All @@ -772,29 +772,29 @@ SEASTAR_THREAD_TEST_CASE(multi_col_in) {
return list_type->decompose(
make_list_value(list_type, std::vector<data_value>(tvals.begin(), tvals.end())));
};
require_rows(e, stmt, {}, {bound_tuples({{11, 21}})}, {{I(11), F(21)}});
require_rows(e, stmt, {}, {bound_tuples({{11, 21}, {11, 99}})}, {{I(11), F(21)}});
require_rows(e, stmt, {}, {bound_tuples({{12, 22}})}, {{I(12), F(22)}});
require_rows(e, stmt, {}, {bound_tuples({{13, 13}, {12, 22}})}, {{I(12), F(22)}});
require_rows(e, stmt, {}, {bound_tuples({{11, 21}})}, {{I(11)}});
require_rows(e, stmt, {}, {bound_tuples({{11, 21}, {11, 99}})}, {{I(11)}});
require_rows(e, stmt, {}, {bound_tuples({{12, 22}})}, {{I(12)}});
require_rows(e, stmt, {}, {bound_tuples({{13, 13}, {12, 22}})}, {{I(12)}});
require_rows(e, stmt, {}, {bound_tuples({{12, 21}})}, {});
require_rows(e, stmt, {}, {bound_tuples({{12, 21}, {12, 21}, {13, 21}, {14, 21}})}, {});
stmt = e.prepare("select ck1 from t where (ck1,ck2) in (?) allow filtering").get0();
auto tpl = [] (int32_t e1, float e2) {
return make_tuple({int32_type, float_type}, {e1, e2});
};
require_rows(e, stmt, {}, {tpl(11, 21)}, {{I(11), F(21)}});
require_rows(e, stmt, {}, {tpl(12, 22)}, {{I(12), F(22)}});
require_rows(e, stmt, {}, {tpl(11, 21)}, {{I(11)}});
require_rows(e, stmt, {}, {tpl(12, 22)}, {{I(12)}});
require_rows(e, stmt, {}, {tpl(12, 21)}, {});
stmt = e.prepare("select ck1 from t where (ck1,ck2) in (:t1,:t2) allow filtering").get0();
require_rows(e, stmt, {{"t1", "t2"}}, {tpl(11, 21), tpl(12, 22)}, {{I(11), F(21)}, {I(12), F(22)}});
require_rows(e, stmt, {{"t1", "t2"}}, {tpl(11, 21), tpl(11, 21)}, {{I(11), F(21)}});
require_rows(e, stmt, {{"t1", "t2"}}, {tpl(11, 21), tpl(99, 99)}, {{I(11), F(21)}});
require_rows(e, stmt, {{"t1", "t2"}}, {tpl(11, 21), tpl(12, 22)}, {{I(11)}, {I(12)}});
require_rows(e, stmt, {{"t1", "t2"}}, {tpl(11, 21), tpl(11, 21)}, {{I(11)}});
require_rows(e, stmt, {{"t1", "t2"}}, {tpl(11, 21), tpl(99, 99)}, {{I(11)}});
require_rows(e, stmt, {{"t1", "t2"}}, {tpl(9, 9), tpl(99, 99)}, {});
// Parsing error:
// stmt = e.prepare("select ck1 from t where (ck1,ck2) in ((13,23),:p1) allow filtering").get0();
stmt = e.prepare("select ck1 from t where (ck1,ck2) in ((13,23),(?,?)) allow filtering").get0();
require_rows(e, stmt, {}, {I(0), F(0)}, {{I(13), F(23)}});
require_rows(e, stmt, {}, {I(11), F(21)}, {{I(11), F(21)}, {I(13), F(23)}});
require_rows(e, stmt, {}, {I(0), F(0)}, {{I(13)}});
require_rows(e, stmt, {}, {I(11), F(21)}, {{I(11)}, {I(13)}});
}).get();
}

Expand Down
2 changes: 1 addition & 1 deletion test/boost/secondary_index_test.cc
View file
Expand Up @@ -1273,7 +1273,7 @@ SEASTAR_TEST_CASE(test_filtering_indexed_column) {
eventually([&] {
auto msg = cquery_nofail(e, "select d from ks.test_index where c > 25 ALLOW FILTERING;");
assert_that(msg).is_rows().with_rows({
{int32_type->decompose(44), int32_type->decompose(33)}
{int32_type->decompose(44)}
});
});
});
Expand Down

0 comments on commit 6773563

Please sign in to comment.