From eb85c5a8e72450e5690ae4ae69f07697bacd5347 Mon Sep 17 00:00:00 2001 From: vdimir Date: Fri, 18 Jun 2021 17:28:52 +0300 Subject: [PATCH 1/3] Fix assert with non uint8 in prewhere --- src/Columns/FilterDescription.cpp | 14 -------------- src/Columns/FilterDescription.h | 3 --- src/Interpreters/ExpressionAnalyzer.cpp | 17 ++--------------- src/Interpreters/ExpressionAnalyzer.h | 2 -- .../MergeTree/MergeTreeBaseSelectProcessor.cpp | 11 +++++++++-- .../01917_prewhere_column_type.reference | 1 + .../0_stateless/01917_prewhere_column_type.sql | 16 ++++++++++++++++ 7 files changed, 28 insertions(+), 36 deletions(-) create mode 100644 tests/queries/0_stateless/01917_prewhere_column_type.reference create mode 100644 tests/queries/0_stateless/01917_prewhere_column_type.sql diff --git a/src/Columns/FilterDescription.cpp b/src/Columns/FilterDescription.cpp index d216094eaab..c9968d841c2 100644 --- a/src/Columns/FilterDescription.cpp +++ b/src/Columns/FilterDescription.cpp @@ -87,18 +87,4 @@ FilterDescription::FilterDescription(const IColumn & column_) ErrorCodes::ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER); } - -void checkColumnCanBeUsedAsFilter(const ColumnWithTypeAndName & column_elem) -{ - ConstantFilterDescription const_filter; - if (column_elem.column) - const_filter = ConstantFilterDescription(*column_elem.column); - - if (!const_filter.always_false && !const_filter.always_true) - { - auto column = column_elem.column ? column_elem.column : column_elem.type->createColumn(); - FilterDescription filter(*column); - } -} - } diff --git a/src/Columns/FilterDescription.h b/src/Columns/FilterDescription.h index 89474ea523c..05812fea283 100644 --- a/src/Columns/FilterDescription.h +++ b/src/Columns/FilterDescription.h @@ -32,7 +32,4 @@ struct FilterDescription struct ColumnWithTypeAndName; -/// Will throw an exception if column_elem is cannot be used as a filter column. -void checkColumnCanBeUsedAsFilter(const ColumnWithTypeAndName & column_elem); - } diff --git a/src/Interpreters/ExpressionAnalyzer.cpp b/src/Interpreters/ExpressionAnalyzer.cpp index fe52b30da7b..fd3d4fc8781 100644 --- a/src/Interpreters/ExpressionAnalyzer.cpp +++ b/src/Interpreters/ExpressionAnalyzer.cpp @@ -953,10 +953,8 @@ ActionsDAGPtr SelectQueryExpressionAnalyzer::appendPrewhere( ExpressionActionsChain & chain, bool only_types, const Names & additional_required_columns) { const auto * select_query = getSelectQuery(); - ActionsDAGPtr prewhere_actions; - if (!select_query->prewhere()) - return prewhere_actions; + return nullptr; Names first_action_names; if (!chain.steps.empty()) @@ -973,6 +971,7 @@ ActionsDAGPtr SelectQueryExpressionAnalyzer::appendPrewhere( throw Exception("Invalid type for filter in PREWHERE: " + filter_type->getName(), ErrorCodes::ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER); + ActionsDAGPtr prewhere_actions; { /// Remove unused source_columns from prewhere actions. auto tmp_actions_dag = std::make_shared(sourceColumns()); @@ -1038,18 +1037,6 @@ ActionsDAGPtr SelectQueryExpressionAnalyzer::appendPrewhere( return prewhere_actions; } -void SelectQueryExpressionAnalyzer::appendPreliminaryFilter(ExpressionActionsChain & chain, ActionsDAGPtr actions_dag, String column_name) -{ - ExpressionActionsChain::Step & step = chain.lastStep(sourceColumns()); - - // FIXME: assert(filter_info); - auto * expression_step = typeid_cast(&step); - expression_step->actions_dag = std::move(actions_dag); - step.addRequiredOutput(column_name); - - chain.addStep(); -} - bool SelectQueryExpressionAnalyzer::appendWhere(ExpressionActionsChain & chain, bool only_types) { const auto * select_query = getSelectQuery(); diff --git a/src/Interpreters/ExpressionAnalyzer.h b/src/Interpreters/ExpressionAnalyzer.h index 70ff5643b7c..2427f65c694 100644 --- a/src/Interpreters/ExpressionAnalyzer.h +++ b/src/Interpreters/ExpressionAnalyzer.h @@ -357,8 +357,6 @@ private: ArrayJoinActionPtr appendArrayJoin(ExpressionActionsChain & chain, ActionsDAGPtr & before_array_join, bool only_types); bool appendJoinLeftKeys(ExpressionActionsChain & chain, bool only_types); JoinPtr appendJoin(ExpressionActionsChain & chain); - /// Add preliminary rows filtration. Actions are created in other expression analyzer to prevent any possible alias injection. - void appendPreliminaryFilter(ExpressionActionsChain & chain, ActionsDAGPtr actions_dag, String column_name); /// remove_filter is set in ExpressionActionsChain::finalize(); /// Columns in `additional_required_columns` will not be removed (they can be used for e.g. sampling or FINAL modifier). ActionsDAGPtr appendPrewhere(ExpressionActionsChain & chain, bool only_types, const Names & additional_required_columns); diff --git a/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp b/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp index d9cb949042c..4ff593ea1c1 100644 --- a/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp +++ b/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp @@ -17,6 +17,7 @@ namespace DB namespace ErrorCodes { + extern const int ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER; extern const int LOGICAL_ERROR; } @@ -430,8 +431,14 @@ void MergeTreeBaseSelectProcessor::executePrewhereActions(Block & block, const P block.erase(prewhere_info->prewhere_column_name); else { - auto & ctn = block.getByName(prewhere_info->prewhere_column_name); - ctn.column = ctn.type->createColumnConst(block.rows(), 1u)->convertToFullColumnIfConst(); + WhichDataType which(prewhere_column.type); + if (which.isInt() || which.isUInt()) + prewhere_column.column = prewhere_column.type->createColumnConst(block.rows(), 1u)->convertToFullColumnIfConst(); + else if (which.isFloat()) + prewhere_column.column = prewhere_column.type->createColumnConst(block.rows(), 1.0f)->convertToFullColumnIfConst(); + else + throw Exception("Illegal type " + prewhere_column.type->getName() + " of column for filter.", + ErrorCodes::ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER); } } } diff --git a/tests/queries/0_stateless/01917_prewhere_column_type.reference b/tests/queries/0_stateless/01917_prewhere_column_type.reference new file mode 100644 index 00000000000..58c9bdf9d01 --- /dev/null +++ b/tests/queries/0_stateless/01917_prewhere_column_type.reference @@ -0,0 +1 @@ +111 diff --git a/tests/queries/0_stateless/01917_prewhere_column_type.sql b/tests/queries/0_stateless/01917_prewhere_column_type.sql new file mode 100644 index 00000000000..4046eb4d891 --- /dev/null +++ b/tests/queries/0_stateless/01917_prewhere_column_type.sql @@ -0,0 +1,16 @@ +DROP TABLE IF EXISTS t1; + +CREATE TABLE t1 ( s String, f Float32, e UInt16 ) ENGINE = MergeTree ORDER BY tuple(); + +INSERT INTO t1 VALUES ('111', 1, 1); + +SELECT s FROM t1 WHERE f AND (e = 1); -- { serverError 59 } +SELECT s FROM t1 PREWHERE f; -- { serverError 59 } +SELECT s FROM t1 PREWHERE f WHERE (e = 1); -- { serverError 59 } +SELECT s FROM t1 PREWHERE f WHERE f AND (e = 1); -- { serverError 59 } + +SELECT s FROM t1 WHERE e AND (e = 1); +SELECT s FROM t1 PREWHERE e; -- { serverError 59 } +SELECT s FROM t1 PREWHERE e WHERE (e = 1); -- { serverError 59 } +SELECT s FROM t1 PREWHERE e WHERE f AND (e = 1); -- { serverError 59 } + From 46a5dd67013311592dd30065c7bde659851c6e99 Mon Sep 17 00:00:00 2001 From: vdimir Date: Mon, 21 Jun 2021 16:01:02 +0300 Subject: [PATCH 2/3] Fix MergeTreeBaseSelectProcessor::executePrewhereActions --- src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp b/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp index 4ff593ea1c1..5f0a0f298af 100644 --- a/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp +++ b/src/Storages/MergeTree/MergeTreeBaseSelectProcessor.cpp @@ -431,7 +431,7 @@ void MergeTreeBaseSelectProcessor::executePrewhereActions(Block & block, const P block.erase(prewhere_info->prewhere_column_name); else { - WhichDataType which(prewhere_column.type); + WhichDataType which(removeNullable(recursiveRemoveLowCardinality(prewhere_column.type))); if (which.isInt() || which.isUInt()) prewhere_column.column = prewhere_column.type->createColumnConst(block.rows(), 1u)->convertToFullColumnIfConst(); else if (which.isFloat()) From ec834fe213fb37d9af5cc91a03f88e23877f928b Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sun, 27 Jun 2021 19:31:55 +0300 Subject: [PATCH 3/3] Update 01917_prewhere_column_type.sql --- tests/queries/0_stateless/01917_prewhere_column_type.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/01917_prewhere_column_type.sql b/tests/queries/0_stateless/01917_prewhere_column_type.sql index 4046eb4d891..5147e6093a9 100644 --- a/tests/queries/0_stateless/01917_prewhere_column_type.sql +++ b/tests/queries/0_stateless/01917_prewhere_column_type.sql @@ -1,6 +1,6 @@ DROP TABLE IF EXISTS t1; -CREATE TABLE t1 ( s String, f Float32, e UInt16 ) ENGINE = MergeTree ORDER BY tuple(); +CREATE TABLE t1 ( s String, f Float32, e UInt16 ) ENGINE = MergeTree ORDER BY tuple() SETTINGS min_bytes_for_wide_part = '100G'; INSERT INTO t1 VALUES ('111', 1, 1);