From 92a43557dc352d9b31caa480e86cc37fed4f30db Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Fri, 28 Mar 2025 21:15:55 +0300 Subject: [PATCH] cql3: statement_restrictions: split _where to boolean factors in preparation for predicates conversion Expressions are a tree-like structure so a single expression is sufficient (for complicated ones, a conjunction is used), but predicates are flat. Prepare for conversion to predicates by storing the expressions that will correspond to predicates, namely the boolean factors of the WHERE clause. --- cql3/restrictions/statement_restrictions.cc | 44 +++++++++++---------- cql3/restrictions/statement_restrictions.hh | 4 +- test/boost/statement_restrictions_test.cc | 18 ++++----- 3 files changed, 34 insertions(+), 32 deletions(-) diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index 130d30c22e..c1824057f1 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -667,7 +667,7 @@ bool has_eq_restriction_on_column(const column_definition& column, const express return eq_restriction_search_res != nullptr; } -std::vector extract_single_column_restrictions_for_column(const expression& expr, +std::vector extract_single_column_restrictions_for_column(std::span exprs, const column_definition& column) { struct visitor { std::vector restrictions; @@ -725,7 +725,9 @@ std::vector extract_single_column_restrictions_for_column(const expr .current_binary_operator = nullptr, }; - expr::visit(v, expr); + for (auto& e : exprs) { + expr::visit(v, e); + } return std::move(v.restrictions); } @@ -784,7 +786,7 @@ single_column_restrictions_map get_single_column_restrictions_map(const expressi std::vector sorted_defs = get_sorted_column_defs(e); for (const column_definition* cdef : sorted_defs) { expression col_restrictions = conjunction { - .children = extract_single_column_restrictions_for_column(e, *cdef) + .children = extract_single_column_restrictions_for_column(std::span(&e, 1), *cdef) }; result.emplace(cdef, std::move(col_restrictions)); } @@ -868,7 +870,7 @@ void with_current_binary_operator( /// Every token, or if no tokens, an EQ/IN of every single PK column. static partition_range_restrictions extract_partition_range( - const expr::expression& where_clause, schema_ptr schema) { + std::span where_clause, schema_ptr schema) { using namespace expr; struct extract_partition_range_visitor { schema_ptr table_schema; @@ -975,7 +977,10 @@ static partition_range_restrictions extract_partition_range( .table_schema = schema }; - expr::visit(v, where_clause); + for (auto& e : where_clause) { + expr::visit(v, e); + } + if (v.tokens) { return token_range_restrictions{ .token_restrictions = predicate{ @@ -999,7 +1004,7 @@ static partition_range_restrictions extract_partition_range( /// boundaries of any clustering slice that can possibly meet where_clause. This vector can be calculated before /// binding expression markers, since LHS and operator are always known. static std::vector extract_clustering_prefix_restrictions( - const expr::expression& where_clause, schema_ptr schema) { + std::span where_clause, schema_ptr schema) { using namespace expr; /// Collects all clustering-column restrictions from an expression. Presumes the expression only uses @@ -1133,7 +1138,9 @@ static std::vector extract_clustering_prefix_restrictions( .table_schema = schema }; - expr::visit(v, where_clause); + for (auto& e : where_clause) { + expr::visit(v, e); + } if (!v.multi.empty()) { return std::move(v.multi); @@ -1332,10 +1339,10 @@ statement_restrictions::statement_restrictions(private_tag, } if (prepared_restriction.op != expr::oper_t::IS_NOT) { - _where = _where.has_value() ? make_conjunction(std::move(*_where), prepared_restriction) : prepared_restriction; + _where.push_back(prepared_restriction); } } - if (_where.has_value()) { + if (!_where.empty()) { if (!has_token_restrictions()) { _single_column_partition_key_restrictions = get_single_column_restrictions_map(_partition_key_restrictions); } @@ -1343,8 +1350,8 @@ statement_restrictions::statement_restrictions(private_tag, _single_column_clustering_key_restrictions = get_single_column_restrictions_map(_clustering_columns_restrictions); } _single_column_nonprimary_key_restrictions = get_single_column_restrictions_map(_nonprimary_key_restrictions); - _clustering_prefix_restrictions = extract_clustering_prefix_restrictions(*_where, _schema); - _partition_range_restrictions = extract_partition_range(*_where, _schema); + _clustering_prefix_restrictions = extract_clustering_prefix_restrictions(_where, _schema); + _partition_range_restrictions = extract_partition_range(_where, _schema); } _has_multi_column = find_binop(_clustering_columns_restrictions, is_multi_column); if (_check_indexes) { @@ -1582,7 +1589,7 @@ const std::vector& statement_restrictions::index_restrictions( } bool statement_restrictions::is_empty() const { - return !_where.has_value(); + return _where.empty(); } // Current score table: @@ -1620,7 +1627,7 @@ std::pair, expr::expression> statement_res expr::for_each_expression(restriction, [&](const expr::column_value& cval) { auto& cdef = cval.col; expr::expression col_restrictions = expr::conjunction { - .children = extract_single_column_restrictions_for_column(restriction, *cdef) + .children = extract_single_column_restrictions_for_column(std::span(&restriction, 1), *cdef) }; for (const auto& index : sim.list_indexes()) { if (cdef->name_as_text() == index.target_column() && @@ -1642,11 +1649,8 @@ statement_restrictions::find_idx(const secondary_index::secondary_index_manager& } bool statement_restrictions::has_eq_restriction_on_column(const column_definition& column) const { - if (!_where.has_value()) { - return false; - } - - return restrictions::has_eq_restriction_on_column(column, *_where); + return std::ranges::any_of(_where, + std::bind_front(restrictions::has_eq_restriction_on_column, std::ref(column))); } std::vector statement_restrictions::get_column_defs_for_filtering(data_dictionary::database db) const { @@ -2932,7 +2936,7 @@ void statement_restrictions::prepare_indexed_local(const schema& idx_tbl_schema) // Find index column restrictions in the WHERE clause std::vector idx_col_restrictions = - extract_single_column_restrictions_for_column(*_where, indexed_column_base_schema); + extract_single_column_restrictions_for_column(_where, indexed_column_base_schema); expr::expression idx_col_restriction_expr = expr::expression(expr::conjunction{std::move(idx_col_restrictions)}); // Translate the restriction to use column from the index schema and add it @@ -3082,7 +3086,7 @@ statement_restrictions::value_for_index_partition_key(const query_options& optio } sstring statement_restrictions::to_string() const { - return _where ? expr::to_string(*_where) : ""; + return !_where.empty() ? expr::to_string(expr::conjunction{.children = _where}) : ""; } static void validate_primary_key_restrictions(const query_options& options, std::ranges::range auto&& restrictions) { diff --git a/cql3/restrictions/statement_restrictions.hh b/cql3/restrictions/statement_restrictions.hh index 55518a9878..37b96f31ac 100644 --- a/cql3/restrictions/statement_restrictions.hh +++ b/cql3/restrictions/statement_restrictions.hh @@ -161,7 +161,7 @@ private: bool _has_queriable_regular_index = false, _has_queriable_pk_index = false, _has_queriable_ck_index = false; bool _has_multi_column; ///< True iff _clustering_columns_restrictions has a multi-column restriction. - std::optional _where; ///< The entire WHERE clause. + std::vector _where; ///< The entire WHERE clause (factorized). /// Parts of _where defining the clustering slice. /// @@ -532,7 +532,7 @@ shared_ptr make_trivial_statement_restrictions( // Does not include token() restrictions. // Does not include boolean constant restrictions. // For example "WHERE c = 1 AND (a, c) = (2, 1) AND token(p) < 2 AND FALSE" will return {"c = 1"}. -std::vector extract_single_column_restrictions_for_column(const expr::expression&, const column_definition&); +std::vector extract_single_column_restrictions_for_column(std::span, const column_definition&); // Checks whether this expression is empty - doesn't restrict anything diff --git a/test/boost/statement_restrictions_test.cc b/test/boost/statement_restrictions_test.cc index 0230f504ee..c50cd63dea 100644 --- a/test/boost/statement_restrictions_test.cc +++ b/test/boost/statement_restrictions_test.cc @@ -1211,7 +1211,7 @@ BOOST_AUTO_TEST_CASE(expression_extract_column_restrictions) { column_definition col_r3 = make_column("r3", column_kind::regular_column, 2); // Empty input test - assert_expr_vec_eq(cql3::restrictions::extract_single_column_restrictions_for_column(conjunction{}, col_pk1), {}); + assert_expr_vec_eq(cql3::restrictions::extract_single_column_restrictions_for_column({}, col_pk1), {}); // BIG_WHERE test // big_where contains: @@ -1293,27 +1293,25 @@ BOOST_AUTO_TEST_CASE(expression_extract_column_restrictions) { big_where.push_back(pk2_expr); big_where.push_back(pk1_pk2_expr); - expression big_where_expr = conjunction{std::move(big_where)}; - - assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where_expr, col_pk1), + assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where, col_pk1), {pk1_restriction}); - assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where_expr, col_pk2), + assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where, col_pk2), {pk2_restriction, pk2_restriction2}); - assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where_expr, col_ck1), + assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where, col_ck1), {ck1_restriction}); - assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where_expr, col_ck2), + assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where, col_ck2), {ck2_restriction}); - assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where_expr, col_r1), + assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where, col_r1), {r1_restriction, r1_restriction2, r1_restriction3}); - assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where_expr, col_r2), + assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where, col_r2), {r2_restriction}); - assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where_expr, col_r3), + assert_expr_vec_eq(restrictions::extract_single_column_restrictions_for_column(big_where, col_r3), {}); }