From 620df7103fd5aa309495ae9fa1cb2bcafa5ad529 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Tue, 22 Oct 2024 23:47:15 +0300 Subject: [PATCH] cql3: statement_restrictions: do not pass view schema back and forth For indexed queries, statement_restrictions calculates _view_schema, which is passed via get_view_schema() to indexed_select_statement(), which passes it right back to statement_restrictions via one of three functions to calculate clustering ranges. Avoid the back-and-forth and use the stored value. Using a different value would be broken. This change allows unifying the signatures of the four functions that get clustering ranges. --- cql3/restrictions/statement_restrictions.cc | 18 +++++++----------- cql3/restrictions/statement_restrictions.hh | 6 +++--- cql3/statements/select_statement.cc | 6 +++--- test/boost/statement_restrictions_test.cc | 7 +++---- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index cd49fa26c3..96cccabcfb 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -2894,27 +2894,24 @@ unsigned int statement_restrictions::num_clustering_prefix_columns_that_need_not } std::vector statement_restrictions::get_global_index_clustering_ranges( - const query_options& options, - const schema& idx_tbl_schema) const { + const query_options& options) const { if (!_idx_tbl_ck_prefix) { on_internal_error( rlogger, "statement_restrictions::get_global_index_clustering_ranges called with unprepared index"); } // Multi column restrictions are not added to _idx_tbl_ck_prefix, they are handled later by filtering. - return get_single_column_clustering_bounds(options, idx_tbl_schema, *_idx_tbl_ck_prefix); + return get_single_column_clustering_bounds(options, *_view_schema, *_idx_tbl_ck_prefix); } std::vector statement_restrictions::get_global_index_token_clustering_ranges( - const query_options& options, - const schema& idx_tbl_schema -) const { + const query_options& options) const { if (!_idx_tbl_ck_prefix.has_value()) { on_internal_error( rlogger, "statement_restrictions::get_global_index_token_clustering_ranges called with unprepared index"); } - const column_definition& token_column = idx_tbl_schema.clustering_column_at(0); + const column_definition& token_column = _view_schema->clustering_column_at(0); // In old indexes the token column was of type blob. // This causes problems with sorting and must be handled separately. @@ -2922,19 +2919,18 @@ std::vector statement_restrictions::get_global_index_to return get_index_v1_token_range_clustering_bounds(options, token_column, _idx_tbl_ck_prefix->at(0)); } - return get_single_column_clustering_bounds(options, idx_tbl_schema, *_idx_tbl_ck_prefix); + return get_single_column_clustering_bounds(options, *_view_schema, *_idx_tbl_ck_prefix); } std::vector statement_restrictions::get_local_index_clustering_ranges( - const query_options& options, - const schema& idx_tbl_schema) const { + const query_options& options) const { if (!_idx_tbl_ck_prefix.has_value()) { on_internal_error( rlogger, "statement_restrictions::get_local_index_clustering_ranges called with unprepared index"); } // Multi column restrictions are not added to _idx_tbl_ck_prefix, they are handled later by filtering. - return get_single_column_clustering_bounds(options, idx_tbl_schema, *_idx_tbl_ck_prefix); + return get_single_column_clustering_bounds(options, *_view_schema, *_idx_tbl_ck_prefix); } sstring statement_restrictions::to_string() const { diff --git a/cql3/restrictions/statement_restrictions.hh b/cql3/restrictions/statement_restrictions.hh index c15c9bf877..051b084021 100644 --- a/cql3/restrictions/statement_restrictions.hh +++ b/cql3/restrictions/statement_restrictions.hh @@ -471,15 +471,15 @@ private: public: /// Calculates clustering ranges for querying a global-index table. std::vector get_global_index_clustering_ranges( - const query_options& options, const schema& idx_tbl_schema) const; + const query_options& options) const; /// Calculates clustering ranges for querying a global-index table for queries with token restrictions present. std::vector get_global_index_token_clustering_ranges( - const query_options& options, const schema& idx_tbl_schema) const; + const query_options& options) const; /// Calculates clustering ranges for querying a local-index table. std::vector get_local_index_clustering_ranges( - const query_options& options, const schema& idx_tbl_schema) const; + const query_options& options) const; sstring to_string() const; diff --git a/cql3/statements/select_statement.cc b/cql3/statements/select_statement.cc index da8a068e55..ecc71b4161 100644 --- a/cql3/statements/select_statement.cc +++ b/cql3/statements/select_statement.cc @@ -1374,11 +1374,11 @@ query::partition_slice view_indexed_table_select_statement::get_partition_slice_ // Only EQ restrictions on base partition key can be used in an index view query if (pk_restrictions_is_single && _restrictions->partition_key_restrictions_is_all_eq()) { partition_slice_builder.with_ranges( - _restrictions->get_global_index_clustering_ranges(options, *_view_schema)); + _restrictions->get_global_index_clustering_ranges(options)); } else if (_restrictions->has_token_restrictions()) { // Restrictions like token(p1, p2) < 0 have all partition key components restricted, but require special handling. partition_slice_builder.with_ranges( - _restrictions->get_global_index_token_clustering_ranges(options, *_view_schema)); + _restrictions->get_global_index_token_clustering_ranges(options)); } } @@ -1389,7 +1389,7 @@ query::partition_slice view_indexed_table_select_statement::get_partition_slice_ partition_slice_builder partition_slice_builder{*_view_schema}; partition_slice_builder.with_ranges( - _restrictions->get_local_index_clustering_ranges(options, *_view_schema)); + _restrictions->get_local_index_clustering_ranges(options)); return partition_slice_builder.build(); } diff --git a/test/boost/statement_restrictions_test.cc b/test/boost/statement_restrictions_test.cc index 977f7b8dfb..0230f504ee 100644 --- a/test/boost/statement_restrictions_test.cc +++ b/test/boost/statement_restrictions_test.cc @@ -1106,14 +1106,13 @@ SEASTAR_TEST_CASE(combinatorial_restrictions) { // --- Index table range APIs --- if (uses_idx) { bool is_local_idx = (expected_idx == "comb_v3_local"); - const auto& view_schema = *sr->get_view_schema(); if (is_local_idx) { // --- get_local_index_clustering_ranges --- // Local index CK prefix = (indexed_col, base_ck1, ...). // The indexed column is always EQ (1 value); CK IN values // multiply via the base CK appended to the prefix. - auto local_ranges = sr->get_local_index_clustering_ranges(query_options({}), view_schema); + auto local_ranges = sr->get_local_index_clustering_ranges(query_options({})); unsigned expected_local = 1 * idx_range_multiplier; BOOST_CHECK_MESSAGE(!local_ranges.empty(), ctx_msg("get_local_index_clustering_ranges should not be empty")); @@ -1125,7 +1124,7 @@ SEASTAR_TEST_CASE(combinatorial_restrictions) { // Global index CK prefix = (token, pk1, pk2, ...base CK...). // With full PK: CK IN values expand the prefix. // Without full PK: prefix is empty → 1 open-ended range. - auto global_ranges = sr->get_global_index_clustering_ranges(query_options({}), view_schema); + auto global_ranges = sr->get_global_index_clustering_ranges(query_options({})); BOOST_CHECK_MESSAGE(!global_ranges.empty(), ctx_msg("get_global_index_clustering_ranges should not be empty")); if (full_pk) { @@ -1148,7 +1147,7 @@ SEASTAR_TEST_CASE(combinatorial_restrictions) { // long_type, so this dispatches to the same // get_single_column_clustering_bounds as // get_global_index_clustering_ranges. - auto token_ranges = sr->get_global_index_token_clustering_ranges(query_options({}), view_schema); + auto token_ranges = sr->get_global_index_token_clustering_ranges(query_options({})); BOOST_CHECK_MESSAGE(token_ranges.size() == global_ranges.size(), ctx_msg(fmt::format( "get_global_index_token_clustering_ranges: {} ranges, want {} (same as global)",