mirrors/scylladb
1
0
Fork 0
mirror of https://github.com/scylladb/scylladb.git synced 2026-05-12 19:02:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

cql: fix qualifying indexed columns for filtering

Browse Source 
When qualifying columns to be fetched for filtering, we also check
if the target column is not used as an index - in which case there's
no need of fetching it. However, the check was incorrectly assuming
that any restriction is eligible for indexing, while it's currently
only true for EQ. The fix makes a more specific check and contains
many dynamic casts, but these will hopefully we gone once our
long planned "restrictions rewrite" is done.
This commit comes with a test.

Fixes #5708
Tests: unit(dev)
This commit is contained in:
Piotr Sarna
2020-03-17 19:28:32 +01:00
parent 14de126ff8
commit 767ff59418
2 changed files with 65 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
cql3/restrictions/statement_restrictions.cc
View File

@@ -392,28 +392,45 @@ std::vector<const column_definition*> statement_restrictions::get_column_defs_fo
if (need_filtering()) {
auto& sim = db.find_column_family(_schema).get_index_manager();
auto [opt_idx, _] = find_idx(sim);
auto column_uses_indexing = [&opt_idx] (const column_definition* cdef) {
return opt_idx && opt_idx->depends_on(*cdef);
auto column_uses_indexing = [&opt_idx] (const column_definition* cdef, ::shared_ptr<single_column_restriction> restr) {
return opt_idx && restr && restr->is_supported_by(*opt_idx);
};
auto single_pk_restrs = dynamic_pointer_cast<single_column_partition_key_restrictions>(_partition_key_restrictions);
if (_partition_key_restrictions->needs_filtering(*_schema)) {
for (auto&& cdef : _partition_key_restrictions->get_column_defs()) {
if (!column_uses_indexing(cdef)) {
::shared_ptr<single_column_restriction> restr;
if (single_pk_restrs) {
auto it = single_pk_restrs->restrictions().find(cdef);
if (it != single_pk_restrs->restrictions().end()) {
restr = dynamic_pointer_cast<single_column_restriction>(it->second);
}
}
if (!column_uses_indexing(cdef, restr)) {
column_defs_for_filtering.emplace_back(cdef);
}
}
}
auto single_ck_restrs = dynamic_pointer_cast<single_column_clustering_key_restrictions>(_clustering_columns_restrictions);
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)) {
::shared_ptr<single_column_restriction> restr;
if (single_pk_restrs) {
auto it = single_ck_restrs->restrictions().find(cdef);
if (it != single_ck_restrs->restrictions().end()) {
restr = dynamic_pointer_cast<single_column_restriction>(it->second);
}
}
if (cdef->id >= first_filtering_id && !column_uses_indexing(cdef, restr)) {
column_defs_for_filtering.emplace_back(cdef);
}
}
}
for (auto&& cdef : _nonprimary_key_restrictions->get_column_defs()) {
if (!column_uses_indexing(cdef)) {
auto restr = dynamic_pointer_cast<single_column_restriction>(_nonprimary_key_restrictions->get_restriction(*cdef));
if (!column_uses_indexing(cdef, restr)) {
column_defs_for_filtering.emplace_back(cdef);
}
}

43
test/boost/secondary_index_test.cc
View File

@@ -1223,3 +1223,46 @@ SEASTAR_TEST_CASE(test_computed_columns) {
});
});
}
// Ref: #5708 - filtering should be applied on an indexed column
// if the restriction is not eligible for indexing (it's not EQ)
SEASTAR_TEST_CASE(test_filtering_indexed_column) {
return do_with_cql_env_thread([] (auto& e) {
cquery_nofail(e, "CREATE TABLE test_index (a INT, b INT, c INT, d INT, e INT, PRIMARY KEY ((a, b),c));");
cquery_nofail(e, "CREATE INDEX ON test_index(d);");
cquery_nofail(e, "INSERT INTO test_index (a, b, c, d, e) VALUES (1, 2, 3, 4, 5);");
cquery_nofail(e, "INSERT INTO test_index (a, b, c, d, e) VALUES (11, 22, 33, 44, 55);");
cquery_nofail(e, "select c,e from ks.test_index where d = 44 ALLOW FILTERING;");
cquery_nofail(e, "select a from ks.test_index where d > 43 ALLOW FILTERING;");
eventually([&] {
auto msg = cquery_nofail(e, "select c,e from ks.test_index where d = 44 ALLOW FILTERING;");
assert_that(msg).is_rows().with_rows({
{int32_type->decompose(33), int32_type->decompose(55)}
});
});
eventually([&] {
auto msg = cquery_nofail(e, "select a from ks.test_index where d > 43 ALLOW FILTERING;");
// NOTE: Column d will also be fetched, because it needs to be filtered.
// It's not the case in the previous select, where d was only used as an index.
assert_that(msg).is_rows().with_rows({
{int32_type->decompose(11), int32_type->decompose(44)}
});
});
cquery_nofail(e, "CREATE INDEX ON test_index(b);");
cquery_nofail(e, "CREATE INDEX ON test_index(c);");
eventually([&] {
auto msg = cquery_nofail(e, "select e from ks.test_index where b > 12 ALLOW FILTERING;");
assert_that(msg).is_rows().with_rows({
{int32_type->decompose(55), int32_type->decompose(22)}
});
});
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)}
});
});
});
}