diff --git a/src/collector/facet_collector.rs b/src/collector/facet_collector.rs index 6eb2c3ee7..92de31470 100644 --- a/src/collector/facet_collector.rs +++ b/src/collector/facet_collector.rs @@ -389,6 +389,13 @@ impl SegmentCollector for FacetSegmentCollector { } let mut facet = vec![]; let (facet_ord, facet_depth) = self.unique_facet_ords[collapsed_facet_ord]; + // u64::MAX is used as a sentinel for unmapped ordinals (e.g. when a + // document has the exact registered facet, not a child of it). + // Passing it to ord_to_term would resolve to the last dictionary + // entry and produce a spurious facet from an unrelated branch. + if facet_ord == u64::MAX { + continue; + } // TODO handle errors. if facet_dict.ord_to_term(facet_ord, &mut facet).is_ok() { if let Some((end_collapsed_facet, _)) = facet @@ -814,6 +821,63 @@ mod tests { assert!(!super::is_child_facet(&b"foo\0bar"[..], &b"foo"[..])); assert!(!super::is_child_facet(&b"foo"[..], &b"foobar\0baz"[..])); } + + // Regression test for https://github.com/quickwit-oss/tantivy/issues/2494 + // When a document has the exact registered facet path (not just a child), + // harvest() must not turn the unmapped sentinel into a spurious root entry. + #[test] + fn test_facet_collector_wrong_root() -> crate::Result<()> { + let mut schema_builder = Schema::builder(); + let facet_field = schema_builder.add_facet_field("facet", FacetOptions::default()); + let schema = schema_builder.build(); + let index = Index::create_in_ram(schema); + + let mut index_writer: IndexWriter = index.writer_for_tests()?; + let facets: Vec<&str> = vec![ + "/science-fiction/asimov", + "/science-fiction/clarke", + "/science-fiction/dick", + "/science-fiction/herbert", + "/science-fiction/orwell", + // This exact match on the registered facet is the bug trigger: + // its ordinal maps to the sentinel (u64::MAX, 0) in the collapse + // mapping, which without the fix resolves to an unrelated term. + "/fantasy/epic-fantasy", + "/fantasy/epic-fantasy/tolkien", + "/fantasy/epic-fantasy/martin", + ]; + for facet_str in &facets { + index_writer.add_document(doc!( + facet_field => Facet::from(*facet_str) + ))?; + } + index_writer.commit()?; + + let reader = index.reader()?; + let searcher = reader.searcher(); + + let term = Term::from_facet(facet_field, &Facet::from("/fantasy/epic-fantasy")); + let query = TermQuery::new(term, IndexRecordOption::Basic); + + let mut facet_collector = FacetCollector::for_field("facet"); + facet_collector.add_facet("/fantasy/epic-fantasy"); + let counts: FacetCounts = searcher.search(&query, &facet_collector)?; + + let result: Vec<(String, u64)> = counts + .get("/") + .map(|(facet, count)| (facet.to_string(), count)) + .collect(); + + // Only children of /fantasy/epic-fantasy should appear, not /science-fiction + assert_eq!( + result, + vec![ + ("/fantasy/epic-fantasy/martin".to_string(), 1), + ("/fantasy/epic-fantasy/tolkien".to_string(), 1), + ] + ); + Ok(()) + } } #[cfg(all(test, feature = "unstable"))]