From 5e7f0db768120e987bd4604e36e181e895f5384b Mon Sep 17 00:00:00 2001 From: Jiayu Liu Date: Sat, 5 Jun 2021 14:23:11 +0800 Subject: [PATCH] privatize rank --- datafusion/src/physical_plan/window_frames.rs | 100 ++++++++---------- 1 file changed, 44 insertions(+), 56 deletions(-) diff --git a/datafusion/src/physical_plan/window_frames.rs b/datafusion/src/physical_plan/window_frames.rs index 3cfa03e481ac7..a4fd7c6d2d7ac 100644 --- a/datafusion/src/physical_plan/window_frames.rs +++ b/datafusion/src/physical_plan/window_frames.rs @@ -110,7 +110,7 @@ impl Default for WindowFrame { /// 5. UNBOUNDED FOLLOWING /// /// in this implementation we'll only allow to be u64 (i.e. no dynamic boundary) -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, Eq)] pub enum WindowFrameBound { /// 1. UNBOUNDED PRECEDING /// The frame boundary is the first row in the partition. @@ -156,19 +156,31 @@ impl fmt::Display for WindowFrameBound { } } +impl PartialEq for WindowFrameBound { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + impl PartialOrd for WindowFrameBound { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } +impl Ord for WindowFrameBound { + fn cmp(&self, other: &Self) -> Ordering { + self.get_rank().cmp(&other.get_rank()) + } +} + impl WindowFrameBound { /// get the rank of this window frame bound. /// /// the rank is a tuple of (u8, u64) because we'll firstly compare the kind and then the value /// which requires special handling e.g. with preceding the larger the value the smaller the /// rank and also for 0 preceding / following it is the same as current row - pub fn get_rank(&self) -> (u8, u64) { + fn get_rank(&self) -> (u8, u64) { match self { WindowFrameBound::Preceding(None) => (0, 0), WindowFrameBound::Following(None) => (4, 0), @@ -181,12 +193,6 @@ impl WindowFrameBound { } } -impl Ord for WindowFrameBound { - fn cmp(&self, other: &Self) -> Ordering { - self.get_rank().cmp(&other.get_rank()) - } -} - /// There are three frame types: ROWS, GROUPS, and RANGE. The frame type determines how the /// starting and ending boundaries of the frame are measured. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -268,83 +274,65 @@ mod tests { } #[test] - fn test_get_rank_eq() { + fn test_eq() { + assert_eq!(WindowFrameBound::CurrentRow, WindowFrameBound::CurrentRow); assert_eq!( - WindowFrameBound::CurrentRow.get_rank(), - WindowFrameBound::CurrentRow.get_rank() + WindowFrameBound::Preceding(Some(0)), + WindowFrameBound::CurrentRow ); assert_eq!( - WindowFrameBound::Preceding(Some(0)).get_rank(), - WindowFrameBound::CurrentRow.get_rank() + WindowFrameBound::CurrentRow, + WindowFrameBound::Following(Some(0)) ); assert_eq!( - WindowFrameBound::CurrentRow.get_rank(), - WindowFrameBound::Following(Some(0)).get_rank() + WindowFrameBound::Following(Some(2)), + WindowFrameBound::Following(Some(2)) ); assert_eq!( - WindowFrameBound::Following(Some(2)).get_rank(), - WindowFrameBound::Following(Some(2)).get_rank() + WindowFrameBound::Following(None), + WindowFrameBound::Following(None) ); assert_eq!( - WindowFrameBound::Following(None).get_rank(), - WindowFrameBound::Following(None).get_rank() + WindowFrameBound::Preceding(Some(2)), + WindowFrameBound::Preceding(Some(2)) ); assert_eq!( - WindowFrameBound::Preceding(Some(2)).get_rank(), - WindowFrameBound::Preceding(Some(2)).get_rank() - ); - assert_eq!( - WindowFrameBound::Preceding(None).get_rank(), - WindowFrameBound::Preceding(None).get_rank() + WindowFrameBound::Preceding(None), + WindowFrameBound::Preceding(None) ); } #[test] - fn test_get_rank_cmp() { - assert!( - WindowFrameBound::Preceding(Some(1)).get_rank() - < WindowFrameBound::CurrentRow.get_rank() - ); + fn test_ord() { + assert!(WindowFrameBound::Preceding(Some(1)) < WindowFrameBound::CurrentRow); // ! yes this is correct! assert!( - WindowFrameBound::Preceding(Some(2)).get_rank() - < WindowFrameBound::Preceding(Some(1)).get_rank() - ); - assert!( - WindowFrameBound::Preceding(Some(u64::MAX)).get_rank() - < WindowFrameBound::Preceding(Some(u64::MAX - 1)).get_rank() - ); - assert!( - WindowFrameBound::Preceding(None).get_rank() - < WindowFrameBound::Preceding(Some(1000000)).get_rank() - ); - assert!( - WindowFrameBound::Preceding(None).get_rank() - < WindowFrameBound::Preceding(Some(u64::MAX)).get_rank() + WindowFrameBound::Preceding(Some(2)) < WindowFrameBound::Preceding(Some(1)) ); assert!( - WindowFrameBound::Preceding(None).get_rank() - < WindowFrameBound::Following(Some(0)).get_rank() + WindowFrameBound::Preceding(Some(u64::MAX)) + < WindowFrameBound::Preceding(Some(u64::MAX - 1)) ); assert!( - WindowFrameBound::Preceding(Some(1)).get_rank() - < WindowFrameBound::Following(Some(1)).get_rank() + WindowFrameBound::Preceding(None) + < WindowFrameBound::Preceding(Some(1000000)) ); assert!( - WindowFrameBound::CurrentRow.get_rank() - < WindowFrameBound::Following(Some(1)).get_rank() + WindowFrameBound::Preceding(None) + < WindowFrameBound::Preceding(Some(u64::MAX)) ); + assert!(WindowFrameBound::Preceding(None) < WindowFrameBound::Following(Some(0))); assert!( - WindowFrameBound::Following(Some(1)).get_rank() - < WindowFrameBound::Following(Some(2)).get_rank() + WindowFrameBound::Preceding(Some(1)) < WindowFrameBound::Following(Some(1)) ); + assert!(WindowFrameBound::CurrentRow < WindowFrameBound::Following(Some(1))); assert!( - WindowFrameBound::Following(Some(2)).get_rank() - < WindowFrameBound::Following(None).get_rank() + WindowFrameBound::Following(Some(1)) < WindowFrameBound::Following(Some(2)) ); + assert!(WindowFrameBound::Following(Some(2)) < WindowFrameBound::Following(None)); assert!( - WindowFrameBound::Following(Some(u64::MAX)).get_rank() - < WindowFrameBound::Following(None).get_rank() + WindowFrameBound::Following(Some(u64::MAX)) + < WindowFrameBound::Following(None) ); } }