Skip to content

Commit

Permalink
Merge pull request #58566 from ClickHouse/cherrypick/23.11/e24ec55451…
Browse files Browse the repository at this point in the history
…24435aa3ccd39110a164f01ca084cd

Cherry pick #58482 to 23.11: Fix a stupid case of intersecting parts
  • Loading branch information
robot-clickhouse committed Jan 5, 2024
2 parents d2b5e4a + e24ec55 commit 3393898
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 5 deletions.
14 changes: 9 additions & 5 deletions src/Storages/MergeTree/MergeTreeData.cpp
Expand Up @@ -3961,8 +3961,15 @@ MergeTreeData::PartsToRemoveFromZooKeeper MergeTreeData::removePartsInRangeFromW
/// FIXME refactor removePartsFromWorkingSet(...), do not remove parts twice
removePartsFromWorkingSet(txn, parts_to_remove, clear_without_timeout, lock);

/// We can only create a covering part for a blocks range that starts with 0 (otherwise we may get "intersecting parts"
/// if we remove a range from the middle when dropping a part).
/// Maybe we could do it by incrementing mutation version to get a name for the empty covering part,
/// but it's okay to simply avoid creating it for DROP PART (for a part in the middle).
/// NOTE: Block numbers in ReplicatedMergeTree start from 0. For MergeTree, is_new_syntax is always false.
assert(!create_empty_part || supportsReplication());
bool range_in_the_middle = drop_range.min_block;
bool is_new_syntax = format_version >= MERGE_TREE_DATA_MIN_FORMAT_VERSION_WITH_CUSTOM_PARTITIONING;
if (create_empty_part && !parts_to_remove.empty() && is_new_syntax)
if (create_empty_part && !parts_to_remove.empty() && is_new_syntax && !range_in_the_middle)
{
/// We are going to remove a lot of parts from zookeeper just after returning from this function.
/// And we will remove parts from disk later (because some queries may use them).
Expand All @@ -3971,12 +3978,9 @@ MergeTreeData::PartsToRemoveFromZooKeeper MergeTreeData::removePartsInRangeFromW
/// We don't need to commit it to zk, and don't even need to activate it.

MergeTreePartInfo empty_info = drop_range;
empty_info.level = empty_info.mutation = 0;
if (!empty_info.min_block)
empty_info.min_block = MergeTreePartInfo::MAX_BLOCK_NUMBER;
empty_info.min_block = empty_info.level = empty_info.mutation = 0;
for (const auto & part : parts_to_remove)
{
empty_info.min_block = std::min(empty_info.min_block, part->info.min_block);
empty_info.level = std::max(empty_info.level, part->info.level);
empty_info.mutation = std::max(empty_info.mutation, part->info.mutation);
}
Expand Down
Expand Up @@ -13,3 +13,5 @@
5 rmt2
7 rmt2
9 rmt2
1
3
17 changes: 17 additions & 0 deletions tests/queries/0_stateless/02486_truncate_and_unexpected_parts.sql
Expand Up @@ -50,3 +50,20 @@ system sync replica rmt1;
system sync replica rmt2;

select *, _table from merge(currentDatabase(), '') order by _table, (*,);


create table rmt3 (n int) engine=ReplicatedMergeTree('/test/02468/{database}3', '1') order by tuple() settings replicated_max_ratio_of_wrong_parts=0, max_suspicious_broken_parts=0, max_suspicious_broken_parts_bytes=0;
set insert_keeper_fault_injection_probability=0;
insert into rmt3 values (1);
insert into rmt3 values (2);
insert into rmt3 values (3);

system stop cleanup rmt3;
system sync replica rmt3 pull;
alter table rmt3 drop part 'all_1_1_0';
optimize table rmt3 final;

detach table rmt3 sync;
attach table rmt3;

select * from rmt3 order by n;

0 comments on commit 3393898

Please sign in to comment.