diff --git a/quickwit/quickwit-doc-mapper/src/doc_mapper/borrowed_json.rs b/quickwit/quickwit-doc-mapper/src/doc_mapper/borrowed_json.rs index f47fa1027fc..2d7f147eccb 100644 --- a/quickwit/quickwit-doc-mapper/src/doc_mapper/borrowed_json.rs +++ b/quickwit/quickwit-doc-mapper/src/doc_mapper/borrowed_json.rs @@ -47,7 +47,9 @@ static SERDE_JSON_PRESERVES_ORDER: LazyLock = LazyLock::new(|| { json_obj.keys().next().map(String::as_str) == Some("b") }); -pub(crate) fn serde_json_preserves_order() -> bool { +/// Returns true if `serde_json::Map` (and therefore [`BorrowedObject`]) iterates in insertion order +/// rather than in sorted key order. +pub fn serde_json_preserves_order() -> bool { *SERDE_JSON_PRESERVES_ORDER } diff --git a/quickwit/quickwit-doc-mapper/src/doc_mapper/mod.rs b/quickwit/quickwit-doc-mapper/src/doc_mapper/mod.rs index 2bbcc853bb5..7ce1df34dd7 100644 --- a/quickwit/quickwit-doc-mapper/src/doc_mapper/mod.rs +++ b/quickwit/quickwit-doc-mapper/src/doc_mapper/mod.rs @@ -32,7 +32,9 @@ use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::ops::Bound; -pub use borrowed_json::{BorrowedJsonDoc, BorrowedObject, BorrowedValue}; +pub use borrowed_json::{ + BorrowedJsonDoc, BorrowedObject, BorrowedValue, serde_json_preserves_order, +}; pub use doc_mapper_builder::DocMapperBuilder; pub use doc_mapper_impl::DocMapper; pub use field_mapping_entry::{ diff --git a/quickwit/quickwit-doc-mapper/src/lib.rs b/quickwit/quickwit-doc-mapper/src/lib.rs index 1f6232776ca..03e1d87a69e 100644 --- a/quickwit/quickwit-doc-mapper/src/lib.rs +++ b/quickwit/quickwit-doc-mapper/src/lib.rs @@ -35,7 +35,7 @@ pub use doc_mapper::{ Automaton, BinaryFormat, BorrowedJsonDoc, BorrowedObject, BorrowedValue, DocMapper, DocMapperBuilder, FastFieldWarmupInfo, FieldMappingEntry, FieldMappingType, JsonObject, NamedField, QuickwitBytesOptions, QuickwitJsonOptions, TermRange, TokenizerConfig, - TokenizerEntry, WarmupInfo, analyze_text, + TokenizerEntry, WarmupInfo, analyze_text, serde_json_preserves_order, }; use doc_mapper::{ FastFieldOptions, FieldMappingEntryForSerialization, IndexRecordOptionSchema, diff --git a/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter.rs b/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter.rs index 3afa34210d6..77be9439b4a 100644 --- a/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter.rs +++ b/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter.rs @@ -65,7 +65,7 @@ use fnv::FnvHasher; use quickwit_config::{ ClusteringMethod, ClusteringPolicy, DocsClusteringConfig, FingerprintPolicy, JsonPath, }; -use quickwit_doc_mapper::BorrowedJsonDoc; +use quickwit_doc_mapper::{BorrowedJsonDoc, serde_json_preserves_order}; use serde_json::Value as JsonValue; use smallvec::SmallVec; @@ -176,6 +176,7 @@ fn is_excluded(exclude: &[JsonPath], path: &[&str]) -> bool { }) } +/// Collects the paths of the leaves of the JSON tree, in iteration order. fn collect_leaf_paths<'a, V: JsonView<'a>>( json_view: V, exclude: &[JsonPath], @@ -195,19 +196,82 @@ fn collect_leaf_paths<'a, V: JsonView<'a>>( }); } +fn hash_leaf_path(path: &[&str], hasher: &mut FnvHasher) { + for component in path { + hasher.write(component.as_bytes()); + hasher.write_u8(PATH_COMPONENT_SEPARATOR); + } + hasher.write_u8(PATH_SEPARATOR); +} + +/// Hashes the paths of the leaves of the JSON tree, in iteration order, without collecting them. +fn hash_leaf_paths_in_iteration_order<'a, V: JsonView<'a>>( + json_view: V, + exclude: &[JsonPath], + current: &mut Vec<&'a str>, + hasher: &mut FnvHasher, +) { + if !matches!(json_view.kind(), JsonViewKind::Object { .. }) { + hash_leaf_path(current, hasher); + return; + } + json_view.for_each_entry(|key, child_view| { + current.push(key); + if !is_excluded(exclude, current) { + hash_leaf_paths_in_iteration_order(child_view, exclude, current, hasher); + } + current.pop(); + }); +} + +/// Hashes the sorted list of the leaf paths of the JSON tree. +/// +/// When object entries iterate in sorted key order (the default `serde_json::Map`), a depth-first +/// walk already visits the leaf paths in sorted order: +/// - the paths of the subtree of a key are compared on that key first, and keys of an object are +/// unique and visited in increasing order; +/// - a leaf path is never a prefix of another leaf path, because a leaf has no children. +/// +/// The paths can then be hashed as they are visited. Otherwise (with the `preserve_order` feature +/// of `serde_json`), they are collected and sorted first. fn hash_structure<'a>(json_view: impl JsonView<'a>, exclude: &[JsonPath], hasher: &mut FnvHasher) { let mut current = Vec::with_capacity(16); + if !serde_json_preserves_order() { + hash_leaf_paths_in_iteration_order(json_view, exclude, &mut current, hasher); + return; + } let mut paths = Vec::with_capacity(32); collect_leaf_paths(json_view, exclude, &mut current, &mut paths); paths.sort_unstable(); + for path in paths { + hash_leaf_path(&path, hasher); + } +} +#[cfg(test)] +pub(super) fn hash_structure_collecting_paths<'a>( + json_view: impl JsonView<'a>, + exclude: &[JsonPath], +) -> u64 { + let mut hasher = FnvHasher::default(); + let mut current = Vec::new(); + let mut paths = Vec::new(); + collect_leaf_paths(json_view, exclude, &mut current, &mut paths); + paths.sort_unstable(); for path in paths { - for component in path { - hasher.write(component.as_bytes()); - hasher.write_u8(PATH_COMPONENT_SEPARATOR); - } - hasher.write_u8(PATH_SEPARATOR); + hash_leaf_path(&path, &mut hasher); } + hasher.finish() +} + +#[cfg(test)] +pub(super) fn hash_structure_for_test<'a>( + json_view: impl JsonView<'a>, + exclude: &[JsonPath], +) -> u64 { + let mut hasher = FnvHasher::default(); + hash_structure(json_view, exclude, &mut hasher); + hasher.finish() } fn hash_raw_value_inner<'a, V: JsonView<'a>>(json_view: V, hasher: &mut FnvHasher) { diff --git a/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter_tests.rs b/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter_tests.rs index 142940ea11d..312374847c2 100644 --- a/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter_tests.rs +++ b/quickwit/quickwit-indexing/src/docs_clustering/fingerprinter_tests.rs @@ -12,11 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -use quickwit_config::DocsClusteringConfig; +use quickwit_config::{DocsClusteringConfig, JsonPath}; use quickwit_doc_mapper::{BorrowedJsonDoc, RandomJsonDocs}; use serde_json::Value as JsonValue; -use super::fingerprinter::Fingerprinter; +use super::fingerprinter::{ + Fingerprinter, hash_structure_collecting_paths, hash_structure_for_test, +}; +use super::json_view::BorrowedJsonNode; fn parse(s: &str) -> JsonValue { serde_json::from_str(s).unwrap() @@ -333,3 +336,80 @@ fn borrowed_fingerprint_same_as_owned_fingerprint_random() { assert_same_fingerprint(&fingerprinter, &json_doc); } } + +/// Documents with nested objects, keys sharing prefixes, and excluded paths, for the structure +/// policy. +const STRUCTURE_DOCS: &[&str] = &[ + r#"{}"#, + r#"{"a": 1}"#, + r#"{"a": {}, "b": 1}"#, + r#"{"b": 1, "a": {"y": 1, "x": {"z": null}}, "ab": [1, {"k": 2}], "a.b": 3}"#, + r#"{"attributes": {"z": 1, "a": 2}, "inner": {"body": "x", "zone": "eu"}, "": {"": 1}}"#, + r#"{"é": 1, "e": 2, "Z": 3, "z": {"é": {"a": 1}, "e": 4}, "z": {"b": 1}}"#, +]; + +/// Structure fingerprints computed before hashing leaf paths in iteration order: the hash must not +/// change, as fingerprints of a split are compared with each other. +#[test] +fn structure_fingerprints_are_stable() { + let expected_fingerprints: [[u64; 2]; 6] = [ + [14695981039346656037, 14695981039346656037], + [16538397715120493737, 16538397715120493737], + [18363328632233708152, 18363328632233708152], + [17645136403288981598, 17645136403288981598], + [1195699640648630522, 546240098895012005], + [1896699566821316143, 1896699566821316143], + ]; + let fingerprinter = Fingerprinter::new(&differential_docs_clustering_config()); + for (json_doc, expected_fingerprint) in STRUCTURE_DOCS.iter().zip(expected_fingerprints) { + let json_value: JsonValue = serde_json::from_str(json_doc).unwrap(); + let borrowed_json_doc = BorrowedJsonDoc::parse(json_doc.as_bytes()).unwrap(); + assert_eq!( + fingerprinter.fingerprint(&json_value)[..2], + expected_fingerprint, + "doc: {json_doc}" + ); + assert_eq!( + fingerprinter.fingerprint_borrowed(&borrowed_json_doc)[..2], + expected_fingerprint, + "doc: {json_doc}" + ); + } +} + +fn assert_same_structure_hash(json_doc: &str, exclude: &[JsonPath]) { + let json_value: JsonValue = serde_json::from_str(json_doc).unwrap(); + let borrowed_json_doc = BorrowedJsonDoc::parse(json_doc.as_bytes()).unwrap(); + let borrowed_json_node = BorrowedJsonNode::Root(&borrowed_json_doc); + let expected_hash = hash_structure_collecting_paths(&json_value, exclude); + assert_eq!( + hash_structure_for_test(&json_value, exclude), + expected_hash, + "doc: {json_doc}" + ); + assert_eq!( + hash_structure_for_test(borrowed_json_node, exclude), + expected_hash, + "doc: {json_doc}" + ); +} + +/// Hashing leaf paths as they are visited must be equivalent to collecting and sorting them. +#[test] +fn structure_hash_same_as_sorted_leaf_paths_hash() { + let excludes: Vec> = vec![ + Vec::new(), + serde_json::from_str(r#"["a", "attributes", "inner.body", "z.é"]"#).unwrap(), + ]; + let mut random_json_docs = RandomJsonDocs::new(0x51_7cc1_b727_220a); + let random_docs: Vec = (0..10_000).map(|_| random_json_docs.next_doc()).collect(); + let json_docs = STRUCTURE_DOCS + .iter() + .copied() + .chain(random_docs.iter().map(String::as_str)); + for json_doc in json_docs { + for exclude in &excludes { + assert_same_structure_hash(json_doc, exclude); + } + } +}