From 167e4903a8f5b175d587efaed15c154ee0837991 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 2 Mar 2023 18:45:59 +0100 Subject: [PATCH] Fix concrete columns PREWHERE support This is the fix for the IStorage::supportedPrewhereColumns() API. Signed-off-by: Azat Khuzhin --- src/Interpreters/InterpreterSelectQuery.cpp | 3 +-- .../MergeTree/MergeTreeWhereOptimizer.cpp | 16 ++++++++++++++++ src/Storages/MergeTree/MergeTreeWhereOptimizer.h | 3 +++ ...rge_prewhere_different_default_kind.reference | 9 +++++---- ...575_merge_prewhere_different_default_kind.sql | 11 +++++++---- 5 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/Interpreters/InterpreterSelectQuery.cpp b/src/Interpreters/InterpreterSelectQuery.cpp index 318ea5fdf42..a4eed606de1 100644 --- a/src/Interpreters/InterpreterSelectQuery.cpp +++ b/src/Interpreters/InterpreterSelectQuery.cpp @@ -593,8 +593,6 @@ InterpreterSelectQuery::InterpreterSelectQuery( Names queried_columns = syntax_analyzer_result->requiredSourceColumns(); const auto & supported_prewhere_columns = storage->supportedPrewhereColumns(); - if (supported_prewhere_columns.has_value()) - std::erase_if(queried_columns, [&](const auto & name) { return !supported_prewhere_columns->contains(name); }); MergeTreeWhereOptimizer{ current_info, @@ -602,6 +600,7 @@ InterpreterSelectQuery::InterpreterSelectQuery( std::move(column_compressed_sizes), metadata_snapshot, queried_columns, + supported_prewhere_columns, log}; } } diff --git a/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp b/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp index 3a866cc8934..fdddc29048b 100644 --- a/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp +++ b/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp @@ -32,10 +32,12 @@ MergeTreeWhereOptimizer::MergeTreeWhereOptimizer( std::unordered_map column_sizes_, const StorageMetadataPtr & metadata_snapshot, const Names & queried_columns_, + const std::optional & supported_columns_, Poco::Logger * log_) : table_columns{collections::map( metadata_snapshot->getColumns().getAllPhysical(), [](const NameAndTypePair & col) { return col.name; })} , queried_columns{queried_columns_} + , supported_columns{supported_columns_} , sorting_key_names{NameSet( metadata_snapshot->getSortingKey().column_names.begin(), metadata_snapshot->getSortingKey().column_names.end())} , block_with_constants{KeyCondition::getBlockWithConstants(query_info.query->clone(), query_info.syntax_analyzer_result, context)} @@ -195,6 +197,8 @@ void MergeTreeWhereOptimizer::analyzeImpl(Conditions & res, const ASTPtr & node, && (!is_final || isExpressionOverSortingKey(node)) /// Only table columns are considered. Not array joined columns. NOTE We're assuming that aliases was expanded. && isSubsetOfTableColumns(cond.identifiers) + /// Some identifiers can unable to support PREWHERE (usually because of different types in Merge engine) + && identifiersSupportsPrewhere(cond.identifiers) /// Do not move conditions involving all queried columns. && cond.identifiers.size() < queried_columns.size(); @@ -321,6 +325,18 @@ UInt64 MergeTreeWhereOptimizer::getIdentifiersColumnSize(const NameSet & identif return size; } +bool MergeTreeWhereOptimizer::identifiersSupportsPrewhere(const NameSet & identifiers) const +{ + if (!supported_columns.has_value()) + return true; + + for (const auto & identifier : identifiers) + if (!supported_columns->contains(identifier)) + return false; + + return true; +} + bool MergeTreeWhereOptimizer::isExpressionOverSortingKey(const ASTPtr & ast) const { if (const auto * func = ast->as()) diff --git a/src/Storages/MergeTree/MergeTreeWhereOptimizer.h b/src/Storages/MergeTree/MergeTreeWhereOptimizer.h index f37255bdbee..8953923542e 100644 --- a/src/Storages/MergeTree/MergeTreeWhereOptimizer.h +++ b/src/Storages/MergeTree/MergeTreeWhereOptimizer.h @@ -39,6 +39,7 @@ public: std::unordered_map column_sizes_, const StorageMetadataPtr & metadata_snapshot, const Names & queried_columns_, + const std::optional & supported_columns_, Poco::Logger * log_); private: @@ -82,6 +83,7 @@ private: void optimizeArbitrary(ASTSelectQuery & select) const; UInt64 getIdentifiersColumnSize(const NameSet & identifiers) const; + bool identifiersSupportsPrewhere(const NameSet & identifiers) const; bool isExpressionOverSortingKey(const ASTPtr & ast) const; @@ -105,6 +107,7 @@ private: const StringSet table_columns; const Names queried_columns; + const std::optional supported_columns; const NameSet sorting_key_names; const Block block_with_constants; Poco::Logger * log; diff --git a/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.reference b/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.reference index 32db2512eab..c17e235ddad 100644 --- a/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.reference +++ b/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.reference @@ -1,12 +1,13 @@ -- { echoOn } -- for pure PREWHERE it is not addressed yet. SELECT * FROM m PREWHERE a = 'OK'; -OK 0 +OK 1970-01-01 0 SELECT * FROM m PREWHERE f = 0; -- { serverError ILLEGAL_PREWHERE } SELECT * FROM m WHERE f = 0 SETTINGS optimize_move_to_prewhere=0; -OK 0 +OK 1970-01-01 0 SELECT * FROM m WHERE f = 0 SETTINGS optimize_move_to_prewhere=1; -OK 0 +OK 1970-01-01 0 -- { echoOn } SELECT * FROM m WHERE f = 0 SETTINGS optimize_move_to_prewhere=1; -OK 0 +OK 1970-01-01 0 +OK 1970-01-01 0 diff --git a/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.sql b/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.sql index 0f1d582a26e..88c7923a570 100644 --- a/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.sql +++ b/tests/queries/0_stateless/02575_merge_prewhere_different_default_kind.sql @@ -6,20 +6,22 @@ DROP TABLE IF EXISTS t2; CREATE TABLE m ( - `a` String, - `f` UInt8 + a String, + date Date, + f UInt8 ) ENGINE = Merge(currentDatabase(), '^(t1|t2)$'); CREATE TABLE t1 ( a String, + date Date, f UInt8 ALIAS 0 ) ENGINE = MergeTree ORDER BY tuple() SETTINGS index_granularity = 8192; -INSERT INTO t1 VALUES ('OK'); +INSERT INTO t1 (a) VALUES ('OK'); -- { echoOn } -- for pure PREWHERE it is not addressed yet. @@ -32,12 +34,13 @@ SELECT * FROM m WHERE f = 0 SETTINGS optimize_move_to_prewhere=1; CREATE TABLE t2 ( a String, + date Date, f UInt8, ) ENGINE = MergeTree ORDER BY tuple() SETTINGS index_granularity = 8192; -INSERT INTO t2 VALUES ('OK', 1); +INSERT INTO t2 (a) VALUES ('OK'); -- { echoOn } SELECT * FROM m WHERE f = 0 SETTINGS optimize_move_to_prewhere=1;