Merge pull request #25484 from vdimir/issue-19589

Fix assert with non uint8 in prewhere

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);
-    }
-}
-
 }

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);
-
 }

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<ActionsDAG>(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<ExpressionActionsChain::ExpressionActionsStep *>(&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();

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);

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);
+            WhichDataType which(removeNullable(recursiveRemoveLowCardinality(prewhere_column.type)));
-            ctn.column = ctn.type->createColumnConst(block.rows(), 1u)->convertToFullColumnIfConst();
+            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);
         }
     }
 }

tests/queries/0_stateless/01917_prewhere_column_type.reference

@@ -0,0 +1 @@
+111

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() SETTINGS min_bytes_for_wide_part = '100G';
+
+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 }
+