Skip to content

Commit

Permalink
[minor]: Fix rank calculation bug when empty order by is seen (#8567)
Browse files Browse the repository at this point in the history
* minor: fix  to support scalars

* Fix empty order by rank implementation

---------

Co-authored-by: comphead <comphead@ukr.net>
  • Loading branch information
mustafasrepo and comphead authored Dec 18, 2023
1 parent d65b51a commit ceead1c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 16 deletions.
13 changes: 10 additions & 3 deletions datafusion/physical-expr/src/window/rank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,16 @@ impl PartitionEvaluator for RankEvaluator {
// There is no argument, values are order by column values (where rank is calculated)
let range_columns = values;
let last_rank_data = get_row_at_idx(range_columns, row_idx)?;
let empty = self.state.last_rank_data.is_empty();
if empty || self.state.last_rank_data != last_rank_data {
self.state.last_rank_data = last_rank_data;
let new_rank_encountered =
if let Some(state_last_rank_data) = &self.state.last_rank_data {
// if rank data changes, new rank is encountered
state_last_rank_data != &last_rank_data
} else {
// First rank seen
true
};
if new_rank_encountered {
self.state.last_rank_data = Some(last_rank_data);
self.state.last_rank_boundary += self.state.current_group_count;
self.state.current_group_count = 1;
self.state.n_rank += 1;
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-expr/src/window/window_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ pub enum WindowFn {
#[derive(Debug, Clone, Default)]
pub struct RankState {
/// The last values for rank as these values change, we increase n_rank
pub last_rank_data: Vec<ScalarValue>,
pub last_rank_data: Option<Vec<ScalarValue>>,
/// The index where last_rank_boundary is started
pub last_rank_boundary: usize,
/// Keep the number of entries in current rank
Expand Down
30 changes: 18 additions & 12 deletions datafusion/sqllogictest/test_files/window.slt
Original file line number Diff line number Diff line change
Expand Up @@ -3801,19 +3801,25 @@ select rank() over (order by 1) rnk from (select 1 a union all select 2 a) x
1
1

# support scalar value in ORDER BY
query I
select dense_rank() over () rnk from (select 1 a union all select 2 a) x
----
1
1

# support scalar value in both ORDER BY and PARTITION BY, RANK function
# TODO: fix the test, some issue in RANK
#query IIIIII
#select rank() over (partition by 1 order by 1) rnk,
# rank() over (partition by a, 1 order by 1) rnk1,
# rank() over (partition by a, 1 order by a, 1) rnk2,
# rank() over (partition by 1) rnk3,
# rank() over (partition by null) rnk4,
# rank() over (partition by 1, null, a) rnk5
#from (select 1 a union all select 2 a) x
#----
#1 1 1 1 1 1
#1 1 1 1 1 1
query IIIIII
select rank() over (partition by 1 order by 1) rnk,
rank() over (partition by a, 1 order by 1) rnk1,
rank() over (partition by a, 1 order by a, 1) rnk2,
rank() over (partition by 1) rnk3,
rank() over (partition by null) rnk4,
rank() over (partition by 1, null, a) rnk5
from (select 1 a union all select 2 a) x
----
1 1 1 1 1 1
1 1 1 1 1 1

# support scalar value in both ORDER BY and PARTITION BY, ROW_NUMBER function
query IIIIII
Expand Down

0 comments on commit ceead1c

Please sign in to comment.