Intermediate changes

6 files changed, 37 insertions, 33 deletions

diff --git a/contrib/clickhouse/src/Interpreters/MutationsInterpreter.cpp b/contrib/clickhouse/src/Interpreters/MutationsInterpreter.cpp
index e50f8488ca..c8fe0205d5 100644
--- a/contrib/clickhouse/src/Interpreters/MutationsInterpreter.cpp
+++ b/contrib/clickhouse/src/Interpreters/MutationsInterpreter.cpp
@@ -671,9 +671,15 @@ void MutationsInterpreter::prepare(bool dry_run)
                 {
                     if (column.default_desc.kind == ColumnDefaultKind::Materialized)
                     {
+                        auto type_literal = std::make_shared<ASTLiteral>(column.type->getName());
+
+                        auto materialized_column = makeASTFunction("_CAST",
+                            column.default_desc.expression->clone(),
+                            type_literal);
+
                         stages.back().column_to_updated.emplace(
                             column.name,
-                            column.default_desc.expression->clone());
+                            materialized_column);
                     }
                 }
             }
diff --git a/contrib/clickhouse/src/Processors/QueryPlan/Optimizations/optimizeUseNormalProjection.cpp b/contrib/clickhouse/src/Processors/QueryPlan/Optimizations/optimizeUseNormalProjection.cpp
index 727afcb1a9..fbe02265dc 100644
--- a/contrib/clickhouse/src/Processors/QueryPlan/Optimizations/optimizeUseNormalProjection.cpp
+++ b/contrib/clickhouse/src/Processors/QueryPlan/Optimizations/optimizeUseNormalProjection.cpp
@@ -236,7 +236,7 @@ bool optimizeUseNormalProjections(Stack & stack, QueryPlan::Nodes & nodes)
     }
     else
     {
-        const auto & main_stream = iter->node->children.front()->step->getOutputStream();
+        const auto & main_stream = iter->node->children[iter->next_child - 1]->step->getOutputStream();
         const auto * proj_stream = &next_node->step->getOutputStream();
 
         if (auto materializing = makeMaterializingDAG(proj_stream->header, main_stream.header))
@@ -252,7 +252,7 @@ bool optimizeUseNormalProjections(Stack & stack, QueryPlan::Nodes & nodes)
         auto & union_node = nodes.emplace_back();
         DataStreams input_streams = {main_stream, *proj_stream};
         union_node.step = std::make_unique<UnionStep>(std::move(input_streams));
-        union_node.children = {iter->node->children.front(), next_node};
+        union_node.children = {iter->node->children[iter->next_child - 1], next_node};
         iter->node->children[iter->next_child - 1] = &union_node;
     }
 
diff --git a/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp b/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp
index 65cf8bbce7..927df3c50e 100644
--- a/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp
+++ b/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp
@@ -2,6 +2,11 @@
 #include <Storages/StorageReplicatedMergeTree.h>
 #include <Common/ZooKeeper/IKeeper.h>
 
+namespace CurrentMetrics
+{
+    extern const Metric ReadonlyReplica;
+}
+
 namespace DB
 {
 
@@ -67,6 +72,9 @@ void ReplicatedMergeTreeAttachThread::run()
             LOG_ERROR(log, "Initialization failed, table will remain readonly. Error: {}", getCurrentExceptionMessage(/* with_stacktrace */ true));
             storage.initialization_done = true;
         }
+
+        if (!std::exchange(storage.is_readonly_metric_set, true))
+            CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
     }
 
     if (!first_try_done.exchange(true))
@@ -74,6 +82,12 @@ void ReplicatedMergeTreeAttachThread::run()
 
     if (shutdown_called)
     {
+        if (std::exchange(storage.is_readonly_metric_set, false))
+        {
+            CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
+            chassert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
+        }
+
         LOG_WARNING(log, "Shutdown called, cancelling initialization");
         return;
     }
diff --git a/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp b/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp
index 79054ef46d..d83e9c3f80 100644
--- a/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp
+++ b/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp
@@ -79,11 +79,10 @@ void ReplicatedMergeTreeRestartingThread::run()
 
     if (first_time)
     {
-        if (storage.is_readonly)
-        {
+        if (storage.is_readonly && !std::exchange(storage.is_readonly_metric_set, true))
             /// We failed to start replication, table is still readonly, so we should increment the metric. See also setNotReadonly().
             CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
-        }
+
         /// It does not matter if replication is actually started or not, just notify after the first attempt.
         storage.startup_event.set();
         first_time = false;
@@ -360,21 +359,19 @@ void ReplicatedMergeTreeRestartingThread::setReadonly(bool on_shutdown)
         return;
 
     if (became_readonly)
-        CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
-
-    /// Replica was already readonly, but we should decrement the metric, because we are detaching/dropping table.
-    /// if first pass wasn't done we don't have to decrement because it wasn't incremented in the first place
-    /// the task should be deactivated if it's full shutdown so no race is present
-    if (!first_time && on_shutdown)
     {
-        CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
-        assert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
+        chassert(!storage.is_readonly_metric_set);
+        storage.is_readonly_metric_set = true;
+        CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
+        return;
     }
 
-    if (storage.since_metadata_err_incr_readonly_metric)
+    /// Replica was already readonly, but we should decrement the metric if it was set because we are detaching/dropping table.
+    /// the task should be deactivated if it's full shutdown so no race is present
+    if (on_shutdown && std::exchange(storage.is_readonly_metric_set, false))
     {
         CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
-        assert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
+        chassert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
     }
 }
 
@@ -384,10 +381,10 @@ void ReplicatedMergeTreeRestartingThread::setNotReadonly()
     /// is_readonly is true on startup, but ReadonlyReplica metric is not incremented,
     /// because we don't want to change this metric if replication is started successfully.
     /// So we should not decrement it when replica stopped being readonly on startup.
-    if (storage.is_readonly.compare_exchange_strong(old_val, false) && !first_time)
+    if (storage.is_readonly.compare_exchange_strong(old_val, false) && std::exchange(storage.is_readonly_metric_set, false))
     {
         CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
-        assert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
+        chassert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
     }
 }
 
diff --git a/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.cpp b/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.cpp
index 2c272e2ecd..ddf9d743aa 100644
--- a/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.cpp
+++ b/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.cpp
@@ -4820,23 +4820,13 @@ void StorageReplicatedMergeTree::startupImpl(bool from_attach_thread)
     /// Do not start replication if ZooKeeper is not configured or there is no metadata in zookeeper
     if (!has_metadata_in_zookeeper.has_value() || !*has_metadata_in_zookeeper)
     {
-        if (!since_metadata_err_incr_readonly_metric)
-        {
-            since_metadata_err_incr_readonly_metric = true;
+        if (!std::exchange(is_readonly_metric_set, true))
             CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
-        }
 
         LOG_TRACE(log, "No connection to ZooKeeper or no metadata in ZooKeeper, will not startup");
         return;
     }
 
-    if (since_metadata_err_incr_readonly_metric)
-    {
-        since_metadata_err_incr_readonly_metric = false;
-        CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
-        assert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
-    }
-
     try
     {
         auto zookeeper = getZooKeeper();
diff --git a/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.h b/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.h
index 794991d8e0..33acf5af83 100644
--- a/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.h
+++ b/contrib/clickhouse/src/Storages/StorageReplicatedMergeTree.h
@@ -427,10 +427,7 @@ private:
     /// If false - ZooKeeper is available, but there is no table metadata. It's safe to drop table in this case.
     std::optional<bool> has_metadata_in_zookeeper;
 
-    /// during server restart or attach table process, set since_metadata_err_incr_readonly_metric = true and increase readonly metric if has_metadata_in_zookeeper = false.
-    /// during detach or drop table process, decrease readonly metric if since_metadata_err_incr_readonly_metric = true.
-    /// during restore replica process, set since_metadata_err_incr_readonly_metric = false and decrease readonly metric if since_metadata_err_incr_readonly_metric = true.
-    bool since_metadata_err_incr_readonly_metric = false;
+    bool is_readonly_metric_set = false;
 
     static const String default_zookeeper_name;
     const String zookeeper_name;