From e7db3def4f9376afca3f3563a0c7cd50f2ae471e Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Tue, 28 Sep 2021 19:53:24 +0300 Subject: [PATCH] cql3: expr: introduce expr::visit, replacing std::visit The new expr::visit() is just a wrapper around std::visit(), but has better constraints. A call to expr::visit() with a visitor that misses an overload will produce an error message that points at the missing type. This is done using the new invocable_on_expression concept. Note it lists the expression types one by one rather than using template magic, since otherwise we won't get the nice messages. Later, we will change the implementation when expression becomes our own type rather than std::variant. Call sites are updated. --- cql3/expr/expression.cc | 26 +++++++++---------- cql3/expr/expression.hh | 28 +++++++++++++++++++-- cql3/expr/term_expr.cc | 6 ++--- cql3/restrictions/statement_restrictions.cc | 16 ++++++------ cql3/selection/selectable.cc | 4 +-- cql3/statements/create_view_statement.cc | 2 +- 6 files changed, 53 insertions(+), 29 deletions(-) diff --git a/cql3/expr/expression.cc b/cql3/expr/expression.cc index 90bdd3a6d3..f79f20daba 100644 --- a/cql3/expr/expression.cc +++ b/cql3/expr/expression.cc @@ -91,7 +91,7 @@ namespace { using children_t = std::vector; // conjunction's children. children_t explode_conjunction(expression e) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [] (const conjunction& c) { return std::move(c.children); }, [&] (const auto&) { return children_t{std::move(e)}; }, }, e); @@ -461,7 +461,7 @@ value_set intersection(value_set a, value_set b, const abstract_type* type) { } bool is_satisfied_by(const binary_operator& opr, const column_value_eval_bag& bag) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const column_value& col) { if (opr.op == oper_t::EQ) { return equal(*opr.rhs, col, bag); @@ -541,7 +541,7 @@ bool is_satisfied_by(const binary_operator& opr, const column_value_eval_bag& ba } bool is_satisfied_by(const expression& restr, const column_value_eval_bag& bag) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [] (const constant& constant_val) { std::optional bool_val = get_bool_value(constant_val); if (bool_val.has_value()) { @@ -686,7 +686,7 @@ nonwrapping_range to_range(oper_t op, const clustering_ke value_set possible_lhs_values(const column_definition* cdef, const expression& expr, const query_options& options) { const auto type = cdef ? get_value_comparator(cdef) : long_type.get(); - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [] (const constant& constant_val) { std::optional bool_val = get_bool_value(constant_val); if (bool_val.has_value()) { @@ -704,7 +704,7 @@ value_set possible_lhs_values(const column_definition* cdef, const expression& e }); }, [&] (const binary_operator& oper) -> value_set { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const column_value& col) -> value_set { if (!cdef || cdef != col.col) { return unbounded_value_set; @@ -875,12 +875,12 @@ nonwrapping_range to_range(const value_set& s) { bool is_supported_by(const expression& expr, const secondary_index::index& idx) { using std::placeholders::_1; - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const conjunction& conj) { return boost::algorithm::all_of(conj.children, std::bind(is_supported_by, _1, idx)); }, [&] (const binary_operator& oper) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const column_value& col) { return idx.supports_expression(*col.col, oper.op); }, @@ -960,7 +960,7 @@ std::ostream& operator<<(std::ostream& os, const column_value& cv) { } std::ostream& operator<<(std::ostream& os, const expression& expr) { - std::visit(overloaded_functor{ + expr::visit(overloaded_functor{ [&] (const constant& v) { os << v.value.to_view(); }, [&] (const conjunction& conj) { fmt::print(os, "({})", fmt::join(conj.children, ") AND (")); }, [&] (const binary_operator& opr) { @@ -1107,7 +1107,7 @@ expression search_and_replace(const expression& e, if (replace_result) { return std::move(*replace_result); } else { - return std::visit( + return expr::visit( overloaded_functor{ [&] (const conjunction& conj) -> expression { return conjunction{ @@ -1206,7 +1206,7 @@ std::vector extract_single_column_restrictions_for_column(const expr void operator()(const conjunction& conj) { for (const expression& child : conj.children) { - std::visit(*this, child); + expr::visit(*this, child); } } @@ -1217,7 +1217,7 @@ std::vector extract_single_column_restrictions_for_column(const expr } current_binary_operator = &oper; - std::visit(*this, *oper.lhs); + expr::visit(*this, *oper.lhs); current_binary_operator = nullptr; } @@ -1247,7 +1247,7 @@ std::vector extract_single_column_restrictions_for_column(const expr .current_binary_operator = nullptr, }; - std::visit(v, expr); + expr::visit(v, expr); return std::move(v.restrictions); } @@ -1340,7 +1340,7 @@ cql3::raw_value_view evaluate_to_raw_view(term& term_ref, const query_options& o } constant evaluate(const expression& e, const query_options& options) { - return std::visit(overloaded_functor { + return expr::visit(overloaded_functor { [](const binary_operator&) -> constant { on_internal_error(expr_logger, "Can't evaluate a binary_operator"); }, diff --git a/cql3/expr/expression.hh b/cql3/expr/expression.hh index ca7f365574..dd625b8b0a 100644 --- a/cql3/expr/expression.hh +++ b/cql3/expr/expression.hh @@ -98,6 +98,30 @@ using expression = std::variant concept ExpressionElement = utils::VariantElement; +template +concept invocable_on_expression + = std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + && std::invocable + ; + +auto visit(invocable_on_expression auto&& visitor, const expression& expr) { + return std::visit(visitor, expr); +} + // An expression that doesn't contain subexpressions template concept LeafExpression @@ -345,7 +369,7 @@ extern std::ostream& operator<<(std::ostream&, const expression&); template requires std::regular_invocable const binary_operator* find_atom(const expression& e, Fn f) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const binary_operator& op) { return f(op) ? &op : nullptr; }, [] (const constant&) -> const binary_operator* { return nullptr; }, [&] (const conjunction& conj) -> const binary_operator* { @@ -414,7 +438,7 @@ const binary_operator* find_atom(const expression& e, Fn f) { template requires std::regular_invocable size_t count_if(const expression& e, Fn f) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const binary_operator& op) -> size_t { return f(op) ? 1 : 0; }, [&] (const conjunction& conj) { return std::accumulate(conj.children.cbegin(), conj.children.cend(), size_t{0}, diff --git a/cql3/expr/term_expr.cc b/cql3/expr/term_expr.cc index f008afb9a4..bc7963ccd6 100644 --- a/cql3/expr/term_expr.cc +++ b/cql3/expr/term_expr.cc @@ -784,7 +784,7 @@ cast_prepare_term(const cast& c, database& db, const sstring& keyspace, lw_share ::shared_ptr prepare_term(const expression& expr, database& db, const sstring& keyspace, lw_shared_ptr receiver) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [] (const constant&) -> ::shared_ptr { on_internal_error(expr_logger, "Can't prepare constant_value, it should not appear in parser output"); }, @@ -849,7 +849,7 @@ prepare_term(const expression& expr, database& db, const sstring& keyspace, lw_s ::shared_ptr prepare_term_multi_column(const expression& expr, database& db, const sstring& keyspace, const std::vector>& receivers) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const bind_variable& bv) -> ::shared_ptr { switch (bv.shape) { case expr::bind_variable::shape_type::scalar: on_internal_error(expr_logger, "prepare_term_multi_column(bind_variable(scalar))"); @@ -871,7 +871,7 @@ prepare_term_multi_column(const expression& expr, database& db, const sstring& k assignment_testable::test_result test_assignment(const expression& expr, database& db, const sstring& keyspace, const column_specification& receiver) { using test_result = assignment_testable::test_result; - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const constant&) -> test_result { // constants shouldn't appear in parser output, only untyped_constants on_internal_error(expr_logger, "constants are not yet reachable via term_raw_expr::test_assignment()"); diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index 08f7ff8228..61c96bf122 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -164,7 +164,7 @@ static std::vector extract_partition_range( const binary_operator* current_binary_operator = nullptr; void operator()(const conjunction& c) { - std::ranges::for_each(c.children, [this] (const expression& child) { std::visit(*this, child); }); + std::ranges::for_each(c.children, [this] (const expression& child) { expr::visit(*this, child); }); } void operator()(const binary_operator& b) { @@ -172,7 +172,7 @@ static std::vector extract_partition_range( throw std::logic_error("Nested binary operators are not supported"); } current_binary_operator = &b; - std::visit(*this, *b.lhs); + expr::visit(*this, *b.lhs); current_binary_operator = nullptr; } @@ -246,7 +246,7 @@ static std::vector extract_partition_range( on_internal_error(rlogger, "extract_partition_range(usertype_constructor)"); } } v; - std::visit(v, where_clause); + expr::visit(v, where_clause); if (v.tokens) { return {std::move(*v.tokens)}; } @@ -273,7 +273,7 @@ static std::vector extract_clustering_prefix_restrictions( const binary_operator* current_binary_operator = nullptr; void operator()(const conjunction& c) { - std::ranges::for_each(c.children, [this] (const expression& child) { std::visit(*this, child); }); + std::ranges::for_each(c.children, [this] (const expression& child) { expr::visit(*this, child); }); } void operator()(const binary_operator& b) { @@ -281,7 +281,7 @@ static std::vector extract_clustering_prefix_restrictions( throw std::logic_error("Nested binary operators are not supported"); } current_binary_operator = &b; - std::visit(*this, *b.lhs); + expr::visit(*this, *b.lhs); current_binary_operator = nullptr; } @@ -356,7 +356,7 @@ static std::vector extract_clustering_prefix_restrictions( on_internal_error(rlogger, "extract_clustering_prefix_restrictions(usertype_constructor)"); } } v; - std::visit(v, where_clause); + expr::visit(v, where_clause); if (!v.multi.empty()) { return move(v.multi); @@ -996,7 +996,7 @@ struct multi_column_range_accumulator { } void operator()(const conjunction& c) { - std::ranges::for_each(c.children, [this] (const expression& child) { std::visit(*this, child); }); + std::ranges::for_each(c.children, [this] (const expression& child) { expr::visit(*this, child); }); } void operator()(const constant& v) { @@ -1103,7 +1103,7 @@ std::vector get_multi_column_clustering_bounds( const std::vector& multi_column_restrictions) { multi_column_range_accumulator acc{options, schema}; for (const auto& restr : multi_column_restrictions) { - std::visit(acc, restr); + expr::visit(acc, restr); } return acc.ranges; } diff --git a/cql3/selection/selectable.cc b/cql3/selection/selectable.cc index ff1038d7f8..be76f8297f 100644 --- a/cql3/selection/selectable.cc +++ b/cql3/selection/selectable.cc @@ -141,7 +141,7 @@ selectable::with_cast::to_string() const { shared_ptr prepare_selectable(const schema& s, const expr::expression& raw_selectable) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const expr::constant&) -> shared_ptr { on_internal_error(slogger, "no way to express SELECT constant in the grammar yet"); }, @@ -228,7 +228,7 @@ prepare_selectable(const schema& s, const expr::expression& raw_selectable) { bool selectable_processes_selection(const expr::expression& raw_selectable) { - return std::visit(overloaded_functor{ + return expr::visit(overloaded_functor{ [&] (const expr::constant&) -> bool { on_internal_error(slogger, "no way to express SELECT constant in the grammar yet"); }, diff --git a/cql3/statements/create_view_statement.cc b/cql3/statements/create_view_statement.cc index 097f3f543e..d87715c50a 100644 --- a/cql3/statements/create_view_statement.cc +++ b/cql3/statements/create_view_statement.cc @@ -204,7 +204,7 @@ future> create_view_statement::a auto& selectable = selector->selectable_; shared_ptr identifier; - std::visit(overloaded_functor{ + expr::visit(overloaded_functor{ [&] (const expr::unresolved_identifier& ui) { identifier = ui.ident; }, [] (const auto& default_case) -> void { throw exceptions::invalid_request_exception(format("Cannot use general expressions when defining a materialized view")); }, }, selectable);