Merge pull request ClickHouse#58638 from amosbird/fix-58620

Fix broken partition key analysis when doing projection optimization
Altinity · Oct 17, 2024 · eb5814e · eb5814e
1 parent 7082e22
commit eb5814e
Show file tree

Hide file tree

Showing 13 changed files with 27 additions and 21 deletions.
diff --git a/src/Processors/QueryPlan/Optimizations/optimizeUseAggregateProjection.cpp b/src/Processors/QueryPlan/Optimizations/optimizeUseAggregateProjection.cpp
@@ -618,7 +618,6 @@ bool optimizeUseAggregateProjections(QueryPlan::Node & node, QueryPlan::Nodes &
                 reader,
                 required_column_names,
                 parts_with_ranges,
-                metadata,
                 query_info,
                 context,
                 max_added_blocks,

diff --git a/src/Processors/QueryPlan/Optimizations/optimizeUseNormalProjection.cpp b/src/Processors/QueryPlan/Optimizations/optimizeUseNormalProjection.cpp
@@ -171,7 +171,6 @@ bool optimizeUseNormalProjections(Stack & stack, QueryPlan::Nodes & nodes)
             reader,
             required_columns,
             parts_with_ranges,
-            metadata,
             query_info,
             context,
             max_added_blocks,

diff --git a/src/Processors/QueryPlan/Optimizations/projectionsCommon.cpp b/src/Processors/QueryPlan/Optimizations/projectionsCommon.cpp
@@ -211,7 +211,6 @@ bool analyzeProjectionCandidate(
     const MergeTreeDataSelectExecutor & reader,
     const Names & required_column_names,
     const RangesInDataParts & parts_with_ranges,
-    const StorageMetadataPtr & metadata,
     const SelectQueryInfo & query_info,
     const ContextPtr & context,
     const std::shared_ptr<PartitionIdToMaxBlock> & max_added_blocks,
@@ -242,7 +241,6 @@ bool analyzeProjectionCandidate(
         std::move(projection_parts),
         nullptr,
         required_column_names,
-        metadata,
         candidate.projection->metadata,
         query_info, /// How it is actually used? I hope that for index we need only added_filter_nodes
         added_filter_nodes,

diff --git a/src/Processors/QueryPlan/Optimizations/projectionsCommon.h b/src/Processors/QueryPlan/Optimizations/projectionsCommon.h
@@ -73,7 +73,6 @@ bool analyzeProjectionCandidate(
     const MergeTreeDataSelectExecutor & reader,
     const Names & required_column_names,
     const RangesInDataParts & parts_with_ranges,
-    const StorageMetadataPtr & metadata,
     const SelectQueryInfo & query_info,
     const ContextPtr & context,
     const std::shared_ptr<PartitionIdToMaxBlock> & max_added_blocks,

diff --git a/src/Processors/QueryPlan/ReadFromMergeTree.cpp b/src/Processors/QueryPlan/ReadFromMergeTree.cpp
@@ -1168,7 +1168,6 @@ MergeTreeDataSelectAnalysisResultPtr ReadFromMergeTree::selectRangesToRead(
         std::move(alter_conversions),
         prewhere_info,
         filter_nodes,
-        storage_snapshot->metadata,
         metadata_for_reading,
         query_info,
         context,
@@ -1354,7 +1353,6 @@ MergeTreeDataSelectAnalysisResultPtr ReadFromMergeTree::selectRangesToRead(
     std::vector<AlterConversionsPtr> alter_conversions,
     const PrewhereInfoPtr & prewhere_info,
     const ActionDAGNodes & added_filter_nodes,
-    const StorageMetadataPtr & metadata_snapshot_base,
     const StorageMetadataPtr & metadata_snapshot,
     const SelectQueryInfo & query_info,
     ContextPtr context,
@@ -1375,7 +1373,6 @@ MergeTreeDataSelectAnalysisResultPtr ReadFromMergeTree::selectRangesToRead(
         return selectRangesToReadImpl(
             std::move(parts),
             std::move(alter_conversions),
-            metadata_snapshot_base,
             metadata_snapshot,
             updated_query_info_with_filter_dag,
             context,
@@ -1391,7 +1388,6 @@ MergeTreeDataSelectAnalysisResultPtr ReadFromMergeTree::selectRangesToRead(
     return selectRangesToReadImpl(
         std::move(parts),
         std::move(alter_conversions),
-        metadata_snapshot_base,
         metadata_snapshot,
         query_info,
         context,
@@ -1407,7 +1403,6 @@ MergeTreeDataSelectAnalysisResultPtr ReadFromMergeTree::selectRangesToRead(
 MergeTreeDataSelectAnalysisResultPtr ReadFromMergeTree::selectRangesToReadImpl(
     MergeTreeData::DataPartsVector parts,
     std::vector<AlterConversionsPtr> alter_conversions,
-    const StorageMetadataPtr & metadata_snapshot_base,
     const StorageMetadataPtr & metadata_snapshot,
     const SelectQueryInfo & query_info,
     ContextPtr context,
@@ -1468,7 +1463,7 @@ MergeTreeDataSelectAnalysisResultPtr ReadFromMergeTree::selectRangesToReadImpl(
             parts,
             alter_conversions,
             part_values,
-            metadata_snapshot_base,
+            metadata_snapshot,
             data,
             context,
             max_block_numbers_to_read.get(),

diff --git a/src/Processors/QueryPlan/ReadFromMergeTree.h b/src/Processors/QueryPlan/ReadFromMergeTree.h
@@ -178,7 +178,6 @@ class ReadFromMergeTree final : public SourceStepWithFilter
         std::vector<AlterConversionsPtr> alter_conversions,
         const PrewhereInfoPtr & prewhere_info,
         const ActionDAGNodes & added_filter_nodes,
-        const StorageMetadataPtr & metadata_snapshot_base,
         const StorageMetadataPtr & metadata_snapshot,
         const SelectQueryInfo & query_info,
         ContextPtr context,
@@ -228,7 +227,6 @@ class ReadFromMergeTree final : public SourceStepWithFilter
     static MergeTreeDataSelectAnalysisResultPtr selectRangesToReadImpl(
         MergeTreeData::DataPartsVector parts,
         std::vector<AlterConversionsPtr> alter_conversions,
-        const StorageMetadataPtr & metadata_snapshot_base,
         const StorageMetadataPtr & metadata_snapshot,
         const SelectQueryInfo & query_info,
         ContextPtr context,

diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp
@@ -6536,7 +6536,6 @@ static void selectBestProjection(
         projection_parts,
         candidate.prewhere_info,
         candidate.required_columns,
-        storage_snapshot->metadata,
         candidate.desc->metadata,
         query_info,
         added_filter_nodes,
@@ -6561,7 +6560,6 @@ static void selectBestProjection(
             query_info.prewhere_info,
             required_columns,
             storage_snapshot->metadata,
-            storage_snapshot->metadata,
             query_info, // TODO syntax_analysis_result set in index
             added_filter_nodes,
             query_context,
@@ -7213,7 +7211,6 @@ std::optional<ProjectionCandidate> MergeTreeData::getQueryProcessingStageWithAgg
                 query_info.prewhere_info,
                 analysis_result.required_columns,
                 metadata_snapshot,
-                metadata_snapshot,
                 query_info,
                 added_filter_nodes,
                 query_context,
@@ -7246,7 +7243,6 @@ std::optional<ProjectionCandidate> MergeTreeData::getQueryProcessingStageWithAgg
             query_info.prewhere_info,
             analysis_result.required_columns,
             metadata_snapshot,
-            metadata_snapshot,
             query_info,
             added_filter_nodes,
             query_context,
@@ -7386,7 +7382,6 @@ bool MergeTreeData::canUseParallelReplicasBasedOnPKAnalysis(
         query_info.prewhere_info,
         storage_snapshot->getMetadataForQuery()->getColumns().getAll().getNames(),
         storage_snapshot->metadata,
-        storage_snapshot->metadata,
         query_info,
         /*added_filter_nodes*/ActionDAGNodes{},
         query_context,

diff --git a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp
@@ -816,6 +816,7 @@ void MergeTreeDataSelectExecutor::filterPartsByPartition(
 
     if (metadata_snapshot->hasPartitionKey())
     {
+        chassert(minmax_idx_condition && partition_pruner);
         const auto & partition_key = metadata_snapshot->getPartitionKey();
         minmax_columns_types = data.getMinMaxColumnsTypes(partition_key);
 
@@ -1231,7 +1232,6 @@ MergeTreeDataSelectAnalysisResultPtr MergeTreeDataSelectExecutor::estimateNumMar
     MergeTreeData::DataPartsVector parts,
     const PrewhereInfoPtr & prewhere_info,
     const Names & column_names_to_return,
-    const StorageMetadataPtr & metadata_snapshot_base,
     const StorageMetadataPtr & metadata_snapshot,
     const SelectQueryInfo & query_info,
     const ActionDAGNodes & added_filter_nodes,
@@ -1260,7 +1260,6 @@ MergeTreeDataSelectAnalysisResultPtr MergeTreeDataSelectExecutor::estimateNumMar
         /*alter_conversions=*/ {},
         prewhere_info,
         added_filter_nodes,
-        metadata_snapshot_base,
         metadata_snapshot,
         query_info,
         context,

diff --git a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.h b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.h
@@ -59,7 +59,6 @@ class MergeTreeDataSelectExecutor
         MergeTreeData::DataPartsVector parts,
         const PrewhereInfoPtr & prewhere_info,
         const Names & column_names,
-        const StorageMetadataPtr & metadata_snapshot_base,
         const StorageMetadataPtr & metadata_snapshot,
         const SelectQueryInfo & query_info,
         const ActionDAGNodes & added_filter_nodes,

diff --git a/tests/queries/0_stateless/01710_projection_fix_crash.reference b/tests/queries/0_stateless/01710_projection_fix_crash.reference
diff --git a/tests/queries/0_stateless/01710_projection_fix_crash.sql b/tests/queries/0_stateless/01710_projection_fix_crash.sql
@@ -0,0 +1,15 @@
+set force_index_by_date = 1;
+
+create table xxxxx (col1 String, col2 String, _time DateTime, projection p (select * order by col2)) engine=MergeTree partition by col1 order by tuple();
+
+create table yyyyyyy (col1 String, col2 String, _time DateTime, projection p (select * order by col2)) engine=MergeTree partition by col1 order by tuple();
+
+insert into xxxxx (col1, col2, _time) values ('xxx', 'zzzz', now()+1);
+insert into yyyyyyy (col1, col2, _time) values ('xxx', 'zzzz', now());
+
+SELECT count()
+FROM xxxxx
+WHERE (col1 = 'xxx') AND (_time = (
+    SELECT max(_time)
+    FROM yyyyyyy
+    WHERE (col1 = 'xxx') AND (col2 = 'zzzz') AND (_time > (now() - toIntervalDay(3)))))
diff --git a/tests/queries/0_stateless/02965_projection_with_partition_pruning.reference b/tests/queries/0_stateless/02965_projection_with_partition_pruning.reference
@@ -0,0 +1 @@
+3	4
diff --git a/tests/queries/0_stateless/02965_projection_with_partition_pruning.sql b/tests/queries/0_stateless/02965_projection_with_partition_pruning.sql
@@ -0,0 +1,9 @@
+drop table if exists a;
+
+create table a (i int, j int, projection p (select * order by j)) engine MergeTree partition by i order by tuple() settings index_granularity = 1;
+
+insert into a values (1, 2), (0, 5), (3, 4);
+
+select * from a where i > 0 and j = 4 settings force_index_by_date = 1;
+
+drop table a;