From 6773563d3d8a867fa6580cbc1b78cf0a2dffef1c Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Mon, 19 Oct 2020 14:46:43 -0400 Subject: [PATCH] cql3: Drop unneeded filtering for continuous CK 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 --- cql3/restrictions/statement_restrictions.cc | 1 - test/boost/cql_query_group_test.cc | 2 +- test/boost/cql_query_test.cc | 8 +-- test/boost/filtering_test.cc | 15 +---- test/boost/restrictions_test.cc | 70 ++++++++++----------- test/boost/secondary_index_test.cc | 2 +- 6 files changed, 42 insertions(+), 56 deletions(-) diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index 9f919232a1..31d5ddf380 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -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); diff --git a/test/boost/cql_query_group_test.cc b/test/boost/cql_query_group_test.cc index 1b0989c92f..c5eea3314a 100644 --- a/test/boost/cql_query_group_test.cc +++ b/test/boost/cql_query_group_test.cc @@ -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<>(); }); } diff --git a/test/boost/cql_query_test.cc b/test/boost/cql_query_test.cc index 3cbbd28fb5..a1811966e8 100644 --- a/test/boost/cql_query_test.cc +++ b/test/boost/cql_query_test.cc @@ -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)}, }); } { @@ -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)}, }); } }); diff --git a/test/boost/filtering_test.cc b/test/boost/filtering_test.cc index 24f56f40bb..a9ff2bf005 100644 --- a/test/boost/filtering_test.cc +++ b/test/boost/filtering_test.cc @@ -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(); @@ -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), diff --git a/test/boost/restrictions_test.cc b/test/boost/restrictions_test.cc index 449ae3acec..1b909a7d93 100644 --- a/test/boost/restrictions_test.cc +++ b/test/boost/restrictions_test.cc @@ -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(); } @@ -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")}}); @@ -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}, {sstring("c")})}, {{T("c")}}); @@ -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(); @@ -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> tuples) { const auto tuple_type = tuple_type_impl::get_instance({int32_type, float_type}); @@ -772,29 +772,29 @@ SEASTAR_THREAD_TEST_CASE(multi_col_in) { return list_type->decompose( make_list_value(list_type, std::vector(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(); } diff --git a/test/boost/secondary_index_test.cc b/test/boost/secondary_index_test.cc index ed6a7be52f..6428b9a0a9 100644 --- a/test/boost/secondary_index_test.cc +++ b/test/boost/secondary_index_test.cc @@ -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)} }); }); });