diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 175cf53ca93..a2344be3887 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -423,6 +423,7 @@ void MergeTreeData::setProperties(const StorageInMemoryMetadata & metadata, bool } ASTPtr skip_indices_with_primary_key_expr_list = new_primary_key_expr_list->clone(); + ASTPtr skip_indices_expr_list = new_primary_key_expr_list->clone(); ASTPtr skip_indices_with_sorting_key_expr_list = new_sorting_key_expr_list->clone(); MergeTreeIndices new_indices; @@ -452,6 +453,7 @@ void MergeTreeData::setProperties(const StorageInMemoryMetadata & metadata, bool { skip_indices_with_primary_key_expr_list->children.push_back(expr->clone()); skip_indices_with_sorting_key_expr_list->children.push_back(expr->clone()); + skip_indices_expr_list->children.push_back(expr->clone()); } indices_names.insert(new_indices.back()->name); @@ -462,6 +464,11 @@ void MergeTreeData::setProperties(const StorageInMemoryMetadata & metadata, bool auto new_indices_with_primary_key_expr = ExpressionAnalyzer( skip_indices_with_primary_key_expr_list, syntax_primary, global_context).getActions(false); + auto syntax_indices = SyntaxAnalyzer(global_context).analyze( + skip_indices_with_primary_key_expr_list, all_columns); + auto new_indices_expr = ExpressionAnalyzer( + skip_indices_expr_list, syntax_indices, global_context).getActions(false); + auto syntax_sorting = SyntaxAnalyzer(global_context).analyze( skip_indices_with_sorting_key_expr_list, all_columns); auto new_indices_with_sorting_key_expr = ExpressionAnalyzer( @@ -494,6 +501,7 @@ void MergeTreeData::setProperties(const StorageInMemoryMetadata & metadata, bool setConstraints(metadata.constraints); + skip_indices_expr = new_indices_expr; primary_key_and_skip_indices_expr = new_indices_with_primary_key_expr; sorting_key_and_skip_indices_expr = new_indices_with_sorting_key_expr; } diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h index bf9bfea88b3..9a3ef3088d5 100644 --- a/src/Storages/MergeTree/MergeTreeData.h +++ b/src/Storages/MergeTree/MergeTreeData.h @@ -647,6 +647,7 @@ public: /// Secondary (data skipping) indices for MergeTree MergeTreeIndices skip_indices; + ExpressionActionsPtr skip_indices_expr; ExpressionActionsPtr primary_key_and_skip_indices_expr; ExpressionActionsPtr sorting_key_and_skip_indices_expr; diff --git a/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp b/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp index e36391f87ec..264e0f34075 100644 --- a/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp +++ b/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp @@ -715,13 +715,10 @@ MergeTreeData::MutableDataPartPtr MergeTreeDataMergerMutator::mergePartsToTempor Pipe pipe(std::move(input)); - if (data.hasPrimaryKey() || data.hasSkipIndices()) + if (data.hasSortingKey()) { - auto expr = std::make_shared(pipe.getHeader(), data.sorting_key_and_skip_indices_expr); + auto expr = std::make_shared(pipe.getHeader(), data.sorting_key_expr); pipe.addSimpleTransform(std::move(expr)); - - auto materializing = std::make_shared(pipe.getHeader()); - pipe.addSimpleTransform(std::move(materializing)); } pipes.emplace_back(std::move(pipe)); @@ -796,6 +793,12 @@ MergeTreeData::MutableDataPartPtr MergeTreeDataMergerMutator::mergePartsToTempor if (need_remove_expired_values) merged_stream = std::make_shared(merged_stream, data, new_data_part, time_of_merge, force_ttl); + if (data.hasSkipIndices()) + { + merged_stream = std::make_shared(merged_stream, data.skip_indices_expr); + merged_stream = std::make_shared(merged_stream); + } + MergedBlockOutputStream to{ new_data_part, merging_columns, diff --git a/tests/queries/0_stateless/01285_data_skip_index_over_aggregation.reference b/tests/queries/0_stateless/01285_data_skip_index_over_aggregation.reference new file mode 100644 index 00000000000..0d221d16995 --- /dev/null +++ b/tests/queries/0_stateless/01285_data_skip_index_over_aggregation.reference @@ -0,0 +1,20 @@ +INSERT +1 0 +1 1 +1 1 +INSERT +1 0 +1 1 +1 0 +1 1 +1 2 +1 3 +1 1 +1 1 +1 3 +OPTIMIZE +1 3 +1 3 +OPTIMIZE +1 3 +1 3 diff --git a/tests/queries/0_stateless/01285_data_skip_index_over_aggregation.sql b/tests/queries/0_stateless/01285_data_skip_index_over_aggregation.sql new file mode 100644 index 00000000000..110c5b65cab --- /dev/null +++ b/tests/queries/0_stateless/01285_data_skip_index_over_aggregation.sql @@ -0,0 +1,34 @@ +DROP TABLE IF EXISTS data_01285; + +SET max_threads=1; + + +CREATE TABLE data_01285 ( + key Int, + value SimpleAggregateFunction(max, Nullable(Int)), + INDEX value_idx assumeNotNull(value) TYPE minmax GRANULARITY 1 +) +ENGINE=AggregatingMergeTree() +ORDER BY key; + +SELECT 'INSERT'; +INSERT INTO data_01285 SELECT 1, number FROM numbers(2); +SELECT * FROM data_01285; +SELECT * FROM data_01285 WHERE assumeNotNull(value) = 1; +SELECT 'INSERT'; +INSERT INTO data_01285 SELECT 1, number FROM numbers(4); +SELECT * FROM data_01285; +SELECT * FROM data_01285 WHERE assumeNotNull(value) = 1; +SELECT * FROM data_01285 WHERE assumeNotNull(value) = 3; +SELECT 'OPTIMIZE'; +OPTIMIZE TABLE data_01285 FINAL; +SELECT * FROM data_01285; +-- before the fix value_idx contains one range {0, 0} +-- and hence cannot find these record. +SELECT * FROM data_01285 WHERE assumeNotNull(value) = 3; +-- one more time just in case +SELECT 'OPTIMIZE'; +OPTIMIZE TABLE data_01285 FINAL; +SELECT * FROM data_01285; +-- and this passes even without fix. +SELECT * FROM data_01285 WHERE assumeNotNull(value) = 3;