From 837fde84b10f4cb74bc2e4f5e7204f529dcce387 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 20 Apr 2023 09:26:55 +0300 Subject: [PATCH] view: Carry data_dictionary arg through standalone helpers There's a bunch of functions in view.{hh|cc} that don't belong to any class and perform view-related claculations for view updates. Lots of them eventually call view_info::select_statement() which will later need the dictionary. By now all those methods' callers have data dictionary at hand and can share it via argument. Signed-off-by: Pavel Emelyanov --- db/view/view.cc | 31 ++++++++++++++++--------------- db/view/view.hh | 9 +++++---- replica/table.cc | 4 ++-- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/db/view/view.cc b/db/view/view.cc index 20e0842800..c2ebf8385a 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -273,7 +273,7 @@ void stats::register_stats() { }); } -bool partition_key_matches(const schema& base, const view_info& view, const dht::decorated_key& key) { +bool partition_key_matches(data_dictionary::database db, const schema& base, const view_info& view, const dht::decorated_key& key) { const cql3::expr::expression& pk_restrictions = view.select_statement().get_restrictions()->get_partition_key_restrictions(); std::vector exploded_pk = key.key().explode(); std::vector exploded_ck; @@ -298,7 +298,7 @@ bool partition_key_matches(const schema& base, const view_info& view, const dht: }); } -bool clustering_prefix_matches(const schema& base, const view_info& view, const partition_key& key, const clustering_key_prefix& ck) { +bool clustering_prefix_matches(data_dictionary::database db, const schema& base, const view_info& view, const partition_key& key, const clustering_key_prefix& ck) { const cql3::expr::expression& r = view.select_statement().get_restrictions()->get_clustering_columns_restrictions(); std::vector exploded_pk = key.explode(); std::vector exploded_ck = ck.explode(); @@ -322,21 +322,21 @@ bool clustering_prefix_matches(const schema& base, const view_info& view, const }); } -bool may_be_affected_by(const schema& base, const view_info& view, const dht::decorated_key& key, const rows_entry& update) { +bool may_be_affected_by(data_dictionary::database db, const schema& base, const view_info& view, const dht::decorated_key& key, const rows_entry& update) { // We can guarantee that the view won't be affected if: // - the primary key is excluded by the view filter (note that this isn't true of the filter on regular columns: // even if an update don't match a view condition on a regular column, that update can still invalidate a // pre-existing entry) - note that the upper layers should already have checked the partition key; - return clustering_prefix_matches(base, view, key.key(), update.key()); + return clustering_prefix_matches(db, base, view, key.key(), update.key()); } -static bool update_requires_read_before_write(const schema& base, +static bool update_requires_read_before_write(data_dictionary::database db, const schema& base, const std::vector& views, const dht::decorated_key& key, const rows_entry& update) { for (auto&& v : views) { view_info& vf = *v.view->view_info(); - if (may_be_affected_by(base, vf, key, update)) { + if (may_be_affected_by(db, base, vf, key, update)) { return true; } } @@ -447,7 +447,7 @@ public: } }; -bool matches_view_filter(const schema& base, const view_info& view, const partition_key& key, const clustering_or_static_row& update, gc_clock::time_point now) { +bool matches_view_filter(data_dictionary::database db, const schema& base, const view_info& view, const partition_key& key, const clustering_or_static_row& update, gc_clock::time_point now) { // TODO: Filtering is only supported in materialized views which don't support // static rows yet. Skip the whole function if it is a static row update. if (update.is_static_row()) { @@ -464,7 +464,7 @@ bool matches_view_filter(const schema& base, const view_info& view, const partit view_filter_checking_visitor visitor(base, view); query::result_view::consume(result, slice, visitor); - return clustering_prefix_matches(base, view, key, *update.key()) + return clustering_prefix_matches(db, base, view, key, *update.key()) && visitor.matches_view_filter(); } @@ -877,7 +877,7 @@ static void add_cells_to_view(const schema& base, const schema& view, column_kin * This method checks that the base row does match the view filter before applying anything. */ void view_updates::create_entry(data_dictionary::database db, const partition_key& base_key, const clustering_or_static_row& update, gc_clock::time_point now) { - if (!matches_view_filter(*_base, _view_info, base_key, update, now)) { + if (!matches_view_filter(db, *_base, _view_info, base_key, update, now)) { return; } @@ -903,7 +903,7 @@ void view_updates::create_entry(data_dictionary::database db, const partition_ke void view_updates::delete_old_entry(data_dictionary::database db, const partition_key& base_key, const clustering_or_static_row& existing, const clustering_or_static_row& update, gc_clock::time_point now) { // Before deleting an old entry, make sure it was matching the view filter // (otherwise there is nothing to delete) - if (matches_view_filter(*_base, _view_info, base_key, existing, now)) { + if (matches_view_filter(db, *_base, _view_info, base_key, existing, now)) { do_delete_old_entry(base_key, existing, update, now); } } @@ -1023,11 +1023,11 @@ bool view_updates::can_skip_view_updates(const clustering_or_static_row& update, void view_updates::update_entry(data_dictionary::database db, const partition_key& base_key, const clustering_or_static_row& update, const clustering_or_static_row& existing, gc_clock::time_point now) { // While we know update and existing correspond to the same view entry, // they may not match the view filter. - if (!matches_view_filter(*_base, _view_info, base_key, existing, now)) { + if (!matches_view_filter(db, *_base, _view_info, base_key, existing, now)) { create_entry(db, base_key, update, now); return; } - if (!matches_view_filter(*_base, _view_info, base_key, update, now)) { + if (!matches_view_filter(db, *_base, _view_info, base_key, update, now)) { do_delete_old_entry(base_key, existing, update, now); return; } @@ -1459,7 +1459,8 @@ view_update_builder make_view_update_builder( return view_update_builder(std::move(db), base_table, base, std::move(vs), std::move(updates), std::move(existings), now); } -future calculate_affected_clustering_ranges(const schema& base, +future calculate_affected_clustering_ranges(data_dictionary::database db, + const schema& base, const dht::decorated_key& key, const mutation_partition& mp, const std::vector& views) { @@ -1498,7 +1499,7 @@ future calculate_affected_clustering_ranges(const } for (auto&& row : mp.clustered_rows()) { - if (update_requires_read_before_write(base, views, key, row)) { + if (update_requires_read_before_write(db, base, views, key, row)) { row_ranges.emplace_back(row.key()); } co_await coroutine::maybe_yield(); @@ -2307,7 +2308,7 @@ public: inject_failure("view_builder_load_views"); for (auto&& vs : _step.build_status) { if (_step.current_token() >= vs.next_token) { - if (partition_key_matches(*_step.reader.schema(), *vs.view->view_info(), _step.current_key)) { + if (partition_key_matches(_builder.get_db().as_data_dictionary(), *_step.reader.schema(), *vs.view->view_info(), _step.current_key)) { _views_to_build.push_back(vs.view); } if (vs.next_token || _step.current_token() != vs.first_token) { diff --git a/db/view/view.hh b/db/view/view.hh index e75cceb547..b8d578551f 100644 --- a/db/view/view.hh +++ b/db/view/view.hh @@ -125,7 +125,7 @@ public: * @return false if we can guarantee that inserting an update for specified key * won't affect the view in any way, true otherwise. */ -bool partition_key_matches(const schema& base, const view_info& view, const dht::decorated_key& key); +bool partition_key_matches(data_dictionary::database, const schema& base, const view_info& view, const dht::decorated_key& key); /** * Whether the view might be affected by the provided update. @@ -140,7 +140,7 @@ bool partition_key_matches(const schema& base, const view_info& view, const dht: * @return false if we can guarantee that inserting update for key * won't affect the view in any way, true otherwise. */ -bool may_be_affected_by(const schema& base, const view_info& view, const dht::decorated_key& key, const rows_entry& update); +bool may_be_affected_by(data_dictionary::database, const schema& base, const view_info& view, const dht::decorated_key& key, const rows_entry& update); /** * Whether a given base row matches the view filter (and thus if the view should have a corresponding entry). @@ -159,9 +159,9 @@ bool may_be_affected_by(const schema& base, const view_info& view, const dht::de * @param now the current time in seconds (to decide what is live and what isn't). * @return whether the base row matches the view filter. */ -bool matches_view_filter(const schema& base, const view_info& view, const partition_key& key, const clustering_or_static_row& update, gc_clock::time_point now); +bool matches_view_filter(data_dictionary::database, const schema& base, const view_info& view, const partition_key& key, const clustering_or_static_row& update, gc_clock::time_point now); -bool clustering_prefix_matches(const schema& base, const partition_key& key, const clustering_key_prefix& ck); +bool clustering_prefix_matches(data_dictionary::database, const schema& base, const partition_key& key, const clustering_key_prefix& ck); /* * When a base-table update modifies a value in a materialized view's key @@ -310,6 +310,7 @@ view_update_builder make_view_update_builder( gc_clock::time_point now); future calculate_affected_clustering_ranges( + data_dictionary::database db, const schema& base, const dht::decorated_key& key, const mutation_partition& mp, diff --git a/replica/table.cc b/replica/table.cc index cebf23a814..5ecc3c1a74 100644 --- a/replica/table.cc +++ b/replica/table.cc @@ -1979,7 +1979,7 @@ const std::vector& table::views() const { std::vector table::affected_views(shared_ptr gen, const schema_ptr& base, const mutation& update) const { //FIXME: Avoid allocating a vector here; consider returning the boost iterator. return boost::copy_range>(_views | boost::adaptors::filtered([&] (auto&& view) { - return db::view::partition_key_matches(*base, *view->view_info(), update.decorated_key()); + return db::view::partition_key_matches(gen->get_db().as_data_dictionary(), *base, *view->view_info(), update.decorated_key()); })); } @@ -2640,7 +2640,7 @@ future table::do_push_view_replica_updates(shared_ptrget_db().as_data_dictionary(), *base, m.decorated_key(), m.partition(), views); const bool need_regular = !cr_ranges.empty(); const bool need_static = db::view::needs_static_row(m.partition(), views); if (!need_regular && !need_static) {