From 47b29092b4d0adff8f734f5b6b847d660cf2e38f Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Tue, 22 Jun 2021 16:47:42 +0300
Subject: [PATCH] Fix bug in MergerMutator parts selector

---
 .../MergeTree/MergeTreeDataMergerMutator.cpp  | 23 +++++++++++++++----
 ...nt_ttl_and_normal_merges_zookeeper_long.sh |  2 +-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp b/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp
index 846ad7b026d..766d988500d 100644
--- a/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp
+++ b/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp
@@ -264,6 +264,10 @@ SelectPartsDecision MergeTreeDataMergerMutator::selectPartsToMerge(
             if (!can_merge_callback(nullptr, part, nullptr))
                 continue;
 
+            /// This part can be merged only with next parts (no prev part exists), so start
+            /// new interval if previous was not empty.
+            if (!parts_ranges.back().empty())
+                parts_ranges.emplace_back();
         }
         else
         {
@@ -271,12 +275,21 @@ SelectPartsDecision MergeTreeDataMergerMutator::selectPartsToMerge(
             /// interval (in the same partition)
             if (!can_merge_callback(*prev_part, part, nullptr))
             {
-                /// Starting new interval in the same partition
-                assert(!parts_ranges.back().empty());
-                parts_ranges.emplace_back();
-
-                /// Now we have no previous part, but it affects only logging
+                /// Now we have no previous part
                 prev_part = nullptr;
+
+                /// Mustn't be empty
+                assert(!parts_ranges.back().empty());
+
+                /// Some parts cannot be merged with previous parts and also cannot be merged with themselves,
+                /// for example, merge is already assigned for such parts, or they participate in quorum inserts
+                /// and so on.
+                /// Also we don't start new interval here (maybe all next parts cannot be merged and we don't want to have empty interval)
+                if (!can_merge_callback(nullptr, part, nullptr))
+                    continue;
+
+                /// Starting new interval in the same partition
+                parts_ranges.emplace_back();
             }
         }
 
diff --git a/tests/queries/0_stateless/01921_concurrent_ttl_and_normal_merges_zookeeper_long.sh b/tests/queries/0_stateless/01921_concurrent_ttl_and_normal_merges_zookeeper_long.sh
index f84a69e8eb0..80e7d6b4c00 100755
--- a/tests/queries/0_stateless/01921_concurrent_ttl_and_normal_merges_zookeeper_long.sh
+++ b/tests/queries/0_stateless/01921_concurrent_ttl_and_normal_merges_zookeeper_long.sh
@@ -15,7 +15,7 @@ for i in $(seq 1 $NUM_REPLICAS); do
     $CLICKHOUSE_CLIENT -n --query "CREATE TABLE ttl_table$i(
         key DateTime
     )
-    ENGINE ReplicatedMergeTree('/test/01921_concurrent_ttl_and_normal_merges/${CLICKHOUSE_TEST_ZOOKEEPER_PREFIX}/ttl_table', '$i')
+    ENGINE ReplicatedMergeTree('/test/01921_concurrent_ttl_and_normal_merges/$CLICKHOUSE_TEST_ZOOKEEPER_PREFIX/ttl_table', '$i')
     ORDER BY tuple()
     TTL key + INTERVAL 1 SECOND
     SETTINGS merge_with_ttl_timeout=1, max_replicated_merges_with_ttl_in_queue=100, max_number_of_merges_with_ttl_in_pool=100;"