Merge "Don't allow CK wrapping ranges" from Duarte

"This pathset ensures user-specified clustering key ranges are never
wrapping, as those types of ranges are not defined for CQL3.

Fixes #1544"
This commit is contained in:
Avi Kivity
2016-08-16 10:09:31 +03:00
7 changed files with 47 additions and 34 deletions

View File

@@ -28,9 +28,9 @@
namespace query {
const std::vector<range<clustering_key_prefix>>&
const clustering_row_ranges&
clustering_key_filtering_context::get_ranges(const partition_key& key) const {
static thread_local std::vector<range<clustering_key_prefix>> full_range = {{}};
static thread_local clustering_row_ranges full_range = {{}};
return _factory ? _factory->get_ranges(key) : full_range;
}
@@ -43,9 +43,9 @@ const clustering_key_filtering_context no_clustering_key_filtering =
class stateless_clustering_key_filter_factory : public clustering_key_filter_factory {
clustering_key_filter _filter;
std::vector<range<clustering_key_prefix>> _ranges;
clustering_row_ranges _ranges;
public:
stateless_clustering_key_filter_factory(std::vector<range<clustering_key_prefix>>&& ranges,
stateless_clustering_key_filter_factory(clustering_row_ranges&& ranges,
clustering_key_filter&& filter)
: _filter(std::move(filter)), _ranges(std::move(ranges)) {}
@@ -57,7 +57,7 @@ public:
return _filter;
}
virtual const std::vector<range<clustering_key_prefix>>& get_ranges(const partition_key& key) override {
virtual const clustering_row_ranges& get_ranges(const partition_key& key) override {
return _ranges;
}
@@ -70,7 +70,7 @@ class partition_slice_clustering_key_filter_factory : public clustering_key_filt
schema_ptr _schema;
const partition_slice& _slice;
clustering_key_prefix::prefix_equal_tri_compare _cmp;
query::clustering_row_ranges _ck_ranges;
clustering_row_ranges _ck_ranges;
public:
partition_slice_clustering_key_filter_factory(schema_ptr s, const partition_slice& slice)
: _schema(std::move(s)), _slice(slice), _cmp(*_schema) {}
@@ -79,7 +79,7 @@ public:
const clustering_row_ranges& ranges = _slice.row_ranges(*_schema, key);
return [this, &ranges] (const clustering_key& key) {
return std::any_of(std::begin(ranges), std::end(ranges),
[this, &key] (const range<clustering_key_prefix>& r) { return r.contains(key, _cmp); });
[this, &key] (const clustering_range& r) { return r.contains(key, _cmp); });
};
}
@@ -87,11 +87,11 @@ public:
const clustering_row_ranges& ranges = _slice.row_ranges(*_schema, key);
return [this, &ranges] (const clustering_key& key) {
return std::any_of(std::begin(ranges), std::end(ranges),
[this, &key] (const range<clustering_key_prefix>& r) { return r.contains(key, _cmp); });
[this, &key] (const clustering_range& r) { return r.contains(key, _cmp); });
};
}
virtual const std::vector<range<clustering_key_prefix>>& get_ranges(const partition_key& key) override {
virtual const clustering_row_ranges& get_ranges(const partition_key& key) override {
if (_slice.options.contains(query::partition_slice::option::reversed)) {
_ck_ranges = _slice.row_ranges(*_schema, key);
std::reverse(_ck_ranges.begin(), _ck_ranges.end());
@@ -113,10 +113,10 @@ create_partition_slice_filter(schema_ptr s, const partition_slice& slice) {
const clustering_key_filtering_context
clustering_key_filtering_context::create(schema_ptr schema, const partition_slice& slice) {
static thread_local clustering_key_filtering_context accept_all = clustering_key_filtering_context(
::make_shared<stateless_clustering_key_filter_factory>(std::vector<range<clustering_key_prefix>>{{}},
::make_shared<stateless_clustering_key_filter_factory>(clustering_row_ranges{{}},
[](const clustering_key&) { return true; }));
static thread_local clustering_key_filtering_context reject_all = clustering_key_filtering_context(
::make_shared<stateless_clustering_key_filter_factory>(std::vector<range<clustering_key_prefix>>{},
::make_shared<stateless_clustering_key_filter_factory>(clustering_row_ranges{},
[](const clustering_key&) { return false; }));
if (slice.get_specific_ranges()) {

View File

@@ -394,7 +394,10 @@ public:
return bounds_range_type::bound(prefix, is_inclusive(b));
};
auto range = bounds_range_type(read_bound(statements::bound::START), read_bound(statements::bound::END));
return { range };
if (range.is_wrap_around(clustering_key_prefix::prefix_equal_tri_compare(*_schema))) {
return { };
}
return { std::move(range) };
}
#if 0
@Override

View File

@@ -46,6 +46,8 @@
#include "cartesian_product.hh"
#include "cql3/restrictions/primary_key_restrictions.hh"
#include "cql3/restrictions/single_column_restrictions.hh"
#include <boost/range/adaptor/transformed.hpp>
#include <boost/range/adaptor/filtered.hpp>
namespace cql3 {
@@ -352,7 +354,10 @@ single_column_primary_key_restrictions<partition_key>::bounds_ranges(const query
template<>
std::vector<query::clustering_range>
single_column_primary_key_restrictions<clustering_key_prefix>::bounds_ranges(const query_options& options) const {
auto bounds = compute_bounds(options);
auto wrapping_bounds = compute_bounds(options);
auto tri_cmp = clustering_key_prefix::tri_compare(*_schema);
auto bounds = boost::copy_range<query::clustering_row_ranges>(wrapping_bounds
| boost::adaptors::filtered([&](auto&& r) { return !r.is_wrap_around(tri_cmp); }));
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()) {

View File

@@ -484,7 +484,7 @@ mutation_partition::clustered_row(const schema& s, const clustering_key_view& ke
}
mutation_partition::rows_type::const_iterator
mutation_partition::lower_bound(const schema& schema, const query::range<clustering_key_prefix>& r) const {
mutation_partition::lower_bound(const schema& schema, const query::clustering_range& r) const {
auto cmp = rows_entry::key_comparator(clustering_key_prefix::prefix_equality_less_compare(schema));
return r.start() ? (r.start()->is_inclusive()
? _rows.lower_bound(r.start()->value(), cmp)
@@ -492,7 +492,7 @@ mutation_partition::lower_bound(const schema& schema, const query::range<cluster
}
mutation_partition::rows_type::const_iterator
mutation_partition::upper_bound(const schema& schema, const query::range<clustering_key_prefix>& r) const {
mutation_partition::upper_bound(const schema& schema, const query::clustering_range& r) const {
auto cmp = rows_entry::key_comparator(clustering_key_prefix::prefix_equality_less_compare(schema));
return r.end() ? (r.end()->is_inclusive()
? _rows.upper_bound(r.end()->value(), cmp)
@@ -500,7 +500,7 @@ mutation_partition::upper_bound(const schema& schema, const query::range<cluster
}
boost::iterator_range<mutation_partition::rows_type::const_iterator>
mutation_partition::range(const schema& schema, const query::range<clustering_key_prefix>& r) const {
mutation_partition::range(const schema& schema, const query::clustering_range& r) const {
return boost::make_iterator_range(lower_bound(schema, r), upper_bound(schema, r));
}
@@ -520,22 +520,22 @@ unconst(Container& c, typename Container::const_iterator i) {
}
boost::iterator_range<mutation_partition::rows_type::iterator>
mutation_partition::range(const schema& schema, const query::range<clustering_key_prefix>& r) {
mutation_partition::range(const schema& schema, const query::clustering_range& r) {
return unconst(_rows, static_cast<const mutation_partition*>(this)->range(schema, r));
}
mutation_partition::rows_type::iterator
mutation_partition::lower_bound(const schema& schema, const query::range<clustering_key_prefix>& r) {
mutation_partition::lower_bound(const schema& schema, const query::clustering_range& r) {
return unconst(_rows, static_cast<const mutation_partition*>(this)->lower_bound(schema, r));
}
mutation_partition::rows_type::iterator
mutation_partition::upper_bound(const schema& schema, const query::range<clustering_key_prefix>& r) {
mutation_partition::upper_bound(const schema& schema, const query::clustering_range& r) {
return unconst(_rows, static_cast<const mutation_partition*>(this)->upper_bound(schema, r));
}
template<typename Func>
void mutation_partition::for_each_row(const schema& schema, const query::range<clustering_key_prefix>& row_range, bool reversed, Func&& func) const
void mutation_partition::for_each_row(const schema& schema, const query::clustering_range& row_range, bool reversed, Func&& func) const
{
auto r = range(schema, row_range);
if (!reversed) {

View File

@@ -688,12 +688,12 @@ public:
tombstone range_tombstone_for_row(const schema& schema, const clustering_key& key) const;
tombstone tombstone_for_row(const schema& schema, const clustering_key& key) const;
tombstone tombstone_for_row(const schema& schema, const rows_entry& e) const;
boost::iterator_range<rows_type::const_iterator> range(const schema& schema, const query::range<clustering_key_prefix>& r) const;
rows_type::const_iterator lower_bound(const schema& schema, const query::range<clustering_key_prefix>& r) const;
rows_type::const_iterator upper_bound(const schema& schema, const query::range<clustering_key_prefix>& r) const;
rows_type::iterator lower_bound(const schema& schema, const query::range<clustering_key_prefix>& r);
rows_type::iterator upper_bound(const schema& schema, const query::range<clustering_key_prefix>& r);
boost::iterator_range<rows_type::iterator> range(const schema& schema, const query::range<clustering_key_prefix>& r);
boost::iterator_range<rows_type::const_iterator> range(const schema& schema, const query::clustering_range& r) const;
rows_type::const_iterator lower_bound(const schema& schema, const query::clustering_range& r) const;
rows_type::const_iterator upper_bound(const schema& schema, const query::clustering_range& r) const;
rows_type::iterator lower_bound(const schema& schema, const query::clustering_range& r);
rows_type::iterator upper_bound(const schema& schema, const query::clustering_range& r);
boost::iterator_range<rows_type::iterator> range(const schema& schema, const query::clustering_range& r);
// Writes this partition using supplied query result writer.
// The partition should be first compacted with compact_for_query(), otherwise
// results may include data which is deleted/expired.
@@ -714,5 +714,5 @@ public:
gc_clock::time_point query_time = gc_clock::time_point::min()) const;
private:
template<typename Func>
void for_each_row(const schema& schema, const query::range<clustering_key_prefix>& row_range, bool reversed, Func&& func) const;
void for_each_row(const schema& schema, const query::clustering_range& row_range, bool reversed, Func&& func) const;
};

View File

@@ -2701,7 +2701,9 @@ storage_proxy::do_query(schema_ptr s,
return make_ready_future<foreign_ptr<lw_shared_ptr<query::result>>>(make_foreign(make_lw_shared<query::result>()));
};
if (partition_ranges.empty()) {
auto& slice = cmd->slice;
if (partition_ranges.empty() ||
(slice.default_row_ranges().empty() && !slice.get_specific_ranges())) {
return make_empty();
}
utils::latency_counter lc;

View File

@@ -346,7 +346,7 @@ public:
row_limit = column_limit;
partition_limit = query::max_partitions;
if (start_column) {
auto sr = query::specific_ranges(*range.start()->value().key(), {make_clustering_range(s, *start_column, std::string())});
auto sr = query::specific_ranges(*range.start()->value().key(), {make_clustering_range_and_validate(s, *start_column, std::string())});
specific_ranges = std::make_unique<query::specific_ranges>(std::move(sr));
}
regular_columns.emplace_back(s.regular_begin()->id);
@@ -1289,8 +1289,8 @@ private:
}));
return query::clustering_range::bound(std::move(ck), last != exclusiveness_marker);
}
static query::clustering_range make_clustering_range(const schema& s, const std::string& start, const std::string& end) {
using bound = query::clustering_range::bound;
static range<clustering_key_prefix> make_clustering_range(const schema& s, const std::string& start, const std::string& end) {
using bound = range<clustering_key_prefix>::bound;
stdx::optional<bound> start_bound;
if (!start.empty()) {
start_bound = make_clustering_bound(s, to_bytes_view(start), composite::eoc::end);
@@ -1299,11 +1299,14 @@ private:
if (!end.empty()) {
end_bound = make_clustering_bound(s, to_bytes_view(end), composite::eoc::start);
}
query::clustering_range range = { std::move(start_bound), std::move(end_bound) };
return { std::move(start_bound), std::move(end_bound) };
}
static query::clustering_range make_clustering_range_and_validate(const schema& s, const std::string& start, const std::string& end) {
auto range = make_clustering_range(s, start, end);
if (range.is_wrap_around(clustering_key_prefix::prefix_equal_tri_compare(s))) {
throw make_exception<InvalidRequestException>("Range finish must come after start in the order of traversal");
}
return range;
return query::clustering_range(std::move(range));
}
static range<bytes> make_range(const std::string& start, const std::string& end) {
using bound = range<bytes>::bound;
@@ -1377,7 +1380,7 @@ private:
}
per_partition_row_limit = static_cast<uint32_t>(range.count);
if (s.thrift().is_dynamic()) {
clustering_ranges.emplace_back(make_clustering_range(s, range.start, range.finish));
clustering_ranges.emplace_back(make_clustering_range_and_validate(s, range.start, range.finish));
regular_columns.emplace_back(s.regular_begin()->id);
} else {
clustering_ranges.emplace_back(query::clustering_range::make_open_ended_both_sides());