Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #51301 to 23.4: Fix fuzzer failure in ActionsDAG #51491

Merged
merged 1 commit into from Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 22 additions & 7 deletions src/Interpreters/ActionsDAG.cpp
Expand Up @@ -1874,10 +1874,10 @@ struct ConjunctionNodes
ActionsDAG::NodeRawConstPtrs rejected;
};

/// Take a node which result is predicate.
/// Take a node which result is a predicate.
/// Assuming predicate is a conjunction (probably, trivial).
/// Find separate conjunctions nodes. Split nodes into allowed and rejected sets.
/// Allowed predicate is a predicate which can be calculated using only nodes from allowed_nodes set.
/// Allowed predicate is a predicate which can be calculated using only nodes from the allowed_nodes set.
ConjunctionNodes getConjunctionNodes(ActionsDAG::Node * predicate, std::unordered_set<const ActionsDAG::Node *> allowed_nodes)
{
ConjunctionNodes conjunction;
Expand Down Expand Up @@ -2111,9 +2111,9 @@ ActionsDAGPtr ActionsDAG::cloneActionsForFilterPushDown(
Node * predicate = const_cast<Node *>(tryFindInOutputs(filter_name));
if (!predicate)
throw Exception(ErrorCodes::LOGICAL_ERROR,
"Output nodes for ActionsDAG do not contain filter column name {}. DAG:\n{}",
filter_name,
dumpDAG());
"Output nodes for ActionsDAG do not contain filter column name {}. DAG:\n{}",
filter_name,
dumpDAG());

/// If condition is constant let's do nothing.
/// It means there is nothing to push down or optimization was already applied.
Expand All @@ -2140,14 +2140,29 @@ ActionsDAGPtr ActionsDAG::cloneActionsForFilterPushDown(
}

auto conjunction = getConjunctionNodes(predicate, allowed_nodes);
if (conjunction.rejected.size() == 1 && WhichDataType{removeNullable(conjunction.rejected.front()->result_type)}.isFloat())

if (conjunction.allowed.empty())
return nullptr;

chassert(predicate->result_type);

if (conjunction.rejected.size() == 1)
{
chassert(conjunction.rejected.front()->result_type);

if (conjunction.allowed.front()->type == ActionType::COLUMN
&& !conjunction.rejected.front()->result_type->equals(*predicate->result_type))
{
/// No further optimization can be done
return nullptr;
}
}

auto actions = cloneActionsForConjunction(conjunction.allowed, all_inputs);
if (!actions)
return nullptr;

/// Now, when actions are created, update current DAG.
/// Now, when actions are created, update the current DAG.

if (conjunction.rejected.empty())
{
Expand Down
@@ -0,0 +1,7 @@
# These queries triggered a crash in old ClickHouse versions:

CREATE TEMPORARY TABLE a (key UInt32, ID LowCardinality(String));
CREATE TEMPORARY TABLE b (key UInt32);
SELECT * FROM b JOIN a USING (key) WHERE ID = '1' HAVING ID = '1';

# PS. Predicate pushdown does not work for LowCardinality(String), but it's another problem.