thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-11-10 01:25:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

ActionsDAG: remove wrong optimization

Browse Source 
Replacing AND with a casto to UInt8 can change the condition.
e.g.
0x100 cast to UInt8 is 0, but it should be 1.
0.1 cast to UInt8 is 0, but it should be 1.

Basically we can't cast anything to UInt8 so we completely remove
this optimization.

Closes #47317
This commit is contained in:
Salvatore Mesoraca 2023-03-15 17:01:19 +01:00
parent 52b6976822
commit d7f4bc4c72
No known key found for this signature in database
GPG Key ID: 0567E50A25403074
3 changed files with 66 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
src/Interpreters/ActionsDAG.cpp
View File

@ -1966,8 +1966,12 @@ ActionsDAGPtr ActionsDAG::cloneActionsForFilterPushDown(
}
auto conjunction = getConjunctionNodes(predicate, allowed_nodes);
if (conjunction.rejected.size() == 1 && WhichDataType{removeNullable(conjunction.rejected.front()->result_type)}.isFloat())
if (conjunction.rejected.size() == 1 && !conjunction.rejected.front()->result_type->equals(*predicate->result_type)
&& conjunction.allowed.front()->type == ActionType::COLUMN)
{
// No further optimization can be done
return nullptr;
}
auto actions = cloneActionsForConjunction(conjunction.allowed, all_inputs);
if (!actions)
@ -2017,55 +2021,26 @@ ActionsDAGPtr ActionsDAG::cloneActionsForFilterPushDown(
else
{
/// Predicate is conjunction, where both allowed and rejected sets are not empty.
/// Replace this node to conjunction of rejected predicates.
NodeRawConstPtrs new_children = std::move(conjunction.rejected);
if (new_children.size() == 1)
if (new_children.size() == 1 && new_children.front()->result_type->equals(*predicate->result_type))
{
/// Rejected set has only one predicate.
if (new_children.front()->result_type->equals(*predicate->result_type))
{
/// If it's type is same, just add alias.
Node node;
node.type = ActionType::ALIAS;
node.result_name = predicate->result_name;
node.result_type = predicate->result_type;
node.children.swap(new_children);
*predicate = std::move(node);
}
else if (!WhichDataType{removeNullable(new_children.front()->result_type)}.isFloat())
{
/// If type is different, cast column.
/// This case is possible, cause AND can use any numeric type as argument.
/// But casting floats to UInt8 or Bool produces different results.
/// so we can't apply this optimization to them.
Node node;
node.type = ActionType::COLUMN;
node.result_name = predicate->result_type->getName();
node.column = DataTypeString().createColumnConst(0, node.result_name);
node.result_type = std::make_shared<DataTypeString>();
const auto * right_arg = &nodes.emplace_back(std::move(node));
const auto * left_arg = new_children.front();
predicate->children = {left_arg, right_arg};
auto arguments = prepareFunctionArguments(predicate->children);
FunctionOverloadResolverPtr func_builder_cast = CastInternalOverloadResolver<CastType::nonAccurate>::createImpl();
predicate->function_base = func_builder_cast->build(arguments);
predicate->function = predicate->function_base->prepare(arguments);
}
/// Rejected set has only one predicate. And the type is the same as the result_type.
/// Just add alias.
Node node;
node.type = ActionType::ALIAS;
node.result_name = predicate->result_name;
node.result_type = predicate->result_type;
node.children.swap(new_children);
*predicate = std::move(node);
}
else
{
/// Predicate is function AND, which still have more then one argument.
/// Or there is only one argument that is a float and we can't just
/// remove the AND.
/// Predicate is function AND, which still have more then one argument
/// or it has one argument of the wrong type.
/// Just update children and rebuild it.
predicate->children.swap(new_children);
if (WhichDataType{removeNullable(predicate->children.front()->result_type)}.isFloat())
if (new_children.size() == 1)
{
Node node;
node.type = ActionType::COLUMN;
@ -2073,8 +2048,9 @@ ActionsDAGPtr ActionsDAG::cloneActionsForFilterPushDown(
node.column = DataTypeUInt8().createColumnConst(0, 1u);
node.result_type = std::make_shared<DataTypeUInt8>();
const auto * const_col = &nodes.emplace_back(std::move(node));
predicate->children.emplace_back(const_col);
new_children.emplace_back(const_col);
}
predicate->children.swap(new_children);
auto arguments = prepareFunctionArguments(predicate->children);
FunctionOverloadResolverPtr func_builder_and = std::make_unique<FunctionToOverloadResolverAdaptor>(std::make_shared<FunctionAnd>());

5
tests/queries/0_stateless/02568_and_consistency.reference Normal file
View File

@ -0,0 +1,5 @@
=
1554690688
=
1554690688
=

42
tests/queries/0_stateless/02568_and_consistency.sql Normal file
View File

@ -0,0 +1,42 @@
DROP TABLE IF EXISTS t1;
CREATE TABLE t1 (c0 Int32, PRIMARY KEY (c0)) ENGINE=MergeTree;
INSERT INTO t1 VALUES (1554690688);
select '=';
SELECT MIN(t1.c0)
FROM t1
GROUP BY
(-sign(cos(t1.c0))) * (-max2(t1.c0, t1.c0 / t1.c0)),
t1.c0 * t1.c0,
sign(-exp(-t1.c0))
HAVING -(-(MIN(t1.c0) + MIN(t1.c0))) AND (pow('{b' > '-657301241', log(-1004522121)) IS NOT NULL)
UNION ALL
SELECT MIN(t1.c0)
FROM t1
GROUP BY
(-sign(cos(t1.c0))) * (-max2(t1.c0, t1.c0 / t1.c0)),
t1.c0 * t1.c0,
sign(-exp(-t1.c0))
HAVING NOT (-(-(MIN(t1.c0) + MIN(t1.c0))) AND (pow('{b' > '-657301241', log(-1004522121)) IS NOT NULL))
UNION ALL
SELECT MIN(t1.c0)
FROM t1
GROUP BY
(-sign(cos(t1.c0))) * (-max2(t1.c0, t1.c0 / t1.c0)),
t1.c0 * t1.c0,
sign(-exp(-t1.c0))
HAVING (-(-(MIN(t1.c0) + MIN(t1.c0))) AND (pow('{b' > '-657301241', log(-1004522121)) IS NOT NULL)) IS NULL
SETTINGS aggregate_functions_null_for_empty = 1, enable_optimize_predicate_expression = 0;
select '=';
SELECT MIN(t1.c0)
FROM t1
GROUP BY t1.c0
HAVING and(MIN(t1.c0) + MIN(t1.c0), 1)
SETTINGS aggregate_functions_null_for_empty = 1, enable_optimize_predicate_expression = 0;
select '=';
DROP TABLE IF EXISTS t1;