diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index f9848b572f9..65a3391b4e0 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -6067,51 +6067,48 @@ bool MergeTreeData::isPrimaryOrMinMaxKeyColumnPossiblyWrappedInFunctions( } bool MergeTreeData::mayBenefitFromIndexForIn( - const ASTPtr & left_in_operand, ContextPtr, const StorageMetadataPtr & metadata_snapshot) const + const ASTPtr & left_in_operand, ContextPtr query_context, const StorageMetadataPtr & metadata_snapshot) const { /// Make sure that the left side of the IN operator contain part of the key. /// If there is a tuple on the left side of the IN operator, at least one item of the tuple - /// must be part of the key (probably wrapped by a chain of some acceptable functions). + /// must be part of the key (probably wrapped by a chain of some acceptable functions). const auto * left_in_operand_tuple = left_in_operand->as(); - const auto & index_wrapper_factory = MergeTreeIndexFactory::instance(); + const auto & index_factory = MergeTreeIndexFactory::instance(); + const auto & query_settings = query_context->getSettingsRef(); + + auto check_for_one_argument = [&](const auto & ast) + { + if (isPrimaryOrMinMaxKeyColumnPossiblyWrappedInFunctions(ast, metadata_snapshot)) + return true; + + if (query_settings.use_skip_indexes) + { + for (const auto & index : metadata_snapshot->getSecondaryIndices()) + if (index_factory.get(index)->mayBenefitFromIndexForIn(ast)) + return true; + } + + if (query_settings.allow_experimental_projection_optimization) + { + for (const auto & projection : metadata_snapshot->getProjections()) + if (projection.isPrimaryKeyColumnPossiblyWrappedInFunctions(ast)) + return true; + } + + return false; + }; + if (left_in_operand_tuple && left_in_operand_tuple->name == "tuple") { for (const auto & item : left_in_operand_tuple->arguments->children) - { - if (isPrimaryOrMinMaxKeyColumnPossiblyWrappedInFunctions(item, metadata_snapshot)) - return true; - for (const auto & index : metadata_snapshot->getSecondaryIndices()) - if (index_wrapper_factory.get(index)->mayBenefitFromIndexForIn(item)) - return true; - for (const auto & projection : metadata_snapshot->getProjections()) - { - if (projection.isPrimaryKeyColumnPossiblyWrappedInFunctions(item)) - return true; - } - } - /// The tuple itself may be part of the primary key, so check that as a last resort. - if (isPrimaryOrMinMaxKeyColumnPossiblyWrappedInFunctions(left_in_operand, metadata_snapshot)) - return true; - for (const auto & projection : metadata_snapshot->getProjections()) - { - if (projection.isPrimaryKeyColumnPossiblyWrappedInFunctions(left_in_operand)) - return true; - } - return false; - } - else - { - for (const auto & index : metadata_snapshot->getSecondaryIndices()) - if (index_wrapper_factory.get(index)->mayBenefitFromIndexForIn(left_in_operand)) + if (check_for_one_argument(item)) return true; - for (const auto & projection : metadata_snapshot->getProjections()) - { - if (projection.isPrimaryKeyColumnPossiblyWrappedInFunctions(left_in_operand)) - return true; - } - return isPrimaryOrMinMaxKeyColumnPossiblyWrappedInFunctions(left_in_operand, metadata_snapshot); + /// The tuple itself may be part of the primary key + /// or skip index, so check that as a last resort. } + + return check_for_one_argument(left_in_operand); } using PartitionIdToMaxBlock = std::unordered_map; diff --git a/tests/queries/0_stateless/02707_skip_index_with_in.reference b/tests/queries/0_stateless/02707_skip_index_with_in.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/02707_skip_index_with_in.sql b/tests/queries/0_stateless/02707_skip_index_with_in.sql new file mode 100644 index 00000000000..4767619cee1 --- /dev/null +++ b/tests/queries/0_stateless/02707_skip_index_with_in.sql @@ -0,0 +1,20 @@ +DROP TABLE IF EXISTS t_skip_index_in; + +CREATE TABLE t_skip_index_in +( + a String, + b String, + c String, + INDEX idx_c c TYPE bloom_filter GRANULARITY 1 +) +ENGINE = MergeTree +ORDER BY (a, b); + +INSERT INTO t_skip_index_in VALUES ('a', 'b', 'c'); + +-- This query checks that set is not being built if indexes are not used, +-- because with EXPLAIN the set will be built only for analysis of indexes. +EXPLAIN SELECT count() FROM t_skip_index_in WHERE c IN (SELECT throwIf(1)) SETTINGS use_skip_indexes = 0 FORMAT Null; +EXPLAIN SELECT count() FROM t_skip_index_in WHERE c IN (SELECT throwIf(1)) SETTINGS use_skip_indexes = 1; -- { serverError FUNCTION_THROW_IF_VALUE_IS_NON_ZERO } + +DROP TABLE t_skip_index_in;