From fa6f239cc7420fcc26368e40930883cef3422ca2 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Sun, 15 Mar 2026 14:33:49 +0200 Subject: [PATCH] cql3: statement_restrictions: add predicate-based index support checking Add `op` and `is_subscript` fields to `struct predicate` and populate them in all predicate creation sites in `to_predicates()`. These fields record the binary operator and whether the LHS is a subscript (map element access), which are the two pieces of information needed to query index support. Add `is_predicate_supported_by()` which mirrors `is_supported_by_helper()` but operates on a single predicate's fields instead of walking the expression tree. Add a predicate-vector overload of `index_supports_some_column()` and use it in the constructor to replace expression-based index support checks for single-column partition key, clustering key, and non-primary-key restrictions. The multi-column clustering key case still uses the existing expression-based path. --- cql3/restrictions/statement_restrictions.cc | 91 ++++++++++++++++++++- cql3/restrictions/statement_restrictions.hh | 2 + 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index 50475485b8..2ac4a096e8 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -281,6 +281,8 @@ make_conjunction(predicate a, predicate b) { .is_upper_bound = false, // A conjunction has no single direction. .is_lower_bound = false, // A conjunction has no single direction. .order = a.order, // Both predicates are on the same column, so comparison order must agree. + .op = std::nullopt, // A conjunction has no single operator. + .is_subscript = a.is_subscript, // Both predicates are on the same target, so they agree on subscript-ness. }; } @@ -382,6 +384,7 @@ to_predicates( .filter = oper, .on = on_column{col.col}, .is_not_null_single_column = is_null_constant(oper.rhs), + .op = oper.op, }); } if (is_compare(oper.op)) { @@ -403,6 +406,7 @@ to_predicates( .is_upper_bound = (oper.op == oper_t::LT || oper.op == oper_t::LTE), .is_lower_bound = (oper.op == oper_t::GT || oper.op == oper_t::GTE), .order = oper.order, + .op = oper.op, }); } else if (oper.op == oper_t::IN) { auto solve = [oper, type, cdef] (const query_options& options) { @@ -415,6 +419,7 @@ to_predicates( .is_singleton = false, .is_in = true, .order = oper.order, + .op = oper.op, }); } else if (oper.op == oper_t::CONTAINS || oper.op == oper_t::CONTAINS_KEY) { auto solve = [oper] (const query_options& options) { @@ -430,6 +435,7 @@ to_predicates( .on = on_column{col.col}, .is_singleton = false, .order = oper.order, + .op = oper.op, }); } return cannot_solve_on_column(oper, col.col); @@ -459,6 +465,8 @@ to_predicates( .is_singleton = true, .equality = true, .order = oper.order, + .op = oper.op, + .is_subscript = true, }); } return cannot_solve_on_column(oper, col.col); @@ -498,6 +506,7 @@ to_predicates( .is_upper_bound = (oper.op == oper_t::LT || oper.op == oper_t::LTE), .is_lower_bound = (oper.op == oper_t::GT || oper.op == oper_t::GTE), .order = oper.order, + .op = oper.op, }); }, [&] (const function_call& token_fun_call) -> std::vector { @@ -542,6 +551,7 @@ to_predicates( .is_upper_bound = (oper.op == oper_t::LT || oper.op == oper_t::LTE), .is_lower_bound = (oper.op == oper_t::GT || oper.op == oper_t::GTE), .order = oper.order, + .op = oper.op, }); }, [&] (const binary_operator&) -> std::vector { @@ -771,6 +781,67 @@ bool is_supported_by(const expression& expr, const secondary_index::index& idx) return s != secondary_index::index::supports_expression_v::from_bool(false); } +// Like is_supported_by_helper, but operates on a single predicate instead of walking +// an expression tree. Returns how an index supports this predicate: UsualYes, CollectionYes, or No. +static secondary_index::index::supports_expression_v +is_predicate_supported_by(const predicate& pred, const secondary_index::index& idx) { + using ret_t = secondary_index::index::supports_expression_v; + if (!pred.op) { + return ret_t::from_bool(false); + } + return std::visit(overloaded_functor{ + [&] (const on_column& oc) -> ret_t { + if (pred.is_subscript) { + return idx.supports_subscript_expression(*oc.column, *pred.op); + } + return idx.supports_expression(*oc.column, *pred.op); + }, + [&] (const on_clustering_key_prefix& ocp) -> ret_t { + // Single-element tuple_constructor: treat like a single column + if (ocp.columns.size() == 1) { + return idx.supports_expression(*ocp.columns[0], *pred.op); + } + // Multi-element tuple: index cannot avoid filtering + return ret_t::from_bool(false); + }, + [&] (const on_partition_key_token&) -> ret_t { + return ret_t::from_bool(false); + }, + [&] (const on_row&) -> ret_t { + return ret_t::from_bool(false); + }, + }, pred.on); +} + +using single_column_predicate_vectors = std::map, schema_pos_column_definition_comparator>; + +// Like index_supports_some_column, but operates on per-column predicate vectors +// instead of walking per-column expression trees. +static bool index_supports_some_column( + const single_column_predicate_vectors& per_column_predicates, + const secondary_index::secondary_index_manager& index_manager, + allow_local_index allow_local) { + using namespace secondary_index; + for (auto& [col, preds] : per_column_predicates) { + for (const auto& idx : index_manager.list_indexes()) { + if (!allow_local && idx.metadata().local()) { + continue; + } + // AND all predicate results for this column-index pair, mirroring the + // conjunction logic in is_supported_by_helper. Initialize with the + // first predicate (not from_bool(true)) to preserve CollectionYes. + auto result = is_predicate_supported_by(preds[0], idx); + for (size_t i = 1; i < preds.size(); ++i) { + result = result && is_predicate_supported_by(preds[i], idx); + } + if (result) { + return true; + } + } + } + return false; +} + bool has_supporting_index( const expression& expr, @@ -1079,6 +1150,9 @@ statement_restrictions::statement_restrictions(private_tag, std::unordered_map pk_range_preds; std::vector mc_ck_preds; std::unordered_map sc_ck_preds; + single_column_predicate_vectors sc_pk_pred_vectors; + single_column_predicate_vectors sc_ck_pred_vectors; + single_column_predicate_vectors sc_nonpk_pred_vectors; for (auto& pred : predicates) { if (pred.is_not_null_single_column) { auto* col = require_on_single_column(pred); @@ -1189,6 +1263,7 @@ statement_restrictions::statement_restrictions(private_tag, auto [it, inserted] = _single_column_partition_key_restrictions.try_emplace(def, expr::conjunction{}); it->second = expr::make_conjunction(std::move(it->second), pred.filter); } + sc_pk_pred_vectors[def].push_back(pred); if (pred.equality || pred.is_in) { auto [it, inserted] = pk_range_preds.try_emplace(def, pred); if (!inserted) { @@ -1225,6 +1300,7 @@ statement_restrictions::statement_restrictions(private_tag, auto [it, inserted] = _single_column_clustering_key_restrictions.try_emplace(def, expr::conjunction{}); it->second = expr::make_conjunction(std::move(it->second), pred.filter); } + sc_ck_pred_vectors[def].push_back(pred); { auto [it, inserted] = sc_ck_preds.try_emplace(def, pred); if (!inserted) { @@ -1243,6 +1319,7 @@ statement_restrictions::statement_restrictions(private_tag, auto [it, inserted] = _single_column_nonprimary_key_restrictions.try_emplace(def, expr::conjunction{}); it->second = expr::make_conjunction(std::move(it->second), pred.filter); } + sc_nonpk_pred_vectors[def].push_back(pred); } } else { throw exceptions::invalid_request_exception(format("Unhandled restriction: {}", pred.filter)); @@ -1289,11 +1366,17 @@ statement_restrictions::statement_restrictions(private_tag, const expr::allow_local_index allow_local( !has_partition_key_unrestricted_components() && partition_key_restrictions_is_all_eq()); - _has_queriable_ck_index = clustering_columns_restrictions_have_supporting_index(sim, allow_local) + if (!_has_multi_column) { + _has_queriable_ck_index = index_supports_some_column(sc_ck_pred_vectors, sim, allow_local) + && !type.is_delete(); + } else { + _has_queriable_ck_index = clustering_columns_restrictions_have_supporting_index(sim, allow_local) + && !type.is_delete(); + } + _has_queriable_pk_index = !has_token + && index_supports_some_column(sc_pk_pred_vectors, sim, allow_local) && !type.is_delete(); - _has_queriable_pk_index = parition_key_restrictions_have_supporting_index(sim, allow_local) - && !type.is_delete(); - _has_queriable_regular_index = index_supports_some_column(_single_column_nonprimary_key_restrictions, sim, allow_local) + _has_queriable_regular_index = index_supports_some_column(sc_nonpk_pred_vectors, sim, allow_local) && !type.is_delete(); } else { _has_queriable_ck_index = false; diff --git a/cql3/restrictions/statement_restrictions.hh b/cql3/restrictions/statement_restrictions.hh index a88675266c..93f038b94b 100644 --- a/cql3/restrictions/statement_restrictions.hh +++ b/cql3/restrictions/statement_restrictions.hh @@ -86,6 +86,8 @@ struct predicate { bool is_upper_bound = false; // operator is LT/LTE bool is_lower_bound = false; // operator is GT/GTE expr::comparison_order order = expr::comparison_order::cql; + std::optional op; // the binary operator, if any + bool is_subscript = false; // whether the LHS is a subscript (map element access) }; ///In some cases checking if columns have indexes is undesired of even