Skip to content

Commit

Permalink
Merge "Backport fixing ignoring ck restrictions in filtering" from Piotr
Browse files Browse the repository at this point in the history
"
Tests: unit(dev)
Refs #4541
"

* 'fix_ignoring_ck_restrictions_in_filtering_for_3.0_2' of https://github.com/psarna/scylla:
  tests: add a test case for filtering clustering key
  cql3: fix qualifying clustering key restrictions for filtering
  cql3: fix fetching clustering key columns for filtering
  • Loading branch information
avikivity committed Jun 24, 2019
2 parents b6fa715 + 55953d7 commit 270d0a8
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 10 deletions.
12 changes: 6 additions & 6 deletions cql3/restrictions/statement_restrictions.cc
Expand Up @@ -363,8 +363,9 @@ std::vector<const column_definition*> statement_restrictions::get_column_defs_fo
}
}
}
if (_clustering_columns_restrictions->needs_filtering(*_schema)) {
column_id first_filtering_id = _schema->clustering_key_columns().begin()->id +
const bool pk_has_unrestricted_components = _partition_key_restrictions->has_unrestricted_components(*_schema);
if (pk_has_unrestricted_components || _clustering_columns_restrictions->needs_filtering(*_schema)) {
column_id first_filtering_id = pk_has_unrestricted_components ? 0 : _schema->clustering_key_columns().begin()->id +
_clustering_columns_restrictions->num_prefix_columns_that_need_not_be_filtered();
for (auto&& cdef : _clustering_columns_restrictions->get_column_defs()) {
if (cdef->id >= first_filtering_id && !column_uses_indexing(cdef)) {
Expand Down Expand Up @@ -479,10 +480,9 @@ bool statement_restrictions::need_filtering() const {
int number_of_filtering_restrictions = _nonprimary_key_restrictions->size();
// If the whole partition key is restricted, it does not imply filtering
if (_partition_key_restrictions->has_unrestricted_components(*_schema) || !_partition_key_restrictions->is_all_eq()) {
number_of_filtering_restrictions += _partition_key_restrictions->size();
if (_clustering_columns_restrictions->has_unrestricted_components(*_schema)) {
number_of_filtering_restrictions += _clustering_columns_restrictions->size() - _clustering_columns_restrictions->prefix_size();
}
number_of_filtering_restrictions += _partition_key_restrictions->size() + _clustering_columns_restrictions->size();
} else if (_clustering_columns_restrictions->has_unrestricted_components(*_schema)) {
number_of_filtering_restrictions += _clustering_columns_restrictions->size() - _clustering_columns_restrictions->prefix_size();
}

if (_partition_key_restrictions->is_multi_column() || _clustering_columns_restrictions->is_multi_column()) {
Expand Down
8 changes: 8 additions & 0 deletions cql3/restrictions/statement_restrictions.hh
Expand Up @@ -395,6 +395,14 @@ public:
return !_nonprimary_key_restrictions->empty();
}

bool pk_restrictions_need_filtering() const {
return _partition_key_restrictions->needs_filtering(*_schema);
}

bool ck_restrictions_need_filtering() const {
return _partition_key_restrictions->has_unrestricted_components(*_schema) || _clustering_columns_restrictions->needs_filtering(*_schema);
}

/**
* @return true if column is restricted by some restriction, false otherwise
*/
Expand Down
8 changes: 4 additions & 4 deletions tests/cql_query_test.cc
Expand Up @@ -2137,8 +2137,8 @@ SEASTAR_TEST_CASE(test_in_restriction) {
return e.execute_cql("select r1 from tir2 where (c1,r1) in ((0, 1),(1,2),(0,1),(1,2),(3,3)) ALLOW FILTERING;");
}).then([&e] (shared_ptr<cql_transport::messages::result_message> msg) {
assert_that(msg).is_rows().with_rows_ignore_order({
{int32_type->decompose(1)},
{int32_type->decompose(2)},
{int32_type->decompose(1), int32_type->decompose(0)},
{int32_type->decompose(2), int32_type->decompose(1)},
});
}).then([&e] {
return e.prepare("select r1 from tir2 where (c1,r1) in ? ALLOW FILTERING;");
Expand All @@ -2163,8 +2163,8 @@ SEASTAR_TEST_CASE(test_in_restriction) {
return e.execute_prepared(prepared_id,raw_values);
}).then([&e] (shared_ptr<cql_transport::messages::result_message> msg) {
assert_that(msg).is_rows().with_rows_ignore_order({
{int32_type->decompose(1)},
{int32_type->decompose(2)},
{int32_type->decompose(1),int32_type->decompose(0)},
{int32_type->decompose(2), int32_type->decompose(1)},
});
});
});
Expand Down
7 changes: 7 additions & 0 deletions tests/filtering_test.cc
Expand Up @@ -634,6 +634,13 @@ SEASTAR_TEST_CASE(test_allow_filtering_with_secondary_index) {
}
});
});

eventually([&] {
auto msg = e.execute_cql("SELECT SUM(e) FROM t WHERE c = 5 AND b = 911 ALLOW FILTERING;").get0();
assert_that(msg).is_rows().with_rows({{ int32_type->decompose(0), {} }});
msg = e.execute_cql("SELECT e FROM t WHERE c = 5 AND b = 3 ALLOW FILTERING;").get0();
assert_that(msg).is_rows().with_rows({{ int32_type->decompose(9), int32_type->decompose(3) }});
});
});
}

Expand Down

0 comments on commit 270d0a8

Please sign in to comment.