Skip to content

Commit 4c9678a

Browse files
authored
Merge pull request #15 from dapper91/fix/sorting-stability
sorting stability bug fixed.
2 parents 2b27589 + a94d5c5 commit 4c9678a

File tree

2 files changed

+46
-5
lines changed

2 files changed

+46
-5
lines changed

src/merger.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ where
6565
C: IntoIterator<Item = Result<T, E>>,
6666
{
6767
// binary heap is max-heap by default so we reverse it to convert it to min-heap
68-
items: BinaryHeap<(std::cmp::Reverse<OrderedWrapper<T, F>>, usize)>,
68+
items: BinaryHeap<(std::cmp::Reverse<OrderedWrapper<T, F>>, std::cmp::Reverse<usize>)>,
6969
chunks: Vec<C::IntoIter>,
7070
initiated: bool,
7171
compare: F,
@@ -114,7 +114,7 @@ where
114114
match item {
115115
Ok(item) => self
116116
.items
117-
.push((std::cmp::Reverse(OrderedWrapper::wrap(item, self.compare)), idx)),
117+
.push((std::cmp::Reverse(OrderedWrapper::wrap(item, self.compare)), std::cmp::Reverse(idx))),
118118
Err(err) => return Some(Err(err)),
119119
}
120120
}
@@ -123,7 +123,7 @@ where
123123
}
124124

125125
let (result, idx) = self.items.pop()?;
126-
if let Some(item) = self.chunks[idx].next() {
126+
if let Some(item) = self.chunks[idx.0].next() {
127127
match item {
128128
Ok(item) => self
129129
.items

src/sort.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,10 @@ mod test {
348348
fn test_external_sorter(#[case] reversed: bool) {
349349
let input_sorted = 0..100;
350350

351-
let mut input: Vec<Result<i32, io::Error>> = Vec::from_iter(input_sorted.clone().map(|item| Ok(item)));
352-
input.shuffle(&mut rand::thread_rng());
351+
let mut input_shuffled = Vec::from_iter(input_sorted.clone());
352+
input_shuffled.shuffle(&mut rand::thread_rng());
353+
354+
let input: Vec<Result<i32, io::Error>> = Vec::from_iter(input_shuffled.into_iter().map(|item| Ok(item)));
353355

354356
let sorter: ExternalSorter<i32, _> = ExternalSorterBuilder::new()
355357
.with_buffer(LimitedBufferBuilder::new(8, true))
@@ -376,4 +378,43 @@ mod test {
376378

377379
assert_eq!(actual_result, expected_result)
378380
}
381+
382+
#[rstest]
383+
#[case(false)]
384+
#[case(true)]
385+
fn test_external_sorter_stability(#[case] reversed: bool) {
386+
let input_sorted = (0..20).flat_map(|x|(0..5).map(move |y| (x, y)));
387+
388+
let mut input_shuffled = Vec::from_iter(input_sorted.clone());
389+
input_shuffled.shuffle(&mut rand::thread_rng());
390+
// sort input by the second field to check sorting stability
391+
input_shuffled.sort_by(|a: &(i32, i32), b: &(i32, i32)| if reversed {a.1.cmp(&b.1).reverse()} else {a.1.cmp(&b.1)});
392+
393+
let input: Vec<Result<(i32, i32), io::Error>> = Vec::from_iter(input_shuffled.into_iter().map(|item| Ok(item)));
394+
395+
let sorter: ExternalSorter<(i32, i32), _> = ExternalSorterBuilder::new()
396+
.with_buffer(LimitedBufferBuilder::new(8, true))
397+
.with_threads_number(2)
398+
.with_tmp_dir(Path::new("./"))
399+
.build()
400+
.unwrap();
401+
402+
let compare = if reversed {
403+
|a: &(i32, i32), b: &(i32, i32)| a.0.cmp(&b.0).reverse()
404+
} else {
405+
|a: &(i32, i32), b: &(i32, i32)| a.0.cmp(&b.0)
406+
};
407+
408+
let result = sorter.sort_by(input, compare).unwrap();
409+
410+
let actual_result: Result<Vec<(i32, i32)>, _> = result.collect();
411+
let actual_result = actual_result.unwrap();
412+
let expected_result = if reversed {
413+
Vec::from_iter(input_sorted.clone().rev())
414+
} else {
415+
Vec::from_iter(input_sorted.clone())
416+
};
417+
418+
assert_eq!(actual_result, expected_result)
419+
}
379420
}

0 commit comments

Comments
 (0)