From 44ca965ba0c66034e47ad8f4fb1f709d3eadbfbb Mon Sep 17 00:00:00 2001 From: Jan Ciolek Date: Thu, 22 Jul 2021 15:19:12 +0200 Subject: [PATCH] cql3: Remove unused functions like bounds_ranges Finding clustering ranges has been rewritten to use the new expression variant. Old bounds_ranges() and other similar ones are no longer needed. Signed-off-by: Jan Ciolek --- cql3/restrictions/multi_column_restriction.hh | 106 ----------- cql3/restrictions/primary_key_restrictions.hh | 16 -- .../single_column_primary_key_restrictions.hh | 176 ------------------ cql3/restrictions/statement_restrictions.cc | 5 - cql3/restrictions/token_restriction.hh | 39 ---- 5 files changed, 342 deletions(-) diff --git a/cql3/restrictions/multi_column_restriction.hh b/cql3/restrictions/multi_column_restriction.hh index 86e3f464cd..174b8b3713 100644 --- a/cql3/restrictions/multi_column_restriction.hh +++ b/cql3/restrictions/multi_column_restriction.hh @@ -205,11 +205,6 @@ public: throw exceptions::invalid_request_exception(format("{} cannot be restricted by more than one relation if it includes an Equal", get_columns_in_commons(other))); } - - virtual std::vector bounds_ranges(const query_options& options) const override { - return { bounds_range_type::make_singular(composite_value(options)) }; - } - #if 0 @Override protected boolean isSupportedBy(SecondaryIndex index) @@ -262,28 +257,6 @@ public: } return false; } - - virtual std::vector bounds_ranges(const query_options& options) const override { - auto split_in_values = split_values(options); - std::vector bounds; - for (auto&& components : split_in_values) { - for (unsigned i = 0; i < components.size(); i++) { - statements::request_validations::check_not_null(components[i], "Invalid null value in condition for column %s", _column_defs.at(i)->name_as_text()); - } - auto prefix = clustering_key_prefix::from_optional_exploded(*_schema, components); - bounds.emplace_back(bounds_range_type::make_singular(prefix)); - } - auto less_cmp = clustering_key_prefix::less_compare(*_schema); - std::sort(bounds.begin(), bounds.end(), [&] (bounds_range_type& x, bounds_range_type& y) { - return less_cmp(x.start()->value(), y.start()->value()); - }); - auto eq_cmp = clustering_key_prefix::equality(*_schema); - bounds.erase(std::unique(bounds.begin(), bounds.end(), [&] (bounds_range_type& x, bounds_range_type& y) { - return eq_cmp(x.start()->value(), y.start()->value()); - }), bounds.end()); - return bounds; - } - #if 0 @Override public void addIndexExpressionTo(List expressions, @@ -313,8 +286,6 @@ public: return index.supportsOperator(Operator.IN); } #endif -protected: - virtual utils::chunked_vector> split_values(const query_options& options) const = 0; }; /** @@ -335,16 +306,6 @@ public: oper_t::IN, ::make_shared(_values)}; } - -protected: - virtual utils::chunked_vector> split_values(const query_options& options) const override { - utils::chunked_vector> buffers(_values.size()); - std::transform(_values.begin(), _values.end(), buffers.begin(), [&] (const ::shared_ptr& value) { - auto term = static_pointer_cast(value->bind(options)); - return term->copy_elements(); - }); - return buffers; - } }; @@ -364,14 +325,6 @@ public: oper_t::IN, std::move(marker)}; } - -protected: - virtual utils::chunked_vector> split_values(const query_options& options) const override { - auto in_marker = static_pointer_cast(_marker); - auto in_value = static_pointer_cast(in_marker->bind(options)); - statements::request_validations::check_not_null(in_value, "Invalid null value for IN restriction"); - return in_value->get_split_values(); - } }; class multi_column_restriction::slice final : public multi_column_restriction { @@ -404,14 +357,6 @@ public: } return false; } - - virtual std::vector bounds_ranges(const query_options& options) const override { - if (_mode == mode::clustering || !is_mixed_order()) { - return bounds_ranges_unified_order(options); - } else { - return bounds_ranges_mixed_order(options); - } - } #if 0 @Override public void addIndexExpressionTo(List expressions, @@ -472,57 +417,6 @@ private: return vals; } - /** - * Retrieve the bounds for the case that all clustering columns have the same order. - * Having the same order implies we can do a prefix search on the data. - * @param options the query options - * @return the vector of ranges for the restriction - */ - std::vector bounds_ranges_unified_order(const query_options& options) const { - std::optional start_bound; - std::optional end_bound; - auto start_components = read_bound_components(options, statements::bound::START); - if (!start_components.empty()) { - auto start_prefix = clustering_key_prefix::from_optional_exploded(*_schema, start_components); - start_bound = bounds_range_type::bound(std::move(start_prefix), _slice.is_inclusive(statements::bound::START)); - } - auto end_components = read_bound_components(options, statements::bound::END); - if (!end_components.empty()) { - auto end_prefix = clustering_key_prefix::from_optional_exploded(*_schema, end_components); - end_bound = bounds_range_type::bound(std::move(end_prefix), _slice.is_inclusive(statements::bound::END)); - } - if (_mode == mode::cql && !is_asc_order()) { - std::swap(start_bound, end_bound); - } - auto range = bounds_range_type(start_bound, end_bound); - auto bounds = bound_view::from_range(range); - if (bound_view::compare(*_schema)(bounds.second, bounds.first)) { - return { }; - } - return { std::move(range) }; - } - - /** - * Retrieve the bounds when clustering columns are mixed order - * (contains ASC and DESC together). - * Having mixed order implies that a prefix search can't take place, - * instead, the bounds have to be broken down to separate prefix serchable - * ranges such that their combination is equivalent to the original range. - * @param options the query options - * @return the vector of ranges for the restriction - */ - std::vector bounds_ranges_mixed_order(const query_options& options) const { - std::vector ret_ranges; - auto mixed_order_restrictions = build_mixed_order_restriction_set(options); - ret_ranges.reserve(mixed_order_restrictions.size()); - for (auto r : mixed_order_restrictions) { - for (auto&& range : r->bounds_ranges(options)) { - ret_ranges.emplace_back(std::move(range)); - } - } - return ret_ranges; - } - /** * The function returns the first real inequality component. * The first real inequality is the index of the first component in the diff --git a/cql3/restrictions/primary_key_restrictions.hh b/cql3/restrictions/primary_key_restrictions.hh index 1c91c9c6c2..d44e543b6f 100644 --- a/cql3/restrictions/primary_key_restrictions.hh +++ b/cql3/restrictions/primary_key_restrictions.hh @@ -65,8 +65,6 @@ namespace restrictions { class partition_key_restrictions: public restriction, public restrictions, public enable_shared_from_this { public: - using bounds_range_type = dht::partition_range; - partition_key_restrictions() = default; virtual void merge_with(::shared_ptr other) = 0; @@ -76,8 +74,6 @@ public: return this->shared_from_this(); } - virtual std::vector bounds_ranges(const query_options& options) const = 0; - using restrictions::has_supporting_index; bool empty() const override { @@ -106,10 +102,6 @@ public: return false; } - virtual size_t prefix_size() const { - return 0; - } - size_t prefix_size(const schema&) const { return 0; } @@ -117,8 +109,6 @@ public: class clustering_key_restrictions : public restriction, public restrictions, public enable_shared_from_this { public: - using bounds_range_type = query::clustering_range; - clustering_key_restrictions() = default; virtual void merge_with(::shared_ptr other) = 0; @@ -128,8 +118,6 @@ public: return this->shared_from_this(); } - virtual std::vector bounds_ranges(const query_options& options) const = 0; - using restrictions::has_supporting_index; bool empty() const override { @@ -161,10 +149,6 @@ public: return false; } - virtual size_t prefix_size() const { - return 0; - } - size_t prefix_size(const schema& schema) const { size_t count = 0; if (schema.clustering_key_columns().empty()) { diff --git a/cql3/restrictions/single_column_primary_key_restrictions.hh b/cql3/restrictions/single_column_primary_key_restrictions.hh index 1b3864e6bf..bec0b52261 100644 --- a/cql3/restrictions/single_column_primary_key_restrictions.hh +++ b/cql3/restrictions/single_column_primary_key_restrictions.hh @@ -87,7 +87,6 @@ template class single_column_primary_key_restrictions : public primary_key_restrictions { using range_type = query::range; using range_bound = typename range_type::bound; - using bounds_range_type = typename primary_key_restrictions::bounds_range_type; template friend class single_column_primary_key_restrictions; private: @@ -149,25 +148,6 @@ public: this->expression = make_conjunction(std::move(this->expression), restriction->expression); } - virtual size_t prefix_size() const override { - return primary_key_restrictions::prefix_size(*_schema); - } - - ::shared_ptr> get_longest_prefix_restrictions() { - static_assert(std::is_same_v, "Only clustering key can produce longest prefix restrictions"); - size_t current_prefix_size = prefix_size(); - if (current_prefix_size == restrictions().size()) { - return dynamic_pointer_cast>(this->shared_from_this()); - } - - auto longest_prefix_restrictions = ::make_shared>(_schema, _allow_filtering); - auto restriction_it = restrictions().begin(); - for (size_t i = 0; i < current_prefix_size; ++i) { - longest_prefix_restrictions->merge_with((restriction_it++)->second); - } - return longest_prefix_restrictions; - } - virtual void merge_with(::shared_ptr restriction) override { if (find_atom(restriction->expression, [] (const expr::binary_operator& b) { return std::holds_alternative(*b.lhs); @@ -207,108 +187,7 @@ public: return result; } -private: - std::vector compute_bounds(const query_options& options) const { - std::vector ranges; - - // TODO: rewrite this to simply invoke possible_lhs_values on each clustering column, find the first - // non-list, and take Cartesian product of that prefix. No need for to_range() and std::get() here. - if (_restrictions->is_all_eq()) { - std::vector components; - components.reserve(_restrictions->size()); - for (auto&& e : restrictions()) { - const column_definition* def = e.first; - assert(components.size() == _schema->position(*def)); - // Because _restrictions is all EQ, possible_lhs_values must return a list, not a range. - const auto b = std::get(possible_lhs_values(e.first, e.second->expression, options)); - // Furthermore, this list is either a single element (when all RHSs are the same) or empty (when at - // least two are different, so the restrictions cannot hold simultaneously -- ie, c=1 AND c=2). - if (b.empty()) { - return {}; - } - components.emplace_back(b.front()); - } - return {range_type::make_singular(ValueType::from_exploded(*_schema, std::move(components)))}; - } - - std::vector> vec_of_values; - for (auto&& e : restrictions()) { - const column_definition* def = e.first; - auto&& r = e.second; - - if (vec_of_values.size() != _schema->position(*def) || find_needs_filtering(r->expression)) { - // The prefixes built so far are the longest we can build, - // the rest of the constraints will have to be applied using filtering. - break; - } - - if (has_slice(r->expression)) { - const auto values = possible_lhs_values(def, r->expression, options); - if (values == expr::value_set(expr::value_list{})) { - return {}; - } - const auto b = expr::to_range(values); - if (cartesian_product_is_empty(vec_of_values)) { - // TODO: use b.transform(). - const auto make_bound = [&] (const std::optional<::range_bound>& bytes_bound) { - return bytes_bound ? - std::optional(range_bound(ValueType::from_single_value(*_schema, std::move(bytes_bound->value())), - bytes_bound->is_inclusive())) : - std::nullopt; - }; - ranges.emplace_back(range_type(make_bound(b.start()), make_bound(b.end()))); - if (def->type->is_reversed()) { - ranges.back().reverse(); - } - return ranges; - } - - auto size = cartesian_product_size(vec_of_values); - check_cartesian_product_size(size, max_cartesian_product_size(options.get_cql_config().restrictions), - restricted_component_name_v); - ranges.reserve(size); - for (auto&& prefix : make_cartesian_product(vec_of_values)) { - // TODO: use ranges.transform(). - auto make_bound = [&prefix, this] (const std::optional<::range_bound>& bytes_bound) { - if (bytes_bound) { - prefix.emplace_back(bytes_bound->value()); - auto val = ValueType::from_optional_exploded(*_schema, prefix); - prefix.pop_back(); - return range_bound(std::move(val), bytes_bound->is_inclusive()); - } else { - return range_bound(ValueType::from_optional_exploded(*_schema, prefix)); - } - }; - ranges.emplace_back(range_type(make_bound(b.start()), make_bound(b.end()))); - if (def->type->is_reversed()) { - ranges.back().reverse(); - } - } - - return ranges; - } - - auto values = std::get(possible_lhs_values(def, r->expression, options)); - if (values.empty()) { - return {}; - } - vec_of_values.emplace_back(std::make_move_iterator(values.begin()), std::make_move_iterator(values.end())); - } - - auto size = cartesian_product_size(vec_of_values); - check_cartesian_product_size(size, max_cartesian_product_size(options.get_cql_config().restrictions), - restricted_component_name_v); - ranges.reserve(size); - for (auto&& prefix : make_cartesian_product(vec_of_values)) { - ranges.emplace_back(range_type::make_singular(ValueType::from_optional_exploded(*_schema, std::move(prefix)))); - } - - return ranges; - } - public: - std::vector bounds_ranges(const query_options& options) const override; - std::vector values(const query_options& options) const { auto src = values_as_keys(options); std::vector res; @@ -355,61 +234,6 @@ public: virtual unsigned int num_prefix_columns_that_need_not_be_filtered() const override; }; -template<> -inline dht::partition_range_vector -single_column_primary_key_restrictions::bounds_ranges(const query_options& options) const { - dht::partition_range_vector ranges; - ranges.reserve(size()); - for (query::range& r : compute_bounds(options)) { - if (!r.is_singular()) { - throw exceptions::invalid_request_exception("Range queries on partition key values not supported."); - } - ranges.emplace_back(std::move(r).transform( - [this] (partition_key&& k) -> query::ring_position { - auto token = dht::get_token(*_schema, k); - return { std::move(token), std::move(k) }; - })); - } - return ranges; -} - -template<> -inline std::vector -single_column_primary_key_restrictions::bounds_ranges(const query_options& options) const { - auto wrapping_bounds = compute_bounds(options); - auto bounds = boost::copy_range(wrapping_bounds - | boost::adaptors::filtered([&](auto&& r) { - auto bounds = bound_view::from_range(r); - return !bound_view::compare(*_schema)(bounds.second, bounds.first); - }) - | boost::adaptors::transformed([&](auto&& r) { return query::clustering_range(std::move(r)); - })); - auto less_cmp = clustering_key_prefix::less_compare(*_schema); - std::sort(bounds.begin(), bounds.end(), [&] (query::clustering_range& x, query::clustering_range& y) { - if (!x.start() && !y.start()) { - return false; - } - if (!x.start()) { - return true; - } - if (!y.start()) { - return false; - } - return less_cmp(x.start()->value(), y.start()->value()); - }); - auto eq_cmp = clustering_key_prefix::equality(*_schema); - bounds.erase(std::unique(bounds.begin(), bounds.end(), [&] (query::clustering_range& x, query::clustering_range& y) { - if (!x.start() && !y.start()) { - return true; - } - if (!x.start() || !y.start()) { - return false; - } - return eq_cmp(x.start()->value(), y.start()->value()); - }), bounds.end()); - return bounds; -} - template<> inline bool single_column_primary_key_restrictions::needs_filtering(const schema& schema) const { return primary_key_restrictions::needs_filtering(schema); diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index 58f7658b60..959ce64a98 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -63,7 +63,6 @@ public: : _allow_filtering(allow_filtering) { this->expression = true; } - using bounds_range_type = typename primary_key_restrictions::bounds_range_type; ::shared_ptr> do_merge_to(schema_ptr schema, ::shared_ptr restriction) const { return ::make_shared>(schema, _allow_filtering)->merge_to(schema, restriction); @@ -80,10 +79,6 @@ public: bytes_opt value_for(const column_definition& cdef, const query_options& options) const override { return {}; } - std::vector bounds_ranges(const query_options&) const override { - // throw? should not reach? - return {}; - } std::vector get_column_defs() const override { // throw? should not reach? return {}; diff --git a/cql3/restrictions/token_restriction.hh b/cql3/restrictions/token_restriction.hh index 1a4bd68d29..d5a5fd8954 100644 --- a/cql3/restrictions/token_restriction.hh +++ b/cql3/restrictions/token_restriction.hh @@ -90,45 +90,6 @@ public: throw exceptions::unsupported_operation_exception(); } #endif - - std::vector bounds_ranges(const query_options& options) const override { - auto values = possible_lhs_values(nullptr, expression, options); - if (values == expr::value_set(expr::value_list{})) { - return {}; - } - const auto bounds = expr::to_range(values); - const auto start_token = bounds.start() ? bounds.start()->value().with_linearized([] (bytes_view bv) { return dht::token::from_bytes(bv); }) - : dht::minimum_token(); - auto end_token = bounds.end() ? bounds.end()->value().with_linearized([] (bytes_view bv) { return dht::token::from_bytes(bv); }) - : dht::maximum_token(); - const bool include_start = bounds.start() && bounds.start()->is_inclusive(); - const auto include_end = bounds.end() && bounds.end()->is_inclusive(); - - /* - * If we ask SP.getRangeSlice() for (token(200), token(200)], it will happily return the whole ring. - * However, wrapping range doesn't really make sense for CQL, and we want to return an empty result in that - * case (CASSANDRA-5573). So special case to create a range that is guaranteed to be empty. - * - * In practice, we want to return an empty result set if either startToken > endToken, or both are equal but - * one of the bound is excluded (since [a, a] can contains something, but not (a, a], [a, a) or (a, a)). - */ - if (start_token > end_token - || (start_token == end_token - && (!include_start || !include_end))) { - return {}; - } - - typedef typename bounds_range_type::bound bound; - - auto start = bound(include_start - ? dht::ring_position::starting_at(start_token) - : dht::ring_position::ending_at(start_token)); - auto end = bound(include_end - ? dht::ring_position::ending_at(end_token) - : dht::ring_position::starting_at(end_token)); - - return { bounds_range_type(std::move(start), std::move(end)) }; - } }; }