From 39cd9dae4e2b13ba2f3d6ef192e9446c7d84a47e Mon Sep 17 00:00:00 2001
From: Eliran Sinvani <eliransin@scylladb.com>
Date: Mon, 23 Nov 2020 15:56:35 +0200
Subject: [PATCH] materialized views: Extract fix legacy schema into its own
 logic

We extract the logic for fixing the view schema into it's own
logic as we will need to use it in more places in the code.
This makes 'maybe_update_legacy_secondary_index_mv_schema' redundant since
it becomes a two liner wrapper for this logic. We also
remove it here and replace the call to it with the equivalent code.
---
 database.cc         |  8 +++++++-
 db/schema_tables.cc | 21 +++++++++++++--------
 db/schema_tables.hh |  4 +++-
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/database.cc b/database.cc
index 07b670f84e..9fbc354ef6 100644
--- a/database.cc
+++ b/database.cc
@@ -848,7 +848,13 @@ future<> database::parse_system_tables(distributed<service::storage_proxy>& prox
             return create_views_from_schema_partition(proxy, v.second).then([this, &mm] (std::vector<view_ptr> views) {
                 return parallel_for_each(views.begin(), views.end(), [this, &mm] (auto&& v) {
                     return this->add_column_family_and_make_directory(v).then([this, &mm, v] {
-                        return maybe_update_legacy_secondary_index_mv_schema(mm.local(), *this, v);
+                        // TODO: Remove once computed columns are guaranteed to be featured in the whole cluster.
+                        view_ptr fixed_v = maybe_fix_legacy_secondary_index_mv_schema(*this, v, nullptr, preserve_version::no);
+                        if (fixed_v) {
+                            return mm.local().announce_view_update(view_ptr(fixed_v));
+                        } else {
+                            return make_ready_future<>();
+                        }
                     });
                 });
             });
diff --git a/db/schema_tables.cc b/db/schema_tables.cc
index 941c3ab1fa..ff96b49119 100644
--- a/db/schema_tables.cc
+++ b/db/schema_tables.cc
@@ -3071,8 +3071,7 @@ std::vector<sstring> all_table_names(schema_features features) {
            boost::adaptors::transformed([] (auto schema) { return schema->cf_name(); }));
 }
 
-future<> maybe_update_legacy_secondary_index_mv_schema(service::migration_manager& mm, database& db, view_ptr v) {
-    // TODO(sarna): Remove once computed columns are guaranteed to be featured in the whole cluster.
+view_ptr maybe_fix_legacy_secondary_index_mv_schema(database& db, const view_ptr& v, schema_ptr base_schema, preserve_version preserve_version) {
     // Legacy format for a secondary index used a hardcoded "token" column, which ensured a proper
     // order for indexed queries. This "token" column is now implemented as a computed column,
     // but for the sake of compatibility we assume that there might be indexes created in the legacy
@@ -3080,26 +3079,32 @@ future<> maybe_update_legacy_secondary_index_mv_schema(service::migration_manage
     // columns marked as computed (because they were either created on a node that supports computed
     // columns or were fixed by this utility function), it's safe to remove this function altogether.
     if (v->clustering_key_size() == 0) {
-        return make_ready_future<>();
+        return view_ptr(nullptr);
     }
     const column_definition& first_view_ck = v->clustering_key_columns().front();
     if (first_view_ck.is_computed()) {
-        return make_ready_future<>();
+        return view_ptr(nullptr);
+    }
+
+    if (!base_schema) {
+        base_schema = db.find_schema(v->view_info()->base_id());
     }
 
-    table& base = db.find_column_family(v->view_info()->base_id());
-    schema_ptr base_schema = base.schema();
     // If the first clustering key part of a view is a column with name not found in base schema,
     // it implies it might be backing an index created before computed columns were introduced,
     // and as such it must be recreated properly.
     if (!base_schema->columns_by_name().contains(first_view_ck.name())) {
         schema_builder builder{schema_ptr(v)};
         builder.mark_column_computed(first_view_ck.name(), std::make_unique<legacy_token_column_computation>());
-        return mm.announce_view_update(view_ptr(builder.build()));
+        if (preserve_version) {
+            builder.with_version(v->version());
+        }
+        return view_ptr(builder.build());
     }
-    return make_ready_future<>();
+    return view_ptr(nullptr);
 }
 
+
 namespace legacy {
 
 table_schema_version schema_mutations::digest() const {
diff --git a/db/schema_tables.hh b/db/schema_tables.hh
index 74d10cc9a1..0d51a88869 100644
--- a/db/schema_tables.hh
+++ b/db/schema_tables.hh
@@ -239,7 +239,9 @@ std::vector<mutation> make_update_view_mutations(lw_shared_ptr<keyspace_metadata
 
 std::vector<mutation> make_drop_view_mutations(lw_shared_ptr<keyspace_metadata> keyspace, view_ptr view, api::timestamp_type timestamp);
 
-future<> maybe_update_legacy_secondary_index_mv_schema(service::migration_manager& mm, database& db, view_ptr v);
+class preserve_version_tag {};
+using preserve_version = bool_class<preserve_version_tag>;
+view_ptr maybe_fix_legacy_secondary_index_mv_schema(database& db, const view_ptr& v, schema_ptr base_schema, preserve_version preserve_version);
 
 sstring serialize_kind(column_kind kind);
 column_kind deserialize_kind(sstring kind);