Do not change the snapshot after at reading from MT

KochetovNicolai · KochetovNicolai · commit 5ee9b56040ae · 2023-08-22T13:50:51.000Z
diff --git a/src/Planner/Planner.cpp b/src/Planner/Planner.cpp
@@ -189,7 +189,8 @@ void collectFiltersForAnalysis(const QueryTreeNodePtr & query_tree, const Planne
 
     const auto & query_context = planner_context->getQueryContext();
 
-    Planner planner(updated_query_tree, {});
+    SelectQueryOptions select_query_options;
+    Planner planner(updated_query_tree, select_query_options);
     planner.buildQueryPlanIfNeeded();
 
     auto & result_query_plan = planner.getQueryPlan();
diff --git a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp
@@ -1304,14 +1304,18 @@ QueryPlanStepPtr MergeTreeDataSelectExecutor::readFromParts(
 
     selectColumnNames(column_names_to_return, data, real_column_names, virt_column_names, sample_factor_column_queried);
 
+    /// Do not keep data parts in snapshot.
+    /// They are stored separately, and some could be released after PK analysis.
+    auto storage_snapshot_copy = storage_snapshot->clone(std::make_unique<MergeTreeData::SnapshotData>());
+
     return std::make_unique<ReadFromMergeTree>(
         std::move(parts),
         std::move(alter_conversions),
         real_column_names,
         virt_column_names,
         data,
         query_info,
-        storage_snapshot,
+        storage_snapshot_copy,
         context,
         max_block_size,
         num_streams,
diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp
@@ -255,13 +255,6 @@ void StorageMergeTree::read(
             processed_stage, nullptr, enable_parallel_reading))
             query_plan = std::move(*plan);
     }
-
-    /// Now, copy of parts that is required for the query, stored in the processors,
-    /// while snapshot_data.parts includes all parts, even one that had been filtered out with partition pruning,
-    /// reset them to avoid holding them.
-    auto & snapshot_data = assert_cast<MergeTreeData::SnapshotData &>(*storage_snapshot->data);
-    snapshot_data.parts = {};
-    snapshot_data.alter_conversions = {};
 }
 
 std::optional<UInt64> StorageMergeTree::totalRows(const Settings &) const
diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp
@@ -5103,15 +5103,6 @@ void StorageReplicatedMergeTree::read(
     const size_t max_block_size,
     const size_t num_streams)
 {
-    SCOPE_EXIT({
-        /// Now, copy of parts that is required for the query, stored in the processors,
-        /// while snapshot_data.parts includes all parts, even one that had been filtered out with partition pruning,
-        /// reset them to avoid holding them.
-        auto & snapshot_data = assert_cast<MergeTreeData::SnapshotData &>(*storage_snapshot->data);
-        snapshot_data.parts = {};
-        snapshot_data.alter_conversions = {};
-    });
-
     const auto & settings = local_context->getSettingsRef();
 
     /// The `select_sequential_consistency` setting has two meanings:
diff --git a/src/Storages/StorageSnapshot.cpp b/src/Storages/StorageSnapshot.cpp
@@ -17,6 +17,16 @@ namespace ErrorCodes
     extern const int COLUMN_QUERIED_MORE_THAN_ONCE;
 }
 
+std::shared_ptr<StorageSnapshot> StorageSnapshot::clone(DataPtr data_) const
+{
+    auto res = std::make_shared<StorageSnapshot>(storage, metadata, object_columns);
+
+    res->projection = projection;
+    res->data = std::move(data_);
+
+    return res;
+}
+
 void StorageSnapshot::init()
 {
     for (const auto & [name, type] : storage.getVirtuals())
diff --git a/src/Storages/StorageSnapshot.h b/src/Storages/StorageSnapshot.h
@@ -60,6 +60,8 @@ struct StorageSnapshot
         init();
     }
 
+    std::shared_ptr<StorageSnapshot> clone(DataPtr data_) const;
+
     /// Get all available columns with types according to options.
     NamesAndTypesList getColumns(const GetColumnsOptions & options) const;
 

Original file line number	Diff line number	Diff line change
`@@ -255,13 +255,6 @@ void StorageMergeTree::read(`
`255`	`255`	`processed_stage, nullptr, enable_parallel_reading))`
`256`	`256`	`query_plan = std::move(*plan);`
`257`	`257`	`}`
`258`		`-`
`259`		`- /// Now, copy of parts that is required for the query, stored in the processors,`
`260`		`- /// while snapshot_data.parts includes all parts, even one that had been filtered out with partition pruning,`
`261`		`- /// reset them to avoid holding them.`
`262`		`- auto & snapshot_data = assert_cast<MergeTreeData::SnapshotData &>(*storage_snapshot->data);`
`263`		`- snapshot_data.parts = {};`
`264`		`- snapshot_data.alter_conversions = {};`
`265`	`258`	`}`
`266`	`259`
`267`	`260`	`std::optional<UInt64> StorageMergeTree::totalRows(const Settings &) const`
Original file line number	Diff line number	Diff line change
`@@ -60,6 +60,8 @@ struct StorageSnapshot`
`60`	`60`	`init();`
`61`	`61`	`}`
`62`	`62`
	`63`	`+ std::shared_ptr<StorageSnapshot> clone(DataPtr data_) const;`
	`64`	`+`
`63`	`65`	`/// Get all available columns with types according to options.`
`64`	`66`	`NamesAndTypesList getColumns(const GetColumnsOptions & options) const;`
`65`	`67`