SubhamSinghal commented on code in PR #23869:
URL: https://github.com/apache/datafusion/pull/23869#discussion_r3667311772


##########
datafusion/physical-optimizer/src/window_topn.rs:
##########
@@ -172,10 +182,10 @@ impl WindowTopN {
             return None;
         }
 
-        // For RANK: an empty ORDER BY makes every row tie at rank 1 —
-        // the optimization is degenerate (we'd retain the entire input)
-        // and tie storage would be unbounded.
-        if matches!(fn_kind, WindowFnKind::Rank)
+        // For RANK / DENSE_RANK: an empty ORDER BY makes every row tie
+        // at rank 1 — the optimization is degenerate (we'd retain the
+        // entire input) and tie storage would be unbounded.
+        if matches!(fn_kind, WindowFnKind::Rank | WindowFnKind::DenseRank)

Review Comment:
    Fixed in 4530bba18ccbf135cf2b1ef85e37fd321cb946d7. Replaced 
order_by().is_empty() with a structural guard: sort_exec.expr().len() <= 
partition_prefix_len → bail. Catches `PARTITION BY pk ORDER BY pk` (and 
no-ORDER-BY ROW_NUMBER). Added SLT regression.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to