diff --git a/cql3/restrictions/primary_key_restrictions.hh b/cql3/restrictions/primary_key_restrictions.hh index bc60484fe9..b9d1a9e888 100644 --- a/cql3/restrictions/primary_key_restrictions.hh +++ b/cql3/restrictions/primary_key_restrictions.hh @@ -100,6 +100,17 @@ public: bool has_unrestricted_components(const schema& schema) const; virtual bool needs_filtering(const schema& schema) const; + + // How long a prefix of the restrictions could have resulted in + // need_filtering() == false. These restrictions do not need to be + // applied during filtering. + // For example, if we have the filter "c1 < 3 and c2 > 3", c1 does + // not need filtering (just a read stopping at c1=3) but c2 does, + // so num_prefix_columns_that_need_not_be_filtered() will be 1. + virtual unsigned int num_prefix_columns_that_need_not_be_filtered() const { + return 0; + } + virtual bool is_all_eq() const { return false; } diff --git a/cql3/restrictions/single_column_primary_key_restrictions.hh b/cql3/restrictions/single_column_primary_key_restrictions.hh index b02f2eda20..d613c5265f 100644 --- a/cql3/restrictions/single_column_primary_key_restrictions.hh +++ b/cql3/restrictions/single_column_primary_key_restrictions.hh @@ -404,6 +404,7 @@ public: } virtual bool needs_filtering(const schema& schema) const override; + virtual unsigned int num_prefix_columns_that_need_not_be_filtered() const override; }; template<> @@ -484,6 +485,39 @@ inline bool single_column_primary_key_restrictions::needs_filter return false; } +// How many of the restrictions (in column order) do not need filtering +// because they are implemented as a slice (potentially, a contiguous disk +// read). For example, if we have the filter "c1 < 3 and c2 > 3", c1 does not +// need filtering but c2 does so num_prefix_columns_that_need_not_be_filtered +// will be 1. +// The implementation of num_prefix_columns_that_need_not_be_filtered() is +// closely tied to that of needs_filtering() above - basically, if only the +// first num_prefix_columns_that_need_not_be_filtered() restrictions existed, +// then needs_filtering() would have returned false. +template<> +inline unsigned single_column_primary_key_restrictions::num_prefix_columns_that_need_not_be_filtered() const { + column_id position = 0; + unsigned int count = 0; + for (const auto& restriction : _restrictions->restrictions() | boost::adaptors::map_values) { + if (restriction->is_contains() || position != restriction->get_column_def().id) { + return count; + } + if (!restriction->is_slice()) { + position = restriction->get_column_def().id + 1; + } + count++; + } + return count; +} + +template<> +inline unsigned single_column_primary_key_restrictions::num_prefix_columns_that_need_not_be_filtered() const { + // skip_filtering() is currently called only for clustering key + // restrictions, so it doesn't matter what we return here. + return 0; +} + + } } diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index 28904a79c2..f7668fe350 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -364,10 +364,10 @@ std::vector statement_restrictions::get_column_defs_fo } } if (_clustering_columns_restrictions->needs_filtering(*_schema)) { - column_id first_non_prefix_id = _schema->clustering_key_columns().begin()->id + - _clustering_columns_restrictions->prefix_size(_schema); + column_id first_filtering_id = _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_non_prefix_id) && (!column_uses_indexing(cdef))) { + if (cdef->id >= first_filtering_id && !column_uses_indexing(cdef)) { column_defs_for_filtering.emplace_back(cdef); } } diff --git a/tests/filtering_test.cc b/tests/filtering_test.cc index 7f5fd26b6b..b1315e0adc 100644 --- a/tests/filtering_test.cc +++ b/tests/filtering_test.cc @@ -259,6 +259,60 @@ SEASTAR_TEST_CASE(test_allow_filtering_clustering_column) { }); } +SEASTAR_TEST_CASE(test_allow_filtering_two_clustering_columns) { + return do_with_cql_env_thread([] (cql_test_env& e) { + e.execute_cql("CREATE TABLE t (p int, c1 int, c2 int, data int, PRIMARY KEY (p, c1, c2))").get(); + + e.execute_cql("INSERT INTO t (p, c1, c2, data) VALUES (1, 2, 3, 1)").get(); + e.execute_cql("INSERT INTO t (p, c1, c2, data) VALUES (1, 3, 4, 2)").get(); + e.execute_cql("INSERT INTO t (p, c1, c2, data) VALUES (1, 2, 5, 3)").get(); + e.execute_cql("INSERT INTO t (p, c1, c2, data) VALUES (2, 3, 4, 4)").get(); + + auto res = e.execute_cql("SELECT * FROM t WHERE p = 1 and c1 < 3 and c2 > 3 ALLOW FILTERING").get0(); + assert_that(res).is_rows().with_rows({ + { + int32_type->decompose(1), + int32_type->decompose(2), + int32_type->decompose(5), + int32_type->decompose(3) + } + }); + // In issue #4121, we noticed that although with "SELECT *" filtering + // was correct, when we select only a column *not* involved in the + // filtering, one of the constraints was ignored. + res = e.execute_cql("SELECT data FROM t WHERE p = 1 and c1 < 3 and c2 > 3 ALLOW FILTERING").get0(); + assert_that(res).is_rows().with_rows({ + { + int32_type->decompose(3), + // Because of issue #4126 our test code also sees as part of + // the results additional columns which were requested just + // for filtering, in this case c2 (=5) was necessary for the + // filtering but c1 was not. These columns may change in the + // future. + int32_type->decompose(5) + } + }); + // Similar to the above test for issue #4121, but with more clustering + // key components, two of them form a slice, two more need filtering. + e.execute_cql("CREATE TABLE t2 (p int, c1 int, c2 int, c3 int, c4 int, data int, PRIMARY KEY (p, c1, c2, c3, c4))").get(); + e.execute_cql("INSERT INTO t2 (p, c1, c2, c3, c4, data) VALUES (1, 1, 2, 3, 3, 1)").get(); + e.execute_cql("INSERT INTO t2 (p, c1, c2, c3, c4, data) VALUES (1, 1, 2, 5, 8, 2)").get(); + e.execute_cql("INSERT INTO t2 (p, c1, c2, c3, c4, data) VALUES (1, 1, 2, 5, 4, 3)").get(); + e.execute_cql("INSERT INTO t2 (p, c1, c2, c3, c4, data) VALUES (1, 1, 4, 3, 4, 4)").get(); + e.execute_cql("INSERT INTO t2 (p, c1, c2, c3, c4, data) VALUES (1, 2, 4, 4, 2, 5)").get(); + res = e.execute_cql("SELECT data FROM t2 WHERE p = 1 and c1 = 1 and c2 < 3 and c3 > 4 and c4 < 7 ALLOW FILTERING").get0(); + assert_that(res).is_rows().with_rows({ + { + int32_type->decompose(3), + // Again, the following appear here just because of issue #4126 + int32_type->decompose(5), + int32_type->decompose(4) + } + }); + }); +} + + SEASTAR_TEST_CASE(test_allow_filtering_static_column) { return do_with_cql_env_thread([] (cql_test_env& e) { e.execute_cql("CREATE TABLE t (a int, b int, c int, s int static, PRIMARY KEY(a, b));").get();