privatize rank

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 <expr> 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<Ordering> {
         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)
         );
     }
 }