From a94d5c5e1f5639f748e344bf8756fb10d8dc35ff Mon Sep 17 00:00:00 2001 From: Dmitry Pershin Date: Fri, 9 May 2025 22:24:28 +0500 Subject: [PATCH] fix: sorting stability bug fixed. --- src/merger.rs | 6 +++--- src/sort.rs | 45 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/merger.rs b/src/merger.rs index 6d6666c..118295e 100644 --- a/src/merger.rs +++ b/src/merger.rs @@ -65,7 +65,7 @@ where C: IntoIterator>, { // binary heap is max-heap by default so we reverse it to convert it to min-heap - items: BinaryHeap<(std::cmp::Reverse>, usize)>, + items: BinaryHeap<(std::cmp::Reverse>, std::cmp::Reverse)>, chunks: Vec, initiated: bool, compare: F, @@ -114,7 +114,7 @@ where match item { Ok(item) => self .items - .push((std::cmp::Reverse(OrderedWrapper::wrap(item, self.compare)), idx)), + .push((std::cmp::Reverse(OrderedWrapper::wrap(item, self.compare)), std::cmp::Reverse(idx))), Err(err) => return Some(Err(err)), } } @@ -123,7 +123,7 @@ where } let (result, idx) = self.items.pop()?; - if let Some(item) = self.chunks[idx].next() { + if let Some(item) = self.chunks[idx.0].next() { match item { Ok(item) => self .items diff --git a/src/sort.rs b/src/sort.rs index 33342ea..88c2397 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -348,8 +348,10 @@ mod test { fn test_external_sorter(#[case] reversed: bool) { let input_sorted = 0..100; - let mut input: Vec> = Vec::from_iter(input_sorted.clone().map(|item| Ok(item))); - input.shuffle(&mut rand::thread_rng()); + let mut input_shuffled = Vec::from_iter(input_sorted.clone()); + input_shuffled.shuffle(&mut rand::thread_rng()); + + let input: Vec> = Vec::from_iter(input_shuffled.into_iter().map(|item| Ok(item))); let sorter: ExternalSorter = ExternalSorterBuilder::new() .with_buffer(LimitedBufferBuilder::new(8, true)) @@ -376,4 +378,43 @@ mod test { assert_eq!(actual_result, expected_result) } + + #[rstest] + #[case(false)] + #[case(true)] + fn test_external_sorter_stability(#[case] reversed: bool) { + let input_sorted = (0..20).flat_map(|x|(0..5).map(move |y| (x, y))); + + let mut input_shuffled = Vec::from_iter(input_sorted.clone()); + input_shuffled.shuffle(&mut rand::thread_rng()); + // sort input by the second field to check sorting stability + input_shuffled.sort_by(|a: &(i32, i32), b: &(i32, i32)| if reversed {a.1.cmp(&b.1).reverse()} else {a.1.cmp(&b.1)}); + + let input: Vec> = Vec::from_iter(input_shuffled.into_iter().map(|item| Ok(item))); + + let sorter: ExternalSorter<(i32, i32), _> = ExternalSorterBuilder::new() + .with_buffer(LimitedBufferBuilder::new(8, true)) + .with_threads_number(2) + .with_tmp_dir(Path::new("./")) + .build() + .unwrap(); + + let compare = if reversed { + |a: &(i32, i32), b: &(i32, i32)| a.0.cmp(&b.0).reverse() + } else { + |a: &(i32, i32), b: &(i32, i32)| a.0.cmp(&b.0) + }; + + let result = sorter.sort_by(input, compare).unwrap(); + + let actual_result: Result, _> = result.collect(); + let actual_result = actual_result.unwrap(); + let expected_result = if reversed { + Vec::from_iter(input_sorted.clone().rev()) + } else { + Vec::from_iter(input_sorted.clone()) + }; + + assert_eq!(actual_result, expected_result) + } }