kosiew commented on code in PR #26160: URL: https://github.com/apache/datafusion/pull/26160#discussion_r4423271019
########## datafusion/sqllogictest/test_files/window_topn.slt: ########## @@ -941,30 +941,61 @@ statement ok DROP TABLE window_topn_rank_t; ############################################################################### -# RANK() — equality predicate (negative: rule supports only </<=/>=/>) +# Ranking functions — equality with one ############################################################################### # -# `extract_window_limit` matches only `<, <=, >, >=`. Equality predicates -# `rk = N` are NOT optimized by this rule (regardless of N). DuckDB -# special-cases `rk = 1` as equivalent to `rk <= 1`; we don't. The two -# tests below pin current behavior so that an accidental rule extension -# (or regression) shows up. +# Ranking functions return positive integers, so `rank = 1` is equivalent to +# `rank <= 1`. Equality with larger values must retain the filter. statement ok CREATE TABLE window_topn_rank_eq_t (id INT, pk INT, val INT) AS VALUES - (1, 1, 10), (2, 1, 20), (3, 1, 30), + (1, 1, 10), (2, 1, 10), (3, 1, 20), (4, 2, 5), (5, 2, 15), (6, 2, 25); -# Test R12: `rk = 1` — correct results, but plan should still contain -# FilterExec + SortExec (rule did NOT fire). +# Test R12: `rk = 1` keeps all tied winners. query III rowsort SELECT id, pk, val FROM ( SELECT *, RANK() OVER (PARTITION BY pk ORDER BY val) as rk FROM window_topn_rank_eq_t ) WHERE rk = 1; ---- 1 1 10 +2 1 10 +4 2 5 + +query III rowsort +SELECT id, pk, val FROM ( + SELECT *, RANK() OVER (PARTITION BY pk ORDER BY val) as rk FROM window_topn_rank_eq_t +) WHERE 1 = rk; +---- +1 1 10 +2 1 10 4 2 5 +query III rowsort +SELECT id, pk, val FROM ( + SELECT *, DENSE_RANK() OVER (PARTITION BY pk ORDER BY val) as rk FROM window_topn_rank_eq_t Review Comment: Optional: Could we add compact EXPLAIN checks for the new ROW_NUMBER and DENSE_RANK equality cases, asserting the corresponding PartitionedTopKExec function and `fetch=1`? Their result tests would also pass without the optimization, while the current positive plan assertion covers only RANK. -- 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]
