Fix review comments

This commit is contained in:
Igor Nikonov 2023-02-02 11:46:13 +00:00
parent 996b833faa
commit 32ab4dc693

View File

@ -36,22 +36,15 @@ namespace
}
return non_const_columns;
}
}
size_t tryRemoveRedundantDistinct(QueryPlan::Node * parent_node, QueryPlan::Nodes & /* nodes*/)
bool canRemoveDistinct(const QueryPlan::Node * distinct_node)
{
if (parent_node->children.empty())
return 0;
/// check if it is preliminary distinct node
QueryPlan::Node * distinct_node = parent_node->children.front();
DistinctStep * distinct_step = typeid_cast<DistinctStep *>(distinct_node->step.get());
if (!distinct_step)
return 0;
const DistinctStep * distinct_step = typeid_cast<DistinctStep *>(distinct_node->step.get());
chassert(distinct_step);
std::vector<ActionsDAGPtr> dag_stack;
const DistinctStep * inner_distinct_step = nullptr;
QueryPlan::Node * node = distinct_node;
const QueryPlan::Node * node = distinct_node;
while (!node->children.empty())
{
const IQueryPlanStep * current_step = node->step.get();
@ -72,7 +65,7 @@ size_t tryRemoveRedundantDistinct(QueryPlan::Node * parent_node, QueryPlan::Node
break;
}
if (!inner_distinct_step)
return 0;
return false;
/// possible cases (outer distinct -> inner distinct):
/// final -> preliminary => do nothing
@ -80,12 +73,12 @@ size_t tryRemoveRedundantDistinct(QueryPlan::Node * parent_node, QueryPlan::Node
/// final -> final => try remove final
/// preliminary -> preliminary => logical error?
if (inner_distinct_step->isPreliminary())
return 0;
return false;
const auto distinct_columns = getDistinctColumns(distinct_step);
auto inner_distinct_columns = getDistinctColumns(inner_distinct_step);
if (distinct_columns.size() != inner_distinct_columns.size())
return 0;
return false;
ActionsDAGPtr path_actions;
if (!dag_stack.empty())
@ -107,11 +100,11 @@ size_t tryRemoveRedundantDistinct(QueryPlan::Node * parent_node, QueryPlan::Node
{
const auto * alias_node = path_actions->getOriginalNodeForOutputAlias(String(column));
if (!alias_node)
return 0;
return false;
auto it = inner_distinct_columns.find(alias_node->result_name);
if (it == inner_distinct_columns.end())
return 0;
return false;
inner_distinct_columns.erase(it);
}
@ -119,14 +112,37 @@ size_t tryRemoveRedundantDistinct(QueryPlan::Node * parent_node, QueryPlan::Node
else
{
if (distinct_columns != inner_distinct_columns)
return 0;
return false;
}
return true;
}
}
///
/// DISTINCT is redundant if DISTINCT on the same columns was executed before
/// Trivial example: SELECT DISTINCT * FROM (SELECT DISTINCT * FROM numbers(3))
///
size_t tryRemoveRedundantDistinct(QueryPlan::Node * parent_node, QueryPlan::Nodes & /* nodes*/)
{
bool applied = false;
for (const auto * node : parent_node->children)
{
/// check if it is distinct node
const DistinctStep * distinct_step = typeid_cast<DistinctStep *>(node->step.get());
if (!distinct_step)
continue;
if (canRemoveDistinct(node))
{
/// remove current distinct
chassert(!distinct_node->children.empty());
parent_node->children[0] = distinct_node->children.front();
chassert(!node->children.empty());
parent_node->children[0] = node->children.front();
applied = true;
}
}
return 1;
return applied;
}
}