From e1f8cb652144135a6ada041aca0db55fc5f3ed4f Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Wed, 19 Oct 2022 11:06:47 +0300 Subject: [PATCH] materialized views: inline used-once and confusing function, replace_entry() The replace_entry() function is nothing more than a convenience for calling delete_old_entry() and then create_entry(). But it is only used once in the code, and we can just open-code the two calls instead of the one. The reason I want to change it now is that the shortcut replace_entry() helped hide a bug (#11801) - replace_entry() works incorrectly if the old and new row have the same key, because if they do we get a deletion and creation of the same row with the same timestamp - and the deletion wins. Having the two calls not hidden by a convenience function makes this potential problem more apparent. Signed-off-by: Nadav Har'El --- db/view/view.cc | 3 ++- db/view/view.hh | 4 ---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/db/view/view.cc b/db/view/view.cc index 74521934cc..a994bb0c46 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -1090,7 +1090,8 @@ void view_updates::generate_update( if (same_row) { update_entry(base_key, update, *existing, now); } else { - replace_entry(base_key, update, *existing, now); + delete_old_entry(base_key, *existing, update, now); + create_entry(base_key, update, now); } } else { delete_old_entry(base_key, *existing, update, now); diff --git a/db/view/view.hh b/db/view/view.hh index 1eb1321a21..09b08fd95a 100644 --- a/db/view/view.hh +++ b/db/view/view.hh @@ -198,10 +198,6 @@ private: void delete_old_entry(const partition_key& base_key, const clustering_row& existing, const clustering_row& update, gc_clock::time_point now); void do_delete_old_entry(const partition_key& base_key, const clustering_row& existing, const clustering_row& update, gc_clock::time_point now); void update_entry(const partition_key& base_key, const clustering_row& update, const clustering_row& existing, gc_clock::time_point now); - void replace_entry(const partition_key& base_key, const clustering_row& update, const clustering_row& existing, gc_clock::time_point now) { - delete_old_entry(base_key, existing, update, now); - create_entry(base_key, update, now); - } void update_entry_for_computed_column(const partition_key& base_key, const clustering_row& update, const std::optional& existing, gc_clock::time_point now); };