From 443aa1732905264161b953c33cb944be2f5142e0 Mon Sep 17 00:00:00 2001 From: Hardik Prajapati Date: Wed, 7 Apr 2021 23:10:48 +0530 Subject: [PATCH 1/4] AAdded failing test for tie scenario in topk --- src/collector/facet_collector.rs | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/collector/facet_collector.rs b/src/collector/facet_collector.rs index 0e6432d5f..077da6b0f 100644 --- a/src/collector/facet_collector.rs +++ b/src/collector/facet_collector.rs @@ -657,6 +657,45 @@ mod tests { ); } } + + #[test] + fn test_facet_collector_topk_tie_break() { + let mut schema_builder = Schema::builder(); + let facet_field = schema_builder.add_facet_field("facet", INDEXED); + let schema = schema_builder.build(); + let index = Index::create_in_ram(schema); + + let docs: Vec = vec![("b", 2), ("a", 2), ("c", 4)] + .into_iter() + .flat_map(|(c, count)| { + let facet = Facet::from(&format!("/facet/{}", c)); + let doc = doc!(facet_field => facet); + iter::repeat(doc).take(count) + }) + .collect(); + + let mut index_writer = index.writer_for_tests().unwrap(); + for doc in docs { + index_writer.add_document(doc); + } + index_writer.commit().unwrap(); + let searcher = index.reader().unwrap().searcher(); + + let mut facet_collector = FacetCollector::for_field(facet_field); + facet_collector.add_facet("/facet"); + let counts: FacetCounts = searcher.search(&AllQuery, &facet_collector).unwrap(); + + { + let facets: Vec<(&Facet, u64)> = counts.top_k("/facet", 2); + assert_eq!( + facets, + vec![ + (&Facet::from("/facet/c"), 4), + (&Facet::from("/facet/a"), 2), + ] + ); + } + } } #[cfg(all(test, feature = "unstable"))] From 50eea4376b256974f28786a62caa440b599897df Mon Sep 17 00:00:00 2001 From: Hardik Prajapati Date: Wed, 7 Apr 2021 23:14:38 +0530 Subject: [PATCH 2/4] Implementation of Ord trait changed for Hit - This will result in lexicographical ordering of facet in BinaryHeep in case of a tie --- src/collector/facet_collector.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/collector/facet_collector.rs b/src/collector/facet_collector.rs index 077da6b0f..108a5d1f3 100644 --- a/src/collector/facet_collector.rs +++ b/src/collector/facet_collector.rs @@ -37,7 +37,10 @@ impl<'a> PartialOrd> for Hit<'a> { impl<'a> Ord for Hit<'a> { fn cmp(&self, other: &Self) -> Ordering { - other.count.cmp(&self.count) + match other.count.cmp(&self.count) { + Ordering::Equal => self.facet.cmp(other.facet), + x => x, + } } } From 54decc60bb91077ad869eafa31bf3e6b1b22e655 Mon Sep 17 00:00:00 2001 From: Hardik Prajapati Date: Wed, 7 Apr 2021 23:59:04 +0530 Subject: [PATCH 3/4] Fixed formatting using cargo fmt --- src/collector/facet_collector.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/collector/facet_collector.rs b/src/collector/facet_collector.rs index 108a5d1f3..9e53d54ce 100644 --- a/src/collector/facet_collector.rs +++ b/src/collector/facet_collector.rs @@ -692,10 +692,7 @@ mod tests { let facets: Vec<(&Facet, u64)> = counts.top_k("/facet", 2); assert_eq!( facets, - vec![ - (&Facet::from("/facet/c"), 4), - (&Facet::from("/facet/a"), 2), - ] + vec![(&Facet::from("/facet/c"), 4), (&Facet::from("/facet/a"), 2)] ); } } From 71309c552802b180d6a61274f1fd27de5dfc266a Mon Sep 17 00:00:00 2001 From: Hardik Prajapati Date: Thu, 8 Apr 2021 01:05:34 +0530 Subject: [PATCH 4/4] Simplified chain orderings --- src/collector/facet_collector.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/collector/facet_collector.rs b/src/collector/facet_collector.rs index 9e53d54ce..db5618442 100644 --- a/src/collector/facet_collector.rs +++ b/src/collector/facet_collector.rs @@ -37,10 +37,10 @@ impl<'a> PartialOrd> for Hit<'a> { impl<'a> Ord for Hit<'a> { fn cmp(&self, other: &Self) -> Ordering { - match other.count.cmp(&self.count) { - Ordering::Equal => self.facet.cmp(other.facet), - x => x, - } + other + .count + .cmp(&self.count) + .then(self.facet.cmp(other.facet)) } }