cql3: expr: push is_satisfied_by regular and static column extraction to callers

is_satisfied_by() rearranges the static and regular columns from
query::result_row_view form (which is a use-once iterator) to
std::vector<managed_bytes_opt> (which uses the standard value
representation, and allows random access which expression
evaluation needs). Doing it in is_saitisfied_by() means that it is
done every time an expression is evaluated, which is wasteful. It's
also done even if the expression doesn't need it at all.

Push it out to callers, which already eliminates some calls.

We still pass cql3::expr::selection, which is a layering violation,
but that is left to another time.

Note that in view.cc's check_if_matches(), we should have been
able to move static_and_regular_columns calculation outside the
loop. However, we get crashes if we do. This is likely due to a
preexisting bug (which the zero iterations loop avoids). However,
in selection.cc, we are able to avoid the computation when the code
claims it is only handling partition keys or clustering keys.
This commit is contained in:
Avi Kivity
2022-06-12 13:22:39 +03:00
parent 4b715226fe
commit c80999fab4
4 changed files with 27 additions and 19 deletions

View File

@@ -456,6 +456,8 @@ managed_bytes_opt next_value(query::result_row_view::iterator_type& iter, const
return std::nullopt;
}
} // anonymous namespace
/// Returns values of non-primary-key columns from selection. The kth element of the result
/// corresponds to the kth column in selection.
std::vector<managed_bytes_opt> get_non_pk_values(const selection& selection, const query::result_row_view& static_row,
@@ -481,6 +483,8 @@ std::vector<managed_bytes_opt> get_non_pk_values(const selection& selection, con
return vals;
}
namespace {
/// True iff cv matches the CQL LIKE pattern.
bool like(const column_value& cv, const raw_value_view& pattern, const column_value_eval_bag& bag) {
if (!cv.col->type->is_string()) {
@@ -796,9 +800,12 @@ expression make_conjunction(expression a, expression b) {
}
bool is_satisfied_by(const expression& restr, const evaluation_inputs& inputs) {
const auto regulars = get_non_pk_values(*inputs.selection, *inputs.static_row, inputs.row);
static const auto dummy_static_and_regular_columns = std::vector<managed_bytes_opt>();
auto& static_and_regular_columns = inputs.static_and_regular_columns
? *inputs.static_and_regular_columns
: dummy_static_and_regular_columns;
return is_satisfied_by(
restr, {*inputs.options, row_data_from_partition_slice{*inputs.partition_key, *inputs.clustering_key, regulars, *inputs.selection}});
restr, {*inputs.options, row_data_from_partition_slice{*inputs.partition_key, *inputs.clustering_key, static_and_regular_columns, *inputs.selection}});
}
template<typename T>

View File

@@ -451,12 +451,15 @@ extern std::ostream& operator<<(std::ostream&, oper_t);
struct evaluation_inputs {
const std::vector<bytes>* partition_key = nullptr;
const std::vector<bytes>* clustering_key = nullptr;
const query::result_row_view* static_row = nullptr;
const query::result_row_view* row = nullptr;
const std::vector<managed_bytes_opt>* static_and_regular_columns = nullptr; // indexes match `selection` member
const cql3::selection::selection* selection = nullptr;
const query_options* options = nullptr;
};
/// Helper for generating evaluation_inputs::static_and_regular_columns
std::vector<managed_bytes_opt> get_non_pk_values(const cql3::selection::selection& selection, const query::result_row_view& static_row,
const query::result_row_view* row);
/// True iff restr evaluates to true, given these inputs
extern bool is_satisfied_by(
const expression& restr, const evaluation_inputs& inputs);

View File

@@ -426,13 +426,14 @@ bool result_set_builder::restrictions_filter::do_filter(const selection& selecti
auto clustering_columns_restrictions = _restrictions->get_clustering_columns_restrictions();
if (dynamic_pointer_cast<cql3::restrictions::multi_column_restriction>(clustering_columns_restrictions)) {
clustering_key_prefix ckey = clustering_key_prefix::from_exploded(clustering_key);
// FIXME: push to upper layer so it happens once per row
auto static_and_regular_columns = expr::get_non_pk_values(selection, static_row, row);
return expr::is_satisfied_by(
clustering_columns_restrictions->expression,
expr::evaluation_inputs{
.partition_key = &partition_key,
.clustering_key = &clustering_key,
.static_row = &static_row,
.row = row,
.static_and_regular_columns = &static_and_regular_columns,
.selection = &selection,
.options = &_options,
});
@@ -454,13 +455,14 @@ bool result_set_builder::restrictions_filter::do_filter(const selection& selecti
continue;
}
restrictions::single_column_restriction& restriction = *restr_it->second;
// FIXME: push to upper layer so it happens once per row
auto static_and_regular_columns = expr::get_non_pk_values(selection, static_row, row);
bool regular_restriction_matches = expr::is_satisfied_by(
restriction.expression,
expr::evaluation_inputs{
.partition_key = &partition_key,
.clustering_key = &clustering_key,
.static_row = &static_row,
.row = row,
.static_and_regular_columns = &static_and_regular_columns,
.selection = &selection,
.options = &_options,
});
@@ -485,8 +487,7 @@ bool result_set_builder::restrictions_filter::do_filter(const selection& selecti
expr::evaluation_inputs{
.partition_key = &partition_key,
.clustering_key = &clustering_key,
.static_row = &static_row,
.row = row,
.static_and_regular_columns = nullptr, // partition key filtering only
.selection = &selection,
.options = &_options,
})) {
@@ -513,8 +514,7 @@ bool result_set_builder::restrictions_filter::do_filter(const selection& selecti
expr::evaluation_inputs{
.partition_key = &partition_key,
.clustering_key = &clustering_key,
.static_row = &static_row,
.row = row,
.static_and_regular_columns = nullptr, // clustering key checks only
.selection = &selection,
.options = &_options,
})) {

View File

@@ -249,8 +249,7 @@ bool partition_key_matches(const schema& base, const view_info& view, const dht:
cql3::expr::evaluation_inputs{
.partition_key = &exploded_pk,
.clustering_key = &exploded_ck,
.static_row = &dummy_row,
.row = &dummy_row,
.static_and_regular_columns = nullptr, // partition key filtering only
.selection = selection.get(),
.options = &dummy_options,
});
@@ -267,7 +266,6 @@ bool clustering_prefix_matches(const schema& base, const view_info& view, const
}
auto selection = cql3::selection::selection::for_columns(base.shared_from_this(), ck_columns);
uint64_t zero = 0;
auto dummy_row = query::result_row_view(ser::qr_row_view{simple_memory_input_stream(reinterpret_cast<const char*>(&zero), 8)});
auto dummy_options = cql3::query_options({ });
// FIXME: pass nullptrs for some of theses dummies
return cql3::expr::is_satisfied_by(
@@ -275,8 +273,7 @@ bool clustering_prefix_matches(const schema& base, const view_info& view, const
cql3::expr::evaluation_inputs{
.partition_key = &exploded_pk,
.clustering_key = &exploded_ck,
.static_row = &dummy_row,
.row = &dummy_row,
.static_and_regular_columns = nullptr, // clustering key only filtering here
.selection = selection.get(),
.options = &dummy_options,
});
@@ -340,6 +337,8 @@ public:
return boost::algorithm::all_of(
_view.select_statement().get_restrictions()->get_non_pk_restriction() | boost::adaptors::map_values,
[&] (auto&& r) {
// FIXME: move outside all_of(). However, crashes.
auto static_and_regular_columns = cql3::expr::get_non_pk_values(*_selection, static_row, &row);
// FIXME: pass dummy_options as nullptr
auto dummy_options = cql3::query_options({});
return cql3::expr::is_satisfied_by(
@@ -347,8 +346,7 @@ public:
cql3::expr::evaluation_inputs{
.partition_key = &_pk,
.clustering_key = &ck,
.static_row = &static_row,
.row = &row,
.static_and_regular_columns = &static_and_regular_columns,
.selection = _selection.get(),
.options = &dummy_options,
});