diff --git a/src/Processors/QueryPlan/Optimizations/removeRedundantOrderBy.cpp b/src/Processors/QueryPlan/Optimizations/removeRedundantOrderBy.cpp index c4721649d91..2e3790ee078 100644 --- a/src/Processors/QueryPlan/Optimizations/removeRedundantOrderBy.cpp +++ b/src/Processors/QueryPlan/Optimizations/removeRedundantOrderBy.cpp @@ -33,52 +33,53 @@ void tryRemoveRedundantOrderBy(QueryPlan::Node * root) QueryPlan::Node * current_node = frame.node; IQueryPlanStep * current_step = frame.node->step.get(); - if (!steps_affect_order.empty()) + + /// if there is parent node which can affect order and current step is sorting + /// then check if we can remove the sorting step (and corresponding expression step) + if (!steps_affect_order.empty() && typeid_cast(current_step)) { - if (SortingStep * ss = typeid_cast(current_step); ss) + auto try_to_remove_sorting_step = [&]() -> bool { - auto try_to_remove_sorting_step = [&]() -> bool + /// if there are LIMITs on top of ORDER BY, the ORDER BY is non-removable + /// if ORDER BY is with FILL WITH, it is non-removable + if (typeid_cast(steps_affect_order.back()) || typeid_cast(steps_affect_order.back()) + || typeid_cast(steps_affect_order.back())) + return false; + + bool remove_sorting = false; + /// (1) aggregation + if (const AggregatingStep * parent_aggr = typeid_cast(steps_affect_order.back()); parent_aggr) { - /// if there are LIMITs on top of ORDER BY, the ORDER BY is non-removable - /// if ORDER BY is with FILL WITH, it is non-removable - if (typeid_cast(steps_affect_order.back()) || typeid_cast(steps_affect_order.back()) - || typeid_cast(steps_affect_order.back())) - return false; - - bool remove_sorting = false; - /// (1) aggregation - if (const AggregatingStep * parent_aggr = typeid_cast(steps_affect_order.back()); parent_aggr) - { - /// check if it contains aggregation functions which depends on order - } - /// (2) sorting - else if (SortingStep * parent_sorting = typeid_cast(steps_affect_order.back()); parent_sorting) - { - remove_sorting = true; - } - - if (remove_sorting) - { - /// need to remove sorting and its expression from plan - QueryPlan::Node * parent = frame.parent_node; - - QueryPlan::Node * next_node = !current_node->children.empty() ? current_node->children.front() : nullptr; - if (next_node && typeid_cast(next_node->step.get())) - next_node = !current_node->children.empty() ? current_node->children.front() : nullptr; - - if (next_node) - parent->children[0] = next_node; - } - return remove_sorting; - }; - if (try_to_remove_sorting_step()) - { - /// current step was removed from plan, its parent has new children, need to visit them - for (auto * child : frame.parent_node->children) - stack.push_back({.node = child, .parent_node = frame.parent_node}); - - continue; + /// TODO: check if it contains aggregation functions which depends on order + remove_sorting = true; } + /// (2) sorting + else if (SortingStep * parent_sorting = typeid_cast(steps_affect_order.back()); parent_sorting) + { + remove_sorting = true; + } + + if (remove_sorting) + { + /// need to remove sorting and its expression from plan + QueryPlan::Node * parent = frame.parent_node; + + QueryPlan::Node * next_node = !current_node->children.empty() ? current_node->children.front() : nullptr; + if (next_node && typeid_cast(next_node->step.get())) + next_node = !current_node->children.empty() ? current_node->children.front() : nullptr; + + if (next_node) + parent->children[0] = next_node; + } + return remove_sorting; + }; + if (try_to_remove_sorting_step()) + { + /// current sorting step has been removed from plan, its parent has new children, need to visit them + for (auto * child : frame.parent_node->children) + stack.push_back({.node = child, .parent_node = frame.parent_node}); + + continue; } } @@ -111,5 +112,4 @@ void tryRemoveRedundantOrderBy(QueryPlan::Node * root) } } } - } diff --git a/tests/queries/0_stateless/02496_remove_redundant_order_by.reference b/tests/queries/0_stateless/02496_remove_redundant_order_by.reference index 933e5ba7209..59f39b84c97 100644 --- a/tests/queries/0_stateless/02496_remove_redundant_order_by.reference +++ b/tests/queries/0_stateless/02496_remove_redundant_order_by.reference @@ -1,5 +1,5 @@ -- Disable query_plan_remove_redundant_order_by --- ORDER BY(s) in subqueries are untouched +-- ORDER BY clauses in subqueries are untouched Expression (Project names) Header: number UInt64 Sorting (Sorting for ORDER BY) @@ -17,7 +17,7 @@ Header: number UInt64 ReadFromStorage (SystemNumbers) Header: number UInt64 -- Enable query_plan_remove_redundant_order_by --- ORDER BY eliminates ORDER BY(s) in subqueries +-- ORDER BY removes ORDER BY clauses in subqueries Expression (Project names) Header: number UInt64 Sorting (Sorting for ORDER BY) @@ -26,7 +26,7 @@ Header: number UInt64 Header: number_0 UInt64 ReadFromStorage (SystemNumbers) Header: number UInt64 --- ORDER BY cannot remove ORDER BY in subquery with ORDER BY WITH FILL +-- ORDER BY cannot remove ORDER BY in subquery WITH FILL Expression (Project names) Header: number UInt64 Sorting (Sorting for ORDER BY) @@ -58,3 +58,12 @@ Header: number UInt64 Header: number_1 UInt64 ReadFromStorage (SystemNumbers) Header: number UInt64 +-- GROUP BY removes ORDER BY in _all_ subqueries +Expression ((Project names + Projection)) +Header: number UInt64 + Aggregating + Header: number_0 UInt64 + Expression ((Before GROUP BY + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + (Change column names to column identifiers + (Project names + (Before ORDER BY + (Projection + Change column names to column identifiers)))))))))) + Header: number_0 UInt64 + ReadFromStorage (SystemNumbers) + Header: number UInt64 diff --git a/tests/queries/0_stateless/02496_remove_redundant_order_by.sql b/tests/queries/0_stateless/02496_remove_redundant_order_by.sql index bfc38990f53..fa8381ec5f8 100644 --- a/tests/queries/0_stateless/02496_remove_redundant_order_by.sql +++ b/tests/queries/0_stateless/02496_remove_redundant_order_by.sql @@ -4,7 +4,7 @@ SET optimize_duplicate_order_by_and_distinct=0; SELECT '-- Disable query_plan_remove_redundant_order_by'; SET query_plan_remove_redundant_order_by=0; -SELECT '-- ORDER BY(s) in subqueries are untouched'; +SELECT '-- ORDER BY clauses in subqueries are untouched'; EXPLAIN header=1 SELECT * FROM @@ -23,7 +23,7 @@ ORDER BY number ASC; SELECT '-- Enable query_plan_remove_redundant_order_by'; SET query_plan_remove_redundant_order_by=1; -SELECT '-- ORDER BY eliminates ORDER BY(s) in subqueries'; +SELECT '-- ORDER BY removes ORDER BY clauses in subqueries'; EXPLAIN header=1 SELECT * FROM @@ -39,7 +39,7 @@ FROM ) ORDER BY number ASC; -SELECT '-- ORDER BY cannot remove ORDER BY in subquery with ORDER BY WITH FILL'; +SELECT '-- ORDER BY cannot remove ORDER BY in subquery WITH FILL'; EXPLAIN header=1 SELECT * FROM @@ -72,21 +72,21 @@ FROM ) ORDER BY number ASC; --- SELECT '-- GROUP BY eliminates ORDER BY in _all_ subqueries'; --- EXPLAIN --- SELECT * --- FROM --- ( --- SELECT * --- FROM --- ( --- SELECT * --- FROM numbers(3) --- ORDER BY number ASC --- ) --- ORDER BY number DESC --- ) --- GROUP BY number; +SELECT '-- GROUP BY removes ORDER BY in _all_ subqueries'; +EXPLAIN header=1 +SELECT * +FROM +( + SELECT * + FROM + ( + SELECT * + FROM numbers(3) + ORDER BY number ASC + ) + ORDER BY number DESC +) +GROUP BY number; -- SELECT '-- GROUP BY with aggregation function which does NOT depend on order -> eliminate ORDER BY(s) in _all_ subqueries'; -- EXPLAIN @@ -224,4 +224,3 @@ ORDER BY number ASC; -- ORDER BY number DESC -- ) AS t2 -- ORDER BY t1.number ASC; -